fix(workbench): accept clean git worktree targets - #812
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad533157f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return ["git_worktree", "git_revision"] | ||
| if scan["target_snapshot_digest"] == clean_worktree_content_digest(): | ||
| return ["git_revision"] | ||
| return ["git_revision", "git_worktree"] |
There was a problem hiding this comment.
Bind clean worktree manifests to the recorded digest
When a clean Git checkout uses the newly permitted git_worktree kind, scan_contract still omits requiredSnapshotDigest, so the initial completion binding does not compare the manifest digest with the recorded clean-worktree digest. A clean CLI scan can therefore seal successfully with any syntactically valid snapshotDigest (for example, 64 b characters), leaving the manifest unbound to the reviewed snapshot and causing later revalidation to reject it. Require the recorded clean digest whenever the selected kind is git_worktree, while continuing to let git_revision omit it.
AGENTS.md reference: AGENTS.md:L32-L33
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in commit 0c302296. Clean worktree contracts now carry the recorded requiredSnapshotDigest, and the completion binding applies it when git_worktree is selected. git_revision bindings still omit the snapshot digest, while a supplied worktree digest is normalized to the recorded value before sealing. Added regression coverage for registration, completion, and the clean Git scan path. Validation: 26 completion-binding tests, 108 workbench DB tests, 185 finalization/export tests, Ruff check/format, and plugin source compatibility all pass.
sylvesterkaczmarek
left a comment
There was a problem hiding this comment.
The follow-up closes the important binding gap. Carrying the recorded digest in the contract, then applying it only when the selected manifest kind is git_worktree, preserves revision semantics while making clean-worktree sealing deterministic and snapshot-bound.
Summary
Fixes #62. Bulk scans can legitimately register a clean checkout as a
git_worktree, but the completion contract currently allows onlygit_revisionwhen the worktree content is clean. The scan can finish with valid artifacts and then fail at sealing withscan.target.kind: must match the workbench target.Changes
git_revisionandgit_worktreefor a clean worktree content digest.git_worktreemanifest successfully.Testing
python -m pytest plugins/codex-security/tests/test_workbench_completion_binding.py plugins/codex-security/tests/test_workbench_db.py -q— 134 passedpython -m pytest plugins/codex-security/tests/test_finalize_scan_contract.py plugins/codex-security/tests/test_workbench_db_exports.py -q— 185 passeduvx --from ruff==0.16.1 ruff check --config plugins/codex-security/pyproject.toml .github/scripts/check_plugin_source_compatibility.py .github/scripts/test_check_plugin_source_compatibility.py plugins/codex-security— passeduvx --from ruff==0.16.1 ruff format --check --config plugins/codex-security/pyproject.toml .github/scripts/check_plugin_source_compatibility.py .github/scripts/test_check_plugin_source_compatibility.py plugins/codex-security— passedpython .github/scripts/check_plugin_source_compatibility.py— passedgit diff --check— passedRisk and rollout
This is a compatibility fix for an existing target representation. It does not change the public CLI, target selection, snapshot validation, or the stricter rules for dirty worktrees, diffs, and directory snapshots.
Public disclosure review