Skip to content

fix(cli): validate agent-review add targets, paths and lines - #663

Merged
Ziinc merged 3 commits into
mainfrom
ccr-b84e8000-3ab84a-agent-review-add
Oct 3, 2026
Merged

Ziinc merged 3 commits into
mainfrom
ccr-b84e8000-3ab84a-agent-review-add

Conversation

@Ziinc

@Ziinc Ziinc commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator
  • agent-review add accepted any --target-type, nonexistent workspace ids, --file /etc/passwd or ../ paths, and line 99999.
  • It now requires workspace_diff (an existing workspace id) or file_browser_file (an existing file inside the repo), a relative --file with no .., and new-side lines within the file on disk. Old-side lines are not bounds-checked, since that file isn't on disk.
  • Test: add_target_must_be_real_and_inside_the_repo.
    🤖 Generated with Claude Code
    https://claude.ai/code/session_01MxqTt1WtVRKkjNXQpgkJS9
    Generated by Claude Code

`agent-review add` stored comments nothing could display: any target
type, workspace ids that don't exist, absolute or `..` file paths, and
lines past the end of the file. It now requires a known target type
(workspace_diff, file_browser_file), an existing target, a relative
--file, and new-side lines within the file on disk.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MxqTt1WtVRKkjNXQpgkJS9
Comment thread src-tauri/src/cli/agent_review_handlers.rs Outdated

@Ziinc Ziinc left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed correctness, surrounding call paths, code quality and regression coverage at 3d4b955b. Found 2 actionable issues; details and suggested regression checks are inline.

Validation: source review and GitHub workflow inspection. The PR-head CI workflow reports success. Rust/integration tests were not rerun locally because this environment has no Cargo toolchain.

Comment thread src-tauri/src/cli/agent_review_handlers.rs Outdated
Comment thread src-tauri/src/cli/agent_review_handlers.rs Outdated
Resolve --file against its root (workspace or repo), canonicalize it and
require it to stay inside the canonical root and be a regular file before
counting lines. For file_browser_file, --target-id must resolve to that
same file.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MxqTt1WtVRKkjNXQpgkJS9
Keep both new agent-review tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MxqTt1WtVRKkjNXQpgkJS9
@Ziinc
Ziinc merged commit da30615 into main Oct 3, 2026
22 checks passed
@Ziinc
Ziinc deleted the ccr-b84e8000-3ab84a-agent-review-add branch October 3, 2026 12:01

This branch was successfully deployed

1 active deployment
preview — 22bd5d4c Deployed Oct 3, 2026 by Ziinc via build #1675
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants