feat(audit-git-checkouts): report audits as decisions and simplify removal gates #77
Loading…
Reference in a new issue
No description provided.
Delete branch "refactor/rebuild-audit-git-checkouts"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Agents now read a rendered markdown report (
render-audit-report.ts) instead of querying the JSON with jq. They reply with its summary and work through the remaining decisions in batches. In 25 past sessions, raw reports ran to 5–16K characters of internal outcome codes. The rejected alternative was keeping the jq workflow with rewritten prose; that leaves every agent carrying the report schema.The report is now schema 5:
gitPrune,summary, andremoval.preciousPathsare gone,removeMergedisremovalEnabled, and--remove-mergedis removed. Nothing outside the skill reads them.An adversarial debate rejected dropping the mount guard (Git deletes through a mount inside a worktree, reproduced) and merging the sparse-checkout outcome. It accepted capping the ignored-file scan at 4,096 entries and 2 GiB, not re-locking after a failed removal, and reusing the audit's merge proof unless HEAD,
origin/<default>, or replace refs changed.After a fast-forward, dependency changes are reported, not installed. The workflow and hook diffs are j4k-align regeneration.
Review
01M38ZNK9786DK6HMH6JHEC151— head11c8adf74a2af352a7e47067e73f589be44fd626Review — j4k-oss/agent-skills @
d1e95321d4Scope: diff against base tree
8825218a8defStatus: dispatched — coverage complete (3/3 slots terminal)
Facts: current review-wide projection
Computed under:
Findings (5)
medium — node-test workflow drops the step that installed jq, but audit-checkouts.test.mjs still shells out to jq on nearly every test
01M3906MVBQXRKC369HBXEQ9E1.forgejo/workflows/node-test.yml(snippet)medium — Mode table omits the submodule checkouts and unstaged Gitlink changes a default run writes
01M38ZXWEKVZQ1FMDZHPFPG0Y7skills/audit-git-checkouts/SKILL.md(snippet)low — Replacement timeout comment restates what timeout-minutes means and drops the one fact an editor needs
01M38ZXX9FRAJSN16S5XSG6AF3.forgejo/workflows/review.yml(snippet)low — The partial-walk activity test is skipped under root, so the find-failure branch of newest_directory_epoch may never execute in CI
01M38ZY3DD4E0V9A2C71D5NGX1skills/audit-git-checkouts/scripts/audit-checkouts.test.mjs(snippet)low — Renderer reports "origin fetch failed" for a default checkout whose fetch succeeded but whose server default-branch lookup failed
01M3907BR6AHXCVJN2G922EACNskills/audit-git-checkouts/scripts/render-audit-report.ts(snippet)Other claims
01M38ZXWW77WWY24J79KP8EKYNlow — Rewrapped concurrency comment leaves one 122-column line mid-paragraphCoverage
Coverage pass: 01M38ZNKBYFX4T077GYQ7Y1CJ6
Accounting: complete
Slot health: healthy
@ -88,3 +76,1 @@# signal handler, so a kill here writes nothing to the PR — the durable# fix is an internal deadline in the wrapper, not a larger number here.# Re-derive when the pin moves.# Limit runner occupancy even if retries or reconciliation are unfinished.low — The 130-minute timeout comment states the obvious purpose and drops the derivation a maintainer needs to change it
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M36XAYBSYDTD1A2XS6FWJA7Zof review01M36X1VFPSMHFG53KPRE556CNThis file is not editable here.
review.ymlis rendered by j4k-align fromtemplates/workflows/review-forgejo.yml.hbs, and the template's current text (align #270, 4ffe59e) is exactly this one-line comment; the header on line 2 marks direct edits as drift. The diff in this PR is the regeneration. If the derivation should come back, that is a change to the align template.superseded by review
01M37ANQNHYDFD5YK60V7FTRERfor head38f0009ff31a9421a94277906c2cad807c604891@ -78,3 +47,1 @@jq '.repositories[] | select(.fatalError != null or .fetch.ok != true or .defaultBranchResolution.remoteHeadOk != true or .forge == null or .defaultBranch == null or .worktreesError != null or .gitPrune.ok != true or .staleRegistrations.ok != true) | {checkout, fatalError, forgeError, defaultBranchError, defaultBranchResolution, fetch, worktreesError, gitPrune, staleRegistrations}' "$report"jq '.repositories[].worktrees[] | select(.removal.outcome != "removed") | select(.statusError != null or .outsideRoot or .defaultComparison == null or .defaultComparison.fresh != true) | {path, outsideRoot, statusError, defaultComparison}' "$report"```When the report does not answer a question, read the item's JSON record, where `<name>` is the path as `report.md` shows it, relative to the root: `jq --arg p <name> '.repositories[].worktrees[] | select(.path | endswith("/" + $p))' "$report/report.json"`. A failure row means coverage is incomplete for that checkout; never report it as clean.low — Documented jq lookup matches every worktree whose path ends in the display name, and never matches the root entry
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M36XAXYEJ6DH8NQM1DHNJ8BQof review01M36X1VFPSMHFG53KPRE556CNFixed in
38f0009: the lookup now joins on.root, with.naming a checkout at the root itself. Reproduced first on a driver run whose root is the checkout: the old query returned nothing, the new one returns the record.@ -786,0 +724,7 @@test("a replace ref forces a fresh containment proof at removal", (context) => {const fixture = createContainedLinkedWorktree(context);recordAuditContainment(fixture);addReplaceRef(fixture.repositoryPath);const result = runMaybeRemove(fixture);assert.equal(result.removal.containment.reused, false);low — Replace-ref containment tests assert only
reused === false, so a fresh proof that errors or fails would keep them greenlens
test-trimming· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M36XA1G5CCXA18S8BKMZDBVKof review01M36X1VFPSMHFG53KPRE556CNFixed in
4605fb7: both replace-ref tests now also assertremoval.outcome === "removed"andcontainment.verdict.verdict === "contained". The suite passes with them, which also settles your proof gap: the fresh proof under a replace ref does remove.@ -0,0 +372,4 @@return `${sections.join("\n\n")}\n`;}// import.meta.main needs Node 24.2+.low — Comment names a feature the code does not use, so the Node floor it implies conflicts with the skill's stated requirement
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M36XAYS7S1510B8BYCKG7SZXof review01M36X1VFPSMHFG53KPRE556CNFixed in
38f0009: the comment now says the path comparison is the workaround and why:import.meta.mainwould be simpler but needs Node 24.2+, and SKILL.md promises Node 24.@ -20,20 +20,6 @@ jobs:with:high — Node test workflow drops the jq install step while audit-checkouts.test.mjs still shells out to a jq-dependent driver
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M378XKFYG4AG7VH14W6XS5JWof review01M36X1VFPSMHFG53KPRE556CNThe Forge runner ships jq now. This head's own
Node tests / node:testrun (#1396, run 44843) executedaudit-checkouts.test.mjswith no install step and passed 54/54, shell-only cases included (a replace ref forces a fresh containment proof at removalis in its log). The bootstrap step was dropped in j4k-align (af91298, #233), andnode-test.ymlis rendered by align, so a step re-added here would be drift that the next regeneration removes.superseded by review
01M37ANQNHYDFD5YK60V7FTRERfor head38f0009ff31a9421a94277906c2cad807c604891@ -92,1 +42,3 @@| `judgment/precious-ignored-files` | Leave `preciousPaths` in place until the owner disposes of them or explicitly authorizes their disposition; no archival bypass. Rerun the driver afterward. || `judgment/containment-not-proven` | Investigate rungs 4–5 in [containment.md](containment.md); keep it without a concrete proof. || `judgment/worktree-locked` | The lock is under seven days old or future-dated. Wait for expiry, or unlock once the owner confirms the pin no longer applies. || `judgment/hidden-index-flags` | Have the owner clear hand-set flags, prove the revealed tree clean, then rerun. `git ls-files -v` shows assume-unchanged as lowercase letters; clear only those (`--no-assume-unchanged`) or hand-set `skip-worktree` outside a sparse cone. |low — "or hand-set
skip-worktreeoutside a sparse cone" reads as an instruction to set the flag, and the clearing flag is missinglens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M378V5KRT68C9G06X06B1FXQof review01M36X1VFPSMHFG53KPRE556CNFixed in
38f0009: the cell names bothls-files -vletters with their clearing switches (--no-assume-unchanged,--no-skip-worktree) and says to leave a sparse cone'sSentries alone, matching what the driver gate distinguishes.@ -990,3 +971,2 @@fiecho operational/removal-failedfiif [ "$expired_lock" = true ] \&& ! git -C "$common_git_dir" worktree unlock "$worktree_path" >>"$removal_output_path" 2>"$removal_error_path"; thenlow — An expired lock is unlocked and the worktree removed without re-reading the lock marker after the containment proof and ignored scan, so a pin renewed in that window is discarded
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M378Y8Y94SZJADKB1PMJ6YX2of review01M36X1VFPSMHFG53KPRE556CNThe recheck was removed on purpose. It guarded a window of seconds against a renewal no tooling performs:
git worktree lockrefuses a second lock (exit 128, reproduced), so renewing a pin means an owner unlocking an already expired lock while an audit is mid-run and re-locking before that same run reaches removal. Expiry overriding stale pins is the intended behavior, and the other gates still protect every commit and uncommitted file. Clarified in38f0009: the lock paragraph in removal-gates.md now states the lock is read once, at the gate, so the window is documented rather than implicit.superseded by review
01M37ANQNHYDFD5YK60V7FTRERfor head38f0009ff31a9421a94277906c2cad807c604891@ -34,4 +23,1 @@apt-get updateapt-get install -y --no-install-recommends "${missing[@]}"- name: Run testsmedium — node-test.yml drops the jq/lsof install step while the selected test files still shell out to jq and lsof, so the CI job fails on the minimal runner image the removed step existed for
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M37T47VRTF37D1B7YJDNB3J3of review01M37ANQNHYDFD5YK60V7FTRERSame evidence as the earlier jq finding on this file, now covering lsof too: this head's
Node tests / node:testrun (#1400, 45s) ran all four selected test files on the Forge runner without the install step and passed, includingmove-codex-session.test.tswith the lsof-backed assertions you name. The runner image ships jq and lsof. j4k-align dropped the bootstrap deliberately (af91298, #233) and renders this workflow, so a step re-added here is drift.superseded by review
01M38YP09SA7WCW3117P8RYJ0Pfor head7e38d16a8d171f2de20933ef4be013e160d245b5@ -88,4 +76,2 @@# signal handler, so a kill here writes nothing to the PR — the durable# fix is an internal deadline in the wrapper, not a larger number here.# Re-derive when the pin moves.# Limit runner occupancy even if retries or reconciliation are unfinished.timeout-minutes: 130low — The replacement comment on timeout-minutes: 130 no longer says where the number comes from or when it must be re-derived
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M37T2432J0XH89FYBNASYEZNof review01M37ANQNHYDFD5YK60V7FTRERSame as the earlier timeout-comment finding:
review.ymlis rendered from j4k-align'stemplates/workflows/review-forgejo.yml.hbs, whose current text (align #270, 4ffe59e) is exactly this line, and the header marks direct edits as drift. The PR diff is the regeneration; the derivation you want back is a change to the align template.superseded by review
01M38YP09SA7WCW3117P8RYJ0Pfor head7e38d16a8d171f2de20933ef4be013e160d245b5@ -11,2 +10,3 @@## Treat user work as untouchableUse the requested directory, or the current directory when none is given. Discover below that root without following symlinks; keep checkout inspection and file cleanup scoped to it. Shared repository maintenance is broader: registered worktrees outside the root may appear in the report, their metadata is read, and fetched mode prunes stale registrations repository-wide, including registrations pointing outside the root. Do not treat those worktrees as audited. If outside-root registration changes are forbidden, use `--no-fetch --no-remove` or scoped local queries. If even outside-root metadata reads are forbidden, use scoped local queries; the driver cannot enforce that boundary.Uncommitted files, commits no other ref holds, stashes, and ignored files the primary checkout does not also hold (apart from the machine-local footprints named under the modes) are user work. Discard, delete, or drop them only on the user's instruction for that item or class, after the proof the matching reference requires. Age, a vanished upstream branch, or divergence from the default branch never prove work disposable. Leave blocked content where it is: moving it into an archive, backup, or trash folder to clear a gate is deletion by another name. Never create branches or refs to "preserve" a checkout, and never edit `.gitmodules`, selectors, or Gitlinks under a `third-party/` path.medium — "Treat user work as untouchable" classes regenerable ignored output as user work, contradicting the default mode that deletes it without confirmation
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M37T1PFKXAMGAV09GF3C7N9Pof review01M37ANQNHYDFD5YK60V7FTRERFixed in
7e38d16: the user-work definition now excepts regenerable output as well as the two machine-local footprints, and points at the "Choose the mode" heading by name.@ -73,3 +45,1 @@has no partial-scan bypass; narrowing the audit root does not narrow a registeredworktree's scan. Ask the user how to handle the named content before changingit, then rerun the complete gate. A partial manual inspection is not a passed gate.Call a worktree safe to remove only when the driver removed it or you have freshly established every gate in [references/removal-gates.md](references/removal-gates.md). A cached or `--no-remove` run proves none of them; its Notes column lists the locks and primary-only ignored files it did observe.low — SKILL.md points at a "Notes column" for observed locks and precious files, but the renderer puts them inside the "Kept because" cell for merged worktrees
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M37T2ANRD07SM63ZMZMDD7KCof review01M37ANQNHYDFD5YK60V7FTRERFixed in
7e38d16. Reproduced on a rendered--no-removereport: the lock and.env.localnotes sit in the kept table's "Kept because" cell. The sentence now names that cell and the Notes column.@ -315,0 +320,4 @@[ "$files" -le 1000 ] || return 1file_epoch=$(stat_epoch "$file_path") || return 1[ "$file_epoch" -le "$newest" ] || newest=$file_epochdone < <(find "$directory_path" -type f -print0 2>/dev/null)low — newest_directory_epoch ignores find's exit status and stderr, so a partially unreadable changed directory yields a stale "newest" mtime with error: null instead of an unknown timestamp
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M37T5XJ4EKEP1E5BDBCB2SN7of review01M37ANQNHYDFD5YK60V7FTRERFixed in
814a1c2. Reproduced first: with achmod 000subdirectory,findexited 1 and the old function returned 0 with the epoch of the one file it could reach. The listing now goes through a scratch file, a non-zerofindfails the lookup, and the caller records the timestamp as unavailable; a regression test covers it (skipped as root, where the mode has no effect).@ -0,0 +361,4 @@}),);assert.ok(markdown.includes("- 1 checkout is current by cached refs."));assert.ok(!markdown.includes("- Updated"));low — Negative "- Updated" assertion in the regenerated-guidance test can never fail on a cached run
lens
test-trimming· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M37T060S31G02H4HHS07FZ3Vof review01M37ANQNHYDFD5YK60V7FTRERFixed in
1b8eb1f: the second assertion now checks that the "Needs your decision: not current" section is absent. Confirmed live by mutatingpendingCountto ignoredeferredOnly, which fails the test.@ -0,0 +105,4 @@function dirtyCount(status: Status): number {if (status === null) return 0;const tree = status.workingTree;return tree.staged + tree.unstaged + tree.untracked + tree.conflicted;high — render-audit-report.ts sums numeric workingTree.staged/unstaged counts the driver's status record never carries, so every dirty checkout renders as "NaN uncommitted files" and dirty-tree decisions are skipped
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M37T47F23Z67ZQG2S7J0NV8Vof review01M37ANQNHYDFD5YK60V7FTRERRefuted by repoq itself.
repoq status --jsonemits both the numeric counts and thefilesarrays: its schema (src/repoq.schemas.tslines 207-216 at f9fd778) declaresstaged,unstaged,untracked, andconflictedas numbers besidefiles. A real driver report from the fixture carries{"isClean":false,"staged":0,"unstaged":1,"untracked":0,"conflicted":0,"files":{...}}for a dirty checkout, and the renderer printsbehind; uncommitted changes (1 file) block the fast-forward, never NaN. The driver fakes omit the counts because the driver reads onlyfiles; the renderer fixtures carry the counts because that is the part it reads.superseded by review
01M38YP09SA7WCW3117P8RYJ0Pfor head7e38d16a8d171f2de20933ef4be013e160d245b5@ -35,1 +33,3 @@# double-post. Keying on github.ref alone would instead split a stacked pull# group deliberately — a manually dispatched review should supersede any live# review pass for the same pull request, and the wrapper's reconcilers list# before they create, so two overlapping passes double-post. Keying on github.ref alone would instead split a stacked pulllow — Reflowed concurrency comment leaves one 122-character line in a block wrapped at about 78 columns
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M38Z42385W427EA26KNEEJS3of review01M38YP09SA7WCW3117P8RYJ0PNot editable here:
review.ymlis rendered by j4k-align, and the 122-character line is line 35 of itstemplates/workflows/review-forgejo.yml.hbs(that template also carries 106- and 206-character lines). The header marks direct edits as drift; a re-wrap belongs in the align template.superseded by review
01M38ZNK9786DK6HMH6JHEC151for head11c8adf74a2af352a7e47067e73f589be44fd626@ -88,4 +76,2 @@# signal handler, so a kill here writes nothing to the PR — the durable# fix is an internal deadline in the wrapper, not a larger number here.# Re-derive when the pin moves.# Limit runner occupancy even if retries or reconciliation are unfinished.timeout-minutes: 130low — Replacement comment on timeout-minutes states what the key does and drops the only non-obvious fact, how 130 was derived
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M38Z42JXVD9NJEXGFEYCDZY8of review01M38YP09SA7WCW3117P8RYJ0PSame as the two earlier timeout-comment findings:
review.ymlis rendered from j4k-align'stemplates/workflows/review-forgejo.yml.hbs, whose current text (align #270, 4ffe59e) is exactly this line. The diff in this PR is the regeneration; restoring the derivation is a change to the align template.superseded by review
01M38ZNK9786DK6HMH6JHEC151for head11c8adf74a2af352a7e47067e73f589be44fd626@ -23,3 +22,1 @@Gated worktree removal is the default; no second permission request is needed. Local-branch deletion, remote-branch deletion, stash drops, and discarding dirty files require cleanup authorization covering that class. A request to clean local branches does not authorize remote deletions or stash drops. Report out-of-scope findings without deleting them.Treat dirty files, local commits, and stashes as user work. Age, default-branch divergence, and disappearance from a remote do not prove disposability. Delete only established disposable content, with one exception: the two machine-local footprints named in [references/removal-gates.md](references/removal-gates.md), which a removal-enabled run deletes without comparing the primary checkout. Leave unresolved files in their worktrees, never move them into archives or backups to bypass a gate. Git object pruning is report-only (`git prune --dry-run`), not an agent cleanup action.The default mode's updates and removals need no further confirmation. Removal proves containment against `origin/<default>` only: for a root holding a fork whose work integrates elsewhere, audit with `--no-remove`. The fetch prunes every `refs/remotes/origin/*` ref that no server branch supplies; for a repository with a custom `origin` fetch refspec, audit with `--no-fetch --no-remove` (see [references/checkout-updates.md](references/checkout-updates.md)). Removal also deletes regenerable ignored output (`node_modules/`, `dist/`, `.venv/`, …) and two machine-local footprints, `.vscode/` and `.ansible/vault_pass.txt`, without comparing them to the primary checkout. If the user wants those kept, use `--no-remove`. Stale-registration pruning is repository-wide: it can drop registrations of worktrees outside the root whose directories are gone.low — The mode-selection caveats are five separate trigger-to-flag rules buried in one paragraph after the mode table
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M38Z43ZXYXZRZPMZD44Q3NWTof review01M38YP09SA7WCW3117P8RYJ0PFixed in
11c8adf: the three stricter-mode conditions are bullets that lead with the trigger and end with the flags, and the stale-registration sentence now says it applies to every fetched mode (the driver only dry-runs the prune on--no-fetch).@ -78,3 +47,1 @@jq '.repositories[] | select(.fatalError != null or .fetch.ok != true or .defaultBranchResolution.remoteHeadOk != true or .forge == null or .defaultBranch == null or .worktreesError != null or .gitPrune.ok != true or .staleRegistrations.ok != true) | {checkout, fatalError, forgeError, defaultBranchError, defaultBranchResolution, fetch, worktreesError, gitPrune, staleRegistrations}' "$report"jq '.repositories[].worktrees[] | select(.removal.outcome != "removed") | select(.statusError != null or .outsideRoot or .defaultComparison == null or .defaultComparison.fresh != true) | {path, outsideRoot, statusError, defaultComparison}' "$report"```When the report does not answer a question, read the item's JSON record, where `<name>` is the path as `report.md` shows it, relative to the root (`.` for a checkout at the root itself): `jq --arg p <name> '.root as $r | .repositories[].worktrees[] | select(.path == (if $p == "." then $r else $r + "/" + $p end))' "$report/report.json"`. A failure row means coverage is incomplete for that checkout; never report it as clean.low — The 'read the item's JSON record' recipe only finds worktree records, but the items it is offered for include leftover directories and repository failures
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M38Z43GSN38ZK1T81ABB2EZ6of review01M38YP09SA7WCW3117P8RYJ0PFixed in
11c8adf: the recipe is now scoped to checkouts, with one clause for.directoryFindings[](samepathrule; a real report carries an absolutepaththere) and one for repository-level failures on the matching.repositories[]entry viacheckout.@ -129,3 +58,1 @@`latestChange` is an activity hint: newest HEAD commit or non-ignored changed-path mtime, with age relative to the report. Repository aggregates can include removed worktrees. Generated mtimes and age are not merge evidence. Never say “safe to delete” without the specific proof and applicable gates.In ad hoc shell work, do not name a variable `status`: it is read-only in zsh.After acting, rerun the driver over the smallest root holding the affected checkouts and their worktrees, which sit beside the primary checkout rather than inside it, and report the resulting state, not the commands that ran.low — Closing instruction asserts linked worktrees always sit beside the primary checkout, which is a layout convention, not a fact
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M38Z4325ZRJY471AF761C4GDof review01M38YP09SA7WCW3117P8RYJ0PFixed in
11c8adf: the instruction now asks for the smallest root holding the checkouts and every linked worktree registered to them, namesgit worktree listas the source, and states the sibling layout as the usual case rather than a fact.@ -0,0 +335,4 @@const directories = report.directoryFindings.filter((finding) => !finding.registeredWorktree).map((finding) => [display(finding.path), finding.outcome === "inspection-failed" ? `inspection failed: ${firstLine(finding.error)}` : finding.outcome]);low — Leftover-directory rows print the driver's outcome codes instead of plain wording in the user-facing report
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M38Z41HC8DJ2KNXRGAA7GGFCof review01M38YP09SA7WCW3117P8RYJ0PFixed in
e29292b: the Finding column now reads "empty" or "not a repository; contents not inspected" (theinspection failed: …form is unchanged). A real report'sold-notesrow renders with the new wording; the test assertion follows it.@ -34,6 +23,3 @@apt-get updateapt-get install -y --no-install-recommends "${missing[@]}"- name: Run testsshell: bashrun: |medium — node-test workflow drops the step that installed jq, but audit-checkouts.test.mjs still shells out to jq on nearly every test
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3906MVBQXRKC369HBXEQ9E1of review01M38ZNK9786DK6HMH6JHEC151Fourth time on this file, same evidence, now on three heads: the
Node tests / node:testruns #1396 (099f085), #1400 (38f0009) and #1407 (11c8adf) each ranaudit-checkouts.test.mjson the Forge runner with no install step and passed (54, 54 and 55 tests). The runner image ships jq; j4k-align dropped the bootstrap deliberately (af91298, #233) and renders this workflow, so a step re-added here is drift.@ -88,4 +76,2 @@# signal handler, so a kill here writes nothing to the PR — the durable# fix is an internal deadline in the wrapper, not a larger number here.# Re-derive when the pin moves.# Limit runner occupancy even if retries or reconciliation are unfinished.timeout-minutes: 130low — Replacement timeout comment restates what timeout-minutes means and drops the one fact an editor needs
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M38ZXX9FRAJSN16S5XSG6AF3of review01M38ZNK9786DK6HMH6JHEC151Same as the three earlier timeout-comment findings:
review.ymlis rendered from j4k-align'stemplates/workflows/review-forgejo.yml.hbs, whose current text (align #270, 4ffe59e) is exactly this line, and the header marks direct edits as drift. The derivation belongs in the align template, not in this repository.@ -17,3 +18,1 @@| Keep worktrees / do not remove them | `--no-remove` | Same fetched audit and updates; no worktree removal. || Read-only / no checkout changes | `--no-fetch --no-remove` | Cached diagnostic; no driver fetch, updates, or registration/worktree deletion. Comparisons are stale. || Cleanup | Default, plus the named cleanup classes | Interior deletions require the evidence and scope below. || Audit, clean up, "which are behind" | none | Fetch and prune `origin`; fast-forward eligible default checkouts; prune stale worktree registrations; remove proven-merged linked worktrees inside the root. |medium — Mode table omits the submodule checkouts and unstaged Gitlink changes a default run writes
lens
writing-quality· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M38ZXWEKVZQ1FMDZHPFPG0Y7of review01M38ZNK9786DK6HMH6JHEC151@ -1550,0 +1418,4 @@assert.equal(result.activityReview.error, null);});test("a changed directory that cannot be fully walked leaves the timestamp unknown", { skip: process.getuid?.() === 0 && "root reads directories regardless of mode" }, (context) => {low — The partial-walk activity test is skipped under root, so the find-failure branch of newest_directory_epoch may never execute in CI
lens
test-trimming· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M38ZY3DD4E0V9A2C71D5NGX1of review01M38ZNK9786DK6HMH6JHEC151@ -0,0 +249,4 @@}case "manual-review": {if (comparison !== null && !comparison.fresh) {if (fetched) return { aheadBehind: formatAheadBehind(comparison), why: "not compared: origin fetch failed" };low — Renderer reports "origin fetch failed" for a default checkout whose fetch succeeded but whose server default-branch lookup failed
lens
general-bug· armdefault· tally 1 valid / 0 invalid / 0 uncertainclaim
01M3907BR6AHXCVJN2G922EACNof review01M38ZNK9786DK6HMH6JHEC151Real, and deferred: this is review round 4 of this PR, where only severe bugs are fixed in place, and a table cell is a follow-up.
The driver's own
--helplists the two writes the table omits ("submodule sync after a fast-forward; branch/tag submodule selectors advanced outside third-party/ paths"). Exact fix, inskills/audit-git-checkouts/SKILL.md: extend the default row's "What changes" cell to "Fetch and pruneorigin; fast-forward eligible default checkouts and sync their submodules; in first-party default checkouts, move branch- or tag-tracked submodules to their selector and leave the Gitlink change unstaged; prune stale worktree registrations; remove proven-merged linked worktrees inside the root." No behavior changes; no commit in this PR.Agreed that the chmod test cannot run as root, and the Forge runner does run as root, so in CI the failure branch is untested. Deferred: this is review round 4 of this PR, and a test-only change is a follow-up.
Exact fix, in
skills/audit-git-checkouts/scripts/audit-checkouts.test.mjs: add a test beside the chmod one that reuses thecreateActivityFixturepattern with a shimmed helper,source "$1"; find() { return 1; }; annotate_activity_review …, on a fixture whose changed path is a directory, assertingoutcome === "unknown"and/timestamp unavailable: sample\.txt/. Keep the chmod test as the real-permissions case for non-root runs. No commit in this PR.The trace holds:
comparison_freshneeds both the fetch and thels-remote --symreflookup, and the manual-review branch ofprimaryDecisionnames only the fetch. Deferred: this is review round 4 of this PR, and a one-string wording change is a follow-up.Exact fix, in
skills/audit-git-checkouts/scripts/render-audit-report.ts,primaryDecision: change"not compared: origin fetch failed"to"not compared: origin fetch or default-branch lookup failed", the wordingstepFailurealready uses for linked worktrees, and update the assertion inrender-audit-report.test.ts("a fetched run never counts an uncompared checkout as current"). No commit in this PR.