fix(workbench): share scan history across linked worktrees - #456
fix(workbench): share scan history across linked worktrees#456mldangelo-oai wants to merge 30 commits into
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eef22c38a6
ℹ️ 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".
|
@codex review |
|
@codex security review |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20b170e3ca
ℹ️ 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".
wholley-oai
left a comment
wholley-oai
left a comment
There was a problem hiding this comment.
I took a cursory glance and have Codex reviewing more thoroughly, but on first pass this looks fine so I'm ok with providing an unblocking stamp.
|
@codex review |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f1eb91dcb
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Security review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
Preserve the published finding-publication migrations and append repository identity at version 31. Normalize the known pre-release identity migration, reuse one identity-schema capability check, and align saved-history tests with the integrated behavior.
Advance the paired standalone plugin version so existing staged marketplaces load the linked-worktree history helpers. Leave the npm package release version unchanged.
Use recorded checkout ownership before falling back to live repository identities. Preserve matching persisted identities for removed worktrees and retain verified same-origin comparisons.
|
@codex review Please review the current head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7f733d281
ℹ️ 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".
|
@codex review Please review exact head |
|
@codex security review Please review the current head |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review Please review the current head |
|
@codex security review Please review the current head |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
kmbroai
left a comment
kmbroai
left a comment
There was a problem hiding this comment.
Reviewed head a2f5676dcef1c70a855ce672b0016cb4987bcd07 for correctness, necessity, and simplification.
Necessity and ownership model
Sharing history across linked worktrees is useful, but it must not equate clones merely because they advertise the same remote or attach an old checkout's decisions to a replacement directory. Persisting generation evidence on each new scan is the important part of this change. Keeping legacy scans target-local instead of retroactively inventing that evidence is the correct conservative boundary.
I traced generation construction, registration, active-scan rejoin checks, history predicates, grouped finding counts, feedback reuse, and the migration. The current generation includes the Git/common object-store directory instances and relative scope; mutable description text and unconditional Windows case folding no longer determine identity. A recreated component directory is accepted when its Git generation and scope remain verified, while a replaced root remains rejected. The earlier product findings in those areas are addressed at this head.
[P2] Make the recreation regression deterministic
The checked-in five-suite run produced 32 passes and 1 failure. workbench-repository-identity.test.ts:860 expected ownerChanged to be true, but it was false. The fixture deletes the old directory, creates one unrelated directory to try to consume its inode, then recreates the target. The allocator is free to reuse the original inode anyway; that extra mkdir is not an identity guarantee.
In a separate test-only copy, I replaced the three delete/consume sequences with renames that retain the old directory inodes, leaving the product code unchanged. The recreated-directory case then passed all 18 assertions, including accepting the recreated component and rejecting the recreated worktree root. Use that deterministic fixture rather than retries or assuming allocator behavior. This is a test defect, not evidence that the repaired component-recreation logic still fails.
Simplification and integration
Land one repository-scope abstraction and make #275 consume it. Maintaining this generation model alongside the browser PR's independent inode/ownership-epoch discovery would create contradictory history semantics. Keep archive rollback and completion-order changes separable where practical; they enlarge an already substantial migration. Preserve the documented distinction between completion-ordered automatic matching and the pre-existing explicit match --all order rather than silently changing that compatibility behavior.
Request-local identity caching and a fixed-size generation predicate are good choices. Do not add a mutable global alias cache or infer durable identity from a remote URL. Linux focused checks and the deterministic fixture ran with cached dependencies; native Windows/macOS and live scan execution were not verified.
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: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aabc44a547
ℹ️ 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".
| if verify_ownership: | ||
| state.require_owner() |
There was a problem hiding this comment.
When creating or saving a workspace, ensure_security_target binds the repository identity before any scan exists; if that repository is then reinitialized at the same path, this unconditional ownership check rejects every subsequent workspace or scan even though there are no scan records to protect. Allow the stored identity to be replaced when no scan references the target or path, and retain the mismatch rejection once history exists.
AGENTS.md reference: AGENTS.md:L9-L9
Useful? React with 👍 / 👎.
4 participants
Summary
Keep saved scans, findings, and reviewer decisions associated with verified Git
worktrees, including after a worktree is removed or its old path is reused.
Changes
feedback, and rerun lineage through saved scan evidence and verified ownership.
history target-local without assigning it a generation.
Preserve the existing ordering of explicit history and
scans match --all.supported prerelease repair and preserve historical records.
queries and production helpers used only by tests. Keep main's shorter README.
0.1.87; keep npm at0.1.20.This PR does not publish a release or add public CLI flags.
Testing
Local first-pass verification on
aabc44a547a30ed9cd71b472c69699ef971f16fa:migration 33 is the only addition.
sandbox denied
psin the process-group test. That exact test passed whenrerun outside the sandbox. No other failures were reported.
After merging main
01bd062, 237 affected tests passed with three platformskips. Types/models, formatting, build, migration-prefix validation, static
package checks and the full installed-package smoke test passed. Real cache
upgrades from 0.1.60 and 0.1.68 matched all 118 installed files and preserved
credentials. The README keeps both shared-worktree guidance and main's Windows
state-recovery instructions without the repetitive command example.
CI is not awaited; its new-head results remain a separate verification step.
Native Windows execution and the full suite were not repeated after this refresh.
fd98a90): package 0.1.21 includes the MCP launcher-permission fix; feature source and bundled payload are unchanged. Types/model generation, formatting, build, 28 focused package/report/launcher tests, static artifact verification and full installed-package smoke passed, including MCP initialization. CI was not awaited.Risk and rollout
Saved task execution stops if the current checkout owner or saved generation no
longer matches. Sealed history and compatible exact-target legacy history remain
available. A target record or another scan never promotes historical scans into
a repository generation.
Migration 33 adds scan-generation evidence and database completion order. Older
writers continue to store NULL generation values. Tests cover legacy migration
repair, removed and replaced worktrees, explicit comparisons, feedback scope,
and archive recovery. No release is published by this PR.
Public disclosure review
The source, commit metadata, description, and public PR surface have been
reviewed. Existing automated review comments contain access-restricted report
references, so the second attestation remains unchecked.