fix: v0.5.18 P1 follow-ups — diff guard, prompt injection, docs, timeout#648
fix: v0.5.18 P1 follow-ups — diff guard, prompt injection, docs, timeout#648anandgupta42 merged 4 commits intomainfrom
Conversation
…s, timeout, changelog - GitLab MR diff size guard: truncate diffs at 50 files / 200KB to avoid context overflow with clear warning message - Prompt injection mitigation: frame MR content as untrusted data in the system prompt - Rewrite `docs/usage/gitlab.md` with correct `altimate gitlab review` command, CI setup, auth docs, and known limitations - Add 15s timeout to `validateCredentials` fetch to prevent TUI hang on network partition - Fix changelog wording: `/connect` provider dialog, not `/login`
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughApplies v0.5.18 follow-ups: update changelog TUI text, refresh GitLab docs (quick start, auth, model selection, known limitations), add 15s AbortController timeout to credential validation, truncate large GitLab MR diffs with CLI warning, and mark MR content as untrusted in prompts. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "CLI (altimate gitlab review)"
participant GitLab as "GitLab API"
participant Trunc as "Diff Truncation\n(max files / bytes)"
participant LLM as "LLM Service"
CLI->>GitLab: fetch MR metadata, comments, changes
GitLab-->>CLI: MR title/desc/comments/diffs
CLI->>Trunc: evaluate changes size
alt diff exceeds limits
Trunc-->>CLI: truncated diffs + truncated=true
CLI->>CLI: append <diffs> note about omitted files
CLI->>CLI: print CLI warning about oversized MR
else within limits
Trunc-->>CLI: full diffs + truncated=false
end
CLI->>LLM: build prompt (frame MR content as untrusted) and send
LLM-->>CLI: review output
sequenceDiagram
participant TUI as "TUI / Client"
participant AltimateAPI as "AltimateApi.validateCredentials"
participant Network as "Altimate Gateway / Network"
TUI->>AltimateAPI: validateCredentials(request)
AltimateAPI->>Network: fetch(..., signal from AbortController)
alt no response within 15s
AltimateAPI->>AltimateAPI: AbortController.abort()
Network-->>AltimateAPI: aborted/error
else response within 15s
Network-->>AltimateAPI: validation response
end
AltimateAPI-->>TUI: return validation result or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/docs/usage/gitlab.md`:
- Around line 74-77: The doc text "50+ files" is inaccurate; update the wording
to match the code behavior which truncates when totalFiles > MAX_DIFF_FILES
(i.e., more than 50 files) — reference the logic using changes.slice(0,
MAX_DIFF_FILES) and truncated = totalFiles > MAX_DIFF_FILES — so change "50+
files" to "more than 50 files" or "51+ files" to reflect the actual truncation
threshold.
In `@packages/opencode/src/cli/cmd/gitlab.ts`:
- Around line 386-390: The CLI warning only checks file count
(mrData.changes.length > MAX_DIFF_FILES) but misses byte-size truncation; call
truncateDiffs earlier on mrData.changes (using the existing truncateDiffs
function and its returned truncated flag) and change the warning condition to
use either truncated === true or mrData.changes.length > MAX_DIFF_FILES so the
UI.println message shows when truncation occurred due to MAX_DIFF_BYTES as well;
update the block around mrData.changes/truncateDiffs and the warning print to
reference the truncateDiffs result and mrData.changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 51fda3b7-221e-4e0a-89ef-d82d86523c7f
📒 Files selected for processing (4)
CHANGELOG.mddocs/docs/usage/gitlab.mdpackages/opencode/src/altimate/api/client.tspackages/opencode/src/cli/cmd/gitlab.ts
Key change: move multi-persona code review to before testing. Actionable P1s found by reviewers are fixed on main before the tag, not as follow-up PRs after release. Old: Test → Review → Tag → follow-up PRs New: Review → Fix → Test → Tag → done clean
- Correct truncation threshold wording in docs (more than 50 / exceeding 200 KB) - CLI warning now fires for both file-count and byte-size truncation by checking truncateDiffs() result instead of file count alone
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/commands/release.md:
- Line 344: The fenced code block containing the "## Release Summary:
v{NEXT_VERSION}" header lacks a language identifier and triggers MD040; update
the opening fence from ``` to ```markdown so the block is labeled as markdown
(leave the block contents and closing fence unchanged) to satisfy the linter.
- Around line 253-254: The changelog format in .claude/commands/release.md
requires bold section headers (Added/Fixed/Changed) with an em-dash style
description, but script/changelog.ts currently emits area-based sections (##
Core, ## TUI, etc.), so update the generator in script/changelog.ts (the
function that assembles changelog entries / generateChangelog or
buildChangelogEntries) to group entries into the three bold categories (Added,
Fixed, Changed), convert each summarized commit into a single bullet using the
"Bold Title — description" pattern, and include the persona Step 4 feedback text
when present; ensure the output format exactly matches the release.md
requirement (bold title + em-dash description), update any unit/integration
tests for script/changelog.ts to expect the new format, and keep area grouping
only as optional sub-context inside those three categories if needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 75f28e2f-7b09-4010-b39d-2cfb6b4547ea
📒 Files selected for processing (1)
.claude/commands/release.md
| Categorize into **Added**, **Fixed**, **Changed**. Use bold title + em-dash description matching existing style. Incorporate release notes feedback from the Step 4 persona reviews. | ||
|
|
There was a problem hiding this comment.
Changelog instructions here conflict with the current generator behavior.
This file mandates Added/Fixed/Changed and “bold title + em-dash” style, but script/changelog.ts currently groups by area (## Core, ## TUI, ## Desktop, ## SDK, ## Extensions) and builds bullet entries from summarized commits. This mismatch can cause release-process drift and inconsistent release notes.
Suggested alignment patch
-### 8b: Write the changelog entry
-
-Categorize into **Added**, **Fixed**, **Changed**. Use bold title + em-dash description matching existing style. Incorporate release notes feedback from the Step 4 persona reviews.
+### 8b: Write the changelog entry
+
+Follow the current generated format used by `script/changelog.ts`:
+- Group by code area sections (for example: `## Core`, `## TUI`, `## Desktop`, `## SDK`, `## Extensions`)
+- Keep concise bullet entries under each section
+- Incorporate release notes feedback from the Step 4 persona reviews-10. **Changelog entries must match existing style.** Bold titles with em-dash descriptions.
+10. **Changelog entries must match existing generated style.** Group entries under area sections with concise bullets.Also applies to: 379-380
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/commands/release.md around lines 253 - 254, The changelog format in
.claude/commands/release.md requires bold section headers (Added/Fixed/Changed)
with an em-dash style description, but script/changelog.ts currently emits
area-based sections (## Core, ## TUI, etc.), so update the generator in
script/changelog.ts (the function that assembles changelog entries /
generateChangelog or buildChangelogEntries) to group entries into the three bold
categories (Added, Fixed, Changed), convert each summarized commit into a single
bullet using the "Bold Title — description" pattern, and include the persona
Step 4 feedback text when present; ensure the output format exactly matches the
release.md requirement (bold title + em-dash description), update any
unit/integration tests for script/changelog.ts to expect the new format, and
keep area grouping only as optional sub-context inside those three categories if
needed.
|
|
||
| ### 12c: Release summary | ||
|
|
||
| ``` |
There was a problem hiding this comment.
Add a language to the fenced block to satisfy markdown linting.
The fenced code block starting on this line has no language identifier (MD040).
Suggested lint fix
-```
+```markdown
## Release Summary: v{NEXT_VERSION}
...
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 344-344: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/commands/release.md at line 344, The fenced code block containing
the "## Release Summary: v{NEXT_VERSION}" header lacks a language identifier and
triggers MD040; update the opening fence from ``` to ```markdown so the block is
labeled as markdown (leave the block contents and closing fence unchanged) to
satisfy the linter.
What does this PR do?
Addresses the 5 P1 issues identified during the v0.5.18 multi-persona release evaluation:
truncateDiffs()caps diffs at 50 files / 200KB with clear user warning, preventing context overflow errors on large MRsdocs/usage/gitlab.mdwith correctaltimate gitlab reviewcommand, CI setup, auth docs, self-hosted config, and known limitationsvalidateCredentialstimeout — 15sAbortSignaltimeout on the credential validation fetch, preventing indefinite TUI hang on network partition/loginreference to correctly say "provider dialog → Altimate"Type of change
Issue for this PR
Closes #647
How did you verify your code works?
Checklist
Summary by CodeRabbit
New Features
Documentation
Bug Fixes