fix(release): F42/F43 lifecycle-safety fixes + final-tree replay (PR #2556 successor) - #2562
Conversation
- .claude/workflows/pm-ledger-verify.js: reusable 3-lens adversarial verification of PM ledger edits (overclaim / consistency / contract), parameterized by wishDir + evidenceFile - FINALIZATION-VERIFICATION.md: evidence record of run 1 (12 findings, 4 must-fix, all corrected pre-push) + rerun instructions - REVIEW-DISPOSITION.md references the harness
- install.sh: foreign_lock_record_is_stale — reapable iff mtime aged outside ±600s AND owner dead (empty/unparseable record counts as dead when aged; symlinked guard never reapable; ps-based liveness treats EPERM/other-user as alive, matching TS lockHasLiveOwner) - recover_stale_lifecycle_lock reaps an abandoned guard on ln failure, then backs off; fresh/live-owner guards stay fail-closed; no rename/quarantine; acquire attempt budget 3 -> 4 - lock_record_is_stale: single-digit pids now parse; kill -0 -> ps -p - agent-sync.ts docblock (no behavior change): documents the one shared shell/TS guard protocol incl. the residual doubly-suspended race, pre-existing at TS parity - tests: +5 shell guard fixtures (abandoned guard proven blocking pre-fix), +2 first-coverage TS aged-guard fixtures, +1 cross-format record parity Independent review: SHIP (empirical pre/post-fix replay); 321 pass / 1 pre-existing skip / 0 fail across the three touched suites.
- durable batch journal schema v2: remove-disposition assets carry
per-kind discriminated identity (skill tree digests, workflow
target+sidecar digests+modes, link target); planned role agents carry
{digest, mode}; progress gains durable preserved[] receipts
- removal proceeds only when live identity equals recorded identity,
re-verified on the parked object (skills/workflows/roles) — the
path-authority TOCTOU between plan and retry is closed; identity-kind
mismatch refuses rather than degrading to unbound removal
- identity mismatch -> preserved receipt + prominent report; batch
clears when completed and preserved receipts cover every member; a
preserved member never regains removal authority
- authentic v1 journals are discarded and re-recorded as fresh v2 after
the confirmation preview (interrupted-member note surfaced); tampered
journals keep failing closed
- deliberate contract changes: modified-in-place recorded assets and
changed role agents are now preserved (batch clears) instead of
blocking completion
Independent review: SHIP; authorization traced end-to-end for all four
kinds; F43 scenario test proven failing pre-fix. Full aggregate:
1,420 pass / 1 pre-existing skip / 0 fail across 63 files.
Merges dev (incl. #2560 duplicate-hooks fix, v5.260712.1 manifests) and resolves the F05 resurrection: the external scheduled metrics-updater pushed .genie/agents/metrics-updater/{runs.jsonl,state.json} and the README METRICS block directly to dev on 2026-07-12 (d0ecc29), turning dev CI red against the merged retirement gate. This merge re-deletes the state and strips the block; the gate test passes again. The external routine itself must be disabled or re-scoped — a repo commit cannot prevent the next push.
Seven-lane replay at 8e147d8 returned 12 confirmed findings; all code-fixable ones close here: - lifecycle lock: shell recovers aged empty/unparseable lock records (was a permanent wedge for debris TS deliberately leaves); TS refuses symlinked/non-regular guard AND lock via lstat (docblock now honored, shell parity); no-trailing-newline records preserved, never mis-reaped as dead; +7 guard/lock matrix tests (future-mtime, symlink, no-newline, empty-lock cells) - hooks: Claude PreToolUse dispatch matcher gains NotebookEdit so the Omni approval gate actually fires for it; new manifest parity test derives the gated tool set from buildOmniRegistry and fails closed on drift (proven failing pre-fix); codex manifest verified no-gap - tests: persistChannel suite isolated to tmp GENIE_HOME (was mutating the live ~/.genie/config.json on every test run) - uninstall: collection scoped to planned paths (restrictToPaths) — O(N) tree digests instead of O(N^2); authorization, park-and-reverify, and kind-mismatch guards untouched (behavior-preserving) Combined independent re-review: SHIP. Quiet full aggregate: 1,432 pass / 1 pre-existing skip / 0 fail across 64 files.
# Conflicts: # README.md
…spositioned - F42/F43 rows flipped to FIXED with commits, SHIP reviews, empirical pre-fix reproductions, and accepted residuals - F05 corrected: gate test detects but cannot prevent direct-to-dev resurrection; external metrics-updater routine remains the open operational action (F44) - replay rows F44-F52: seven lanes at 8e147d8 (4 SHIP / 3 FIX-FIRST), 12 verified findings fixed in loop 1 (af9fcad, d1814e4) or dispositioned; targeted SHIP re-review covers the fixes, no full re-replay ran - validation snapshot at e3abf2b: 1,434 pass / 1 skip / 0 fail across 64 files; merge-tree clean vs dev e9059f4; live baseline 36/36 + 14/14 byte-identical - pm-ledger-verify run 2 recorded (14 findings, 10 must-fix — pass-count off-by-one and final-tree overstatement — corrected pre-commit); harness tolerates stringified args
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the pm-ledger-verify workflow to adversarially verify PM release-ledger edits, updates release-gate documentation for PR #2545, and enhances the durable uninstall batch system to schema v2 by binding removable assets to their planned physical identities (addressing F43). It also aligns the shell installer's lock recovery logic with TypeScript (addressing F42) and adds comprehensive test coverage. The review feedback highlights a bug in the ledger verification script where filtering results shifts array indices and misaligns lens keys, as well as a temporary file leak of guard_owner in install.sh on the successful lock recovery path.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| LENSES.map(l => () => agent(l.prompt, { label: `verify:${l.key}`, phase: 'Verify', schema: FINDINGS, effort: 'high' })) | ||
| ) | ||
|
|
||
| const all = results.filter(Boolean).flatMap((r, i) => r.findings.map(f => ({ ...f, lens: LENSES[i].key }))) |
There was a problem hiding this comment.
Filtering results before calling flatMap changes the array indices. As a result, the index i passed to the flatMap callback will correspond to the filtered array's index rather than the original results array's index, leading to incorrect lens key mapping (e.g., LENSES[i].key will point to the wrong lens if any preceding lens returned a falsy value). Preserving the original indices by mapping first or using flatMap with a ternary check resolves this issue.
const all = results.flatMap((r, i) => r ? r.findings.map(f => ({ ...f, lens: LENSES[i].key })) : [])| IFS= read -r current < "$LIFECYCLE_LOCK" || [ -n "$current" ] || current="" | ||
| if [[ "$current" == "$observed" ]]; then rm -f "$LIFECYCLE_LOCK"; fi | ||
| fi | ||
| if [[ -e "$guard" && -e "$guard_owner" && "$guard" -ef "$guard_owner" ]]; then rm -f "$guard"; fi |
There was a problem hiding this comment.
The temporary file guard_owner is created on line 180. While the guard link is unlinked on line 194, the original guard_owner file is never removed when the link operation succeeds, leading to leaked temporary files in the directory. Adding rm -f "$guard_owner" at the end of the function ensures proper cleanup.
| if [[ -e "$guard" && -e "$guard_owner" && "$guard" -ef "$guard_owner" ]]; then rm -f "$guard"; fi | |
| if [[ -e "$guard" && -e "$guard_owner" && "$guard" -ef "$guard_owner" ]]; then rm -f "$guard"; fi | |
| rm -f "$guard_owner" |
…d unmet) - Status DONE; all nine success criteria checked against consolidated ledger evidence, with a note that per-group and QA criteria remain the historical execution record - F01 CLOSED: PR automagik-dev#2562 CI passed 15 checks / 0 failures on its exact head bd8c612 under CI's pinned Bun 1.3.11 (repo declares >=1.3.10) - F02 recorded **NOT MET**, not waived: automagik-dev#2562 carries one COMMENTED review and no APPROVED review, and the merging identity is the one the commits were authored under, so no independent approval exists - F05/F44 closed by observation: the metrics-updater has committed nothing since 2026-07-14 and the maintainer's routine list is empty, but the owning account was never identified, so deletion is inferred - standing conditions kept explicit: untrusted hook hashes, upstream- blocked startup probe, F16-F18/F31 still blocking stable release, no package rebuilt at the successor head, accepted lock/uninstall residuals pm-ledger-verify run 3 recorded: 21 findings, 18 must-fix (wrong exact SHA and a self-merge presented as independent approval) — all corrected before this commit.
1 participant
PR #2556 successor — F42/F43 fixes, final-tree replay, verification harness
Successor to merged PR #2556 (the PR #2545 ultra release-gate remediation). Closes the two documented-open HIGH follow-ups, runs the seven-lane specialist replay that was the remaining honest gap in the gate table, and ships the PM ledger-verification harness. Does not authorize a stable release.
.genie/wishes/pr-2545-ultra-release-gate/WISH.md· Ledger:REVIEW-DISPOSITION.md· Verification record:FINALIZATION-VERIFICATION.md614219ee(aggregate/merge-sim/baseline evidence at code heade3abf2b5)What ships
09b368a5): abandoned.steallock-guard debris no longer permanently bricksgenie install/update. The shell now reaps a foreign guard only when aged (±600s) AND its owner is provably dead (empty/unparseable record = dead when aged; symlinked guard never; other-user EPERM = alive). Fresh/live guards stay fail-closed. Independent review: SHIP, with the permanent block reproduced pre-fix.8e147d87):genie uninstallretry can no longer delete files it never observed. Batch journal v2 binds per-kind physical identity at plan time; removal requires a live identity match, re-verified on the parked object; mismatches become durablepreservedreceipts (batch clears, authority never regained); authentic v1 journals are discarded and re-recorded post-preview. Independent review: SHIP, with the pre-fix path-authority deletion reproduced empirically. Deliberate contract change: modified-in-place assets and changed role agents are now preserved instead of blocking completion.8e147d87: architecture/security/code-quality/performance SHIP; repo-hygiene/QA/dx-docs FIX-FIRST. 12 adversarially-verified findings (ledger rows F44–F52), all code-fixable ones closed in fix loop 1 (d1814e40): shell recovery of empty lock debris, TS symlink refusal on guard+lock, no-newline record preservation, NotebookEdit added to the Claude approval matcher with a fail-closed parity test, persistChannel test isolation (was mutating the live~/.genie/config.json), O(N) uninstall digesting, +8 regression tests. A targeted independent SHIP re-review covers the fixes; no full re-replay ran after them.pm-ledger-verifyharness (.claude/workflows/pm-ledger-verify.js): 3-lens adversarial verification of PM ledger edits, now part of the repo. Its two recorded runs caught 4 and 10 must-fix ledger defects respectively — all corrected pre-commit (seeFINALIZATION-VERIFICATION.md).af9fcad1(dev independently re-retired in6495bde4).Evidence at the shipped head
bun run check: 1,434 pass / 1 pre-existing skip / 0 fail (1,435 ran) across 64 files, exit 0 (local Bun 1.3.9).git merge-tree --write-tree origin/dev HEAD: exit 0 (treecc6ea068) vsorigin/dev=e9059f41;git diff --checkclean.Still open (this PR does not close them)
stable-release-security-gatewish); stable promotion stays BLOCKED.Draft until CI and human review complete.