fix: worktree-guard aligns to canonical 2-segment worktree paths (OPE61-00004) - #11
Merged
Conversation
managedWorktreeInfo() enforced a 3-segment <station-id>/<repo-slug>-<hex>/wt_<hex> lease layout, so every worktree at the canonical rule-8 path was classified UNMANAGED. Agent Operating Rules rule 8 (@hasna/identities >= 0.4.4) mandates $HOME/.hasna/repos/worktrees/<repo-name>/<worktree-name> — "repo name then worktree name; no station-id or machine segment, never flat under the worktrees root". Impact was not limited to the worktree-guard git block: managedLeaseRoot() feeds classifyDangerousOperation(), which is also used by the registered pre-bash hook, so canonical worktrees were denied the scoped ~/.hasna write carve-out and Write/Edit/apply_patch into them was blocked outright. - Classify against the canonical 2-segment shape; accept subdirectories of a canonical worktree by resolving the worktree root, not by path depth alone. - Reject flat, station-id-prefixed, and over-nested shapes with a specific reason that cites the canonical template. - Keep the station-id lease layout as clearly-labelled deprecated read-only tolerance for the dangerous-operation carve-out only, so pre-rule-8 worktrees are not stranded mid-migration. Git provenance verification is unchanged and still fails closed at both depths. - Guard messages recommend `git worktree add` at the canonical path plus `repos scan`; the repos CLI has no worktree verb. - Allow a leading underscore in path segments (fleet has connectors/_base); keep refusing a leading . or -. Verified against all 708 real worktrees under ~/.hasna/repos/worktrees: 198 canonical accepted, 303 flat / 160 over-nested / 47 legacy-lease rejected with reasons. Task: OPE61-00004
…ame via repos CLI Addresses two adversarial review rounds on the canonical 2-segment change. Reviewer 1 (canon claims) — BLOCKER: claimCommand() derived <repo-name> from the git remote basename, but rule 8 mandates the exact repos CLI lookup. The remote basename names a different directory for 46 of 50 indexed repos (open-hooks is github.com/hasna/hooks), so the guard pointed agents at a path that does not exist — and disagreed with managedWorktreeInfo() inside a single hook run. Resolution now uses `repos repo --remote <host/org/name> --json` (the exact, non-fuzzy form), falling back to the local checkout directory, and takes the real default_branch instead of an unrunnable placeholder. Best-effort: a missing repos CLI degrades to local information. Reviewer 2 (regression risk) — BLOCKER: depth 2 was accepted on path shape alone, so `<root>/<flat-worktree>/<subdir>` — shape-identical to the canonical path — laundered a forbidden flat worktree into a compliant one. A single `cd` bypassed the guard, and the commit really landed. 0.4.0 blocked this, so it was a new regression that nullified the largest rejection bucket. Related: an invented `mkdir -p <root>/a/b` and a symlink aimed at a shared checkout both passed. Classification is now grounded in the filesystem — the two-segment path must really be a git worktree root, and no segment may be a symlink (lstat, not existsSync, which follows them). Also from review: - The legacy station-id lease layout is recognised from inside it, and git work there now warns instead of blocking. Those 73 worktrees are live; hard-blocking them on day one would leave agents able to edit but never land. Scoped, labelled, and removed once they are re-homed. - Segment validation no longer uses an allowlist that rejected legal directory names (long names, +, ~, spaces, non-ASCII); it is bounded by the filesystem limit and still refuses a leading . or - and control characters. - Reason strings no longer attribute the over-nesting rejection to an explicit rule-8 prohibition; it follows from the exact-path mandate. - Documented that the restored write carve-out covers linked worktrees only, so a standalone clone at a canonical path can commit but not be written to. Guard verdicts across the 764 real worktrees on the reference machine: 203 block -> allow, 73 allow -> warn, 488 block -> block, 0 allow -> block. Task: OPE61-00004
Third adversarial review round found two BLOCKERs in a02fbb1, both proven by landing real commits on a shared checkout through a hook that returned continue. 1. A forged `.git` FILE at the canonical depth was accepted. A `.git` file is two lines of text; pointing it at a shared checkout's `.git` grafts a second working tree onto that checkout, so git commit/push from the forged directory lands there — the exact outcome the symlink check was added to prevent, by an easier route needing no symlink. 0.4.0 blocked this path, so it was also a regression against the deployed baseline. 2. The legacy-lease tolerance did no filesystem check at all, so two directories named to match the pattern laundered a symlink into a warn-and-allow. Both had the same root cause: the classifier proved shape and symlink-freedom but never proved provenance. It now requires a `.git` file's `gitdir:` target to live under its repository's worktrees/ directory with a back-pointer resolving to that control file, and a `.git` directory not to be grafted on by a commondir — the same standard the write carve-out already applied. The legacy branch clears the same grounding, and covers both historical variants (checkout at the lease dir, or in a repo/ child). Also from review: - canonicalRepoIdentity() could return a worktree row from the repos index, whose name is a worktree name and whose default_branch is that worktree's branch. When the row resolves under the worktrees root, the repo name is taken from its first segment there and the branch is dropped as unrecoverable. - The repos lookup is now hard-bounded. runCommand's timeout kills the direct child but still awaits its pipes, which a forking CLI holds open indefinitely; the hook sits on the PreToolUse path, so the lookup races a ceiling and the hook exits once its verdict is written. - claimCommand() validates every interpolated value at the point of use, since a task id is unvalidated hook input and a repo name is remote-influenced. This is also where a repo slug is refused rather than silently trimmed to a wrong name. - Tests now build real git worktrees instead of hand-written .git files, and cover the repos-CLI resolution, the worktree-row correction, and the hang path — all of which previously survived mutation testing untested. Guard verdicts across the 764 real worktrees on the reference machine: 189 block -> allow, 70 allow -> warn, 502 block -> block, 3 allow -> block. The 3 are worktrees whose repository was pruned, where git itself refuses to operate. Across all 207 canonical-path checkouts the guard's verdict now agrees with git's own verdict 207/207. Task: OPE61-00004
…ct storage Final adversarial round found two more BLOCKERs, both proven by moving a victim repository's HEAD through a hook that reported the path compliant. 1. `process.exit(0)` after an async `process.stdout.write` truncated any verdict larger than the pipe buffer at 64 KiB, leaving unparseable JSON — the caller would see no decision at all. respond() now uses writeSync. Verified complete and parseable at 256 KiB, 1 MiB and 4 MiB. 2. The `.git` directory branch checked only for a `commondir` and a HEAD, so a directory with `objects` and `refs` symlinked into another repository was certified canonical, and commits landed on that repository's refs. It needs no write inside the victim's `.git`, which the previous comment wrongly claimed was required. Self-contained object and ref storage is now required. Also: the legacy tolerance is switchable via HASNA_HOOKS_LEGACY_WORKTREE_TOLERANCE=0 so it can be retired once the 70 affected worktrees are re-homed, and its remaining limitation is documented — it keys off the path name, so a worktree deliberately named to match also gets the warn tier. That is an opt-out from a guardrail, not a way past the provenance proof, which both tiers must clear. The objects/refs requirement was checked against every currently-accepted real worktree before landing: 0 newly rejected. Guard verdict still agrees with git's own verdict 210/210 across canonical-path checkouts. Gate: typecheck clean, 941 pass / 0 fail (3 consecutive runs), validate:package passes. Baseline origin/main is 907 tests, so this adds 34. Task: OPE61-00004
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
managedWorktreeInfo()enforced a 3-segment worktree layout, so every worktree at the canonical rule-8 path was classified UNMANAGED. This aligns it to the canonical 2-segment shape and grounds the classification in verified git provenance.Task: OPE61-00004
This was not dormant
The task was filed on the understanding that
worktree-guardis unregistered. It is not: bothhooks run worktree-guardandhooks run pre-bashare registered in~/.codewith/config.toml, and@hasna/hooks@0.4.0is the globally installed copy, so the 3-segment rule is live for Codewith agents today.The impact was also wider than the git block.
managedWorktreeInfo()feedsmanagedLeaseRoot()→classifyDangerousOperation(), whichpre-bashalso uses, so a canonical worktree was additionally denied the scoped~/.hasnawrite carve-out —Write/Edit/apply_patchinto it was blocked as a dangerous operation. Both reproduced against the installed 0.4.0.What the 3rd segment was
From the git history (
989c041"Add Codewith native hooks support", hardened ine4649b4), the shape was arepos worktrees claimlease layout:The guard was built around a repos CLI that would hand out per-machine, per-repo worktree leases. That verb never shipped —
repos --helphas no worktree verb today — and rule 8 later standardised on the flat 2-segment shape. The remediation string still recommendedrepos worktrees claim ... --mode required, a command that cannot be run.Canonical shape
Rule 8, as published by the
@hasna/identities0.4.4 global agent rules (the mandate appears in 4 distinct instruction sources there):Classification matrix
<repo>/<worktree>(real worktree root)<flat-worktree><flat-worktree>/<subdir><station-id>/<repo>/<worktree><repo>/<worktree>.git.gitfile pointing at a shared checkout.gitdir withcommondir, or symlinkedobjects/refs<station-id>/<slug>-<hex>/wt_<hex>(+repo/variant)Shape alone proves nothing, which drove most of the review findings.
<root>/<flat-worktree>/<subdir>is shape-identical to<root>/<repo>/<worktree>, so a singlecdlaundered a forbidden flat worktree into a compliant one. A symlink, a two-line forged.gitfile, or a.gitdirectory with symlinked object storage aimed a compliant-looking path at a shared checkout — andgit commit/git pushlanded there, which is precisely what rule 10 forbids. Classification now requires a real worktree root, no symlinked segment, and a.gitthat proves it owns its own history.Deprecated: the station-id lease layout
Not a compliant shape, and never reported as managed. Two scoped, temporary tolerances keep the 70 affected live worktrees usable while they are re-homed: git work there warns instead of blocking (rather than leaving agents able to edit but never land), and the layout keeps its
~/.hasnawrite carve-out. Both clear the same provenance proof as the canonical path. Retire withHASNA_HOOKS_LEGACY_WORKTREE_TOLERANCE=0.Known limitation, documented in the hook README: the tolerance keys off the path name, so a worktree deliberately named to match also gets the warn tier. That is an opt-out from a guardrail by a cooperating agent, not a way past the provenance proof.
Fleet impact
Measured against all 764 real worktrees under
~/.hasna/repos/worktreeson the reference machine, guard verdict before (0.4.0) → after:The 3 are worktrees whose repository has been pruned, where
git statusitself returnsfatal: not a git repository. No working setup is stranded.The strongest available check: across every depth-2 checkout containing a
.git, the guard's verdict agrees with git's own verdict 210/210 — everything it rejects is somewhere git itself refuses to operate, and everything it accepts is somewhere git works.A
[BREAKING]heads-up was posted toannouncementsbefore opening this PR.Also fixed
<repo-name>resolution. The remediation derived the repo name from the git remote basename, which names a different directory for 46 of 50 indexed repos —open-hooksisgithub.com/hasna/hooks— so the guard pointed agents at a path that does not exist, and disagreed with its own classifier inside one hook run. Now resolved with the exactrepos repo --remote <host/org/name> --jsonlookup rule 8 mandates, falling back to the local checkout. A row that is itself a worktree directory is corrected rather than trusted. Best-effort and hard-bounded: a missing, failing, or hanging repos CLI degrades to local information, and it is not invoked at all on the allowed path.process.stdout.writeis async on a pipe, so a large verdict could be truncated on exit, leaving unparseable JSON and no decision.+,~, spaces, non-ASCII); it is bounded by the filesystem limit and still refuses a leading.or-and control characters. Found via a real fleet false positive,connectors/_base.Gate
bun run typecheckclean ·bun test941 pass / 0 fail across 17 files, stable over 3 consecutive runs ·bun run validate:packagepasses. Baselineorigin/mainis 907 tests, so this adds 34.One MCP SSE test flaked once under heavy parallel load during review; it passes 3/3 standalone and 3/3 in full-suite runs, and this change touches nothing under
src/mcp.Version bumped 0.4.0 → 0.4.1 with a CHANGELOG entry. Publishing is the lander's step, not done here.
Review rounds
Four adversarial rounds, five blockers found and fixed:
@hasna/identities0.4.4. Found the<repo-name>blocker above.cdinto a flat worktree's subdirectory bypassed the guard, and the commit really landed..gitfile still grafted onto a shared checkout, and that the legacy tolerance did no filesystem check at all. Both proven by moving a victim repository's HEAD..gitdirectory graft via symlinkedobjects/refs. Both proven the same way.Each fix was mutation-tested: reverting the provenance check fails 11 tests, removing the legacy grounding fails 2. The
objects/refsrequirement was checked against every currently-accepted real worktree before landing — 0 newly rejected.Two findings were accepted rather than fixed, both documented: a canonical path holding a standalone clone can
git commitbut is still not writable by file tools, because the write carve-out's anti-forgery proof requires linked-worktree provenance (deliberately unchanged); and pre-existing shell bypasses (cd <shared> && git commit,GIT_DIR=…) behave identically on 0.4.0 and this branch, since the guard only inspects the pre-command cwd. Both are follow-up material.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.