Repository navigation
feat(history): audited history-repair re-anchor command; clear legacy drift so pm health is ok - #44
Merged
Merged
Conversation
…gacy drift so pm health is ok For months `pm health` and `pm validate --check-history-drift` returned ok:false because 31 legacy history streams carried stale hash chains (31 chain + 15 current-item mismatches) left over from old hand-edits, hierarchy repairs, and the 2026-05 redaction one-off scrub. `history-redact` only re-anchors a chain when a redaction match occurs, so cleanly-drifted streams could never be repaired. Add `pm history-repair <id>`, the missing peer to `history-redact`: - replays a stream from empty and recomputes every before/after hash - recovers legacy patch ops that no longer strictly apply (replace->add on first-write paths; skips unresolvable array-shape ops) - reconciles the latest after_hash with the on-disk item document - appends an auditable op=history_repair marker - never modifies item content; safe no-op on a clean stream - --dry-run/--author/--message/--force with the same ownership+lock governance as history-redact Wired into the CLI (register-mutation), MCP (history-repair action + server instructions), SDK contracts (PM_CORE_COMMAND_NAMES, PM_TOOL_ACTIONS, flag contracts, parameter keys), contracts.ts, shell completion (bash/zsh/fish), and docs (COMMANDS.md, AGENT_GUIDE.md). Consolidate the duplicated replay/patch mechanics shared by history, restore, history-redact, and history-repair into src/core/history/replay.ts (ReplayDocument, replayHash, normalizeReplayPatch*, toReplayDocument, tryApplyReplayPatch, verifyHistoryChain, reanchorHistoryEntries). Each command keeps a thin local error-formatting wrapper to preserve exact CLI error contracts. Addresses pm-pjs5. No behavior change. replayHash now falls back to a deterministic structural hash for legacy/partial replay states that the canonicalizer cannot normalize (e.g. a stream whose first entry is a partial update rather than a create); fully formed documents are unaffected. Ran history-repair across all 31 drifted streams: pm health is now ok:true and validate reports 0 drifted items. git diff confirms only history JSONL changed. Tests: tests/unit/history-replay.spec.ts (replay.ts at 100% coverage) and tests/unit/history-repair-command.spec.ts; release-readiness allowlist updated. Full coverage gate (1457 tests), static, docs-skills, check-secrets, and package-first dogfood all pass. Refs pm-85hm, pm-pjs5, pm-xk39. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reviewer's GuideIntroduces a new audited Sequence diagram for the new history-repair command flowsequenceDiagram
actor User
participant PmCli as pm_cli
participant HistoryRepair as runHistoryRepair
participant Replay as core_history_replay
participant Store as readLocatedItem
participant FS as writeFileAtomic
User ->> PmCli: history-repair <id> [CLI]
PmCli ->> HistoryRepair: runHistoryRepair(id, options, global)
HistoryRepair ->> HistoryRepair: resolvePmRoot / readSettings
HistoryRepair ->> HistoryRepair: resolveItemTypeRegistry
HistoryRepair ->> HistoryRepair: resolveHistorySubject
HistoryRepair ->> HistoryRepair: readHistoryEntries
HistoryRepair ->> Replay: verifyHistoryChain(entries)
Replay -->> HistoryRepair: { ok, errors }
HistoryRepair ->> Replay: reanchorHistoryEntries(entries)
Replay -->> HistoryRepair: { entries, finalDocument, details }
alt item exists
HistoryRepair ->> Store: readLocatedItem(located)
Store -->> HistoryRepair: { document }
HistoryRepair ->> Replay: replayHash(finalDocument)
HistoryRepair ->> Replay: replayHash(currentItemReplay)
end
HistoryRepair ->> Replay: verifyHistoryChain(rewrittenEntries)
Replay -->> HistoryRepair: { ok: true }
alt changed && !dryRun
HistoryRepair ->> HistoryRepair: acquireLock(...)
HistoryRepair ->> FS: writeFileAtomic(historyPath, historyEntriesToRaw(rewrittenEntries))
HistoryRepair ->> HistoryRepair: runActiveOnWriteHooks(...)
end
HistoryRepair -->> PmCli: HistoryRepairResult
PmCli -->> User: printResult(result)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR implements a new ChangesHistory-Repair Feature
🎯 4 (Complex) | ⏱️ ~60 minutes |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/cli/commands/history-repair.ts" line_range="94-96" />
<code_context>
+ let currentItemReplay: ReplayDocument | null = null;
+ let currentItemPath: string | null = subject.located?.itemPath ?? null;
+ let matchedChainBefore: boolean | null = null;
+ if (subject.located) {
+ const loaded = await readLocatedItem(subject.located, { schema: settings.schema });
+ currentItemReplay = {
+ metadata: loaded.document.metadata as unknown as Record<string, unknown>,
+ body: loaded.document.body,
</code_context>
<issue_to_address>
**suggestion:** Use shared `toReplayDocument` for item reconciliation to ensure hashing matches validate/health semantics.
Here you’re manually constructing `currentItemReplay` from `loaded.document` and then using `replayHash`. Please use the shared `toReplayDocument` helper from `core/history/replay.ts` so metadata canonicalization, ordering, and body defaults match the semantics used by validate/health, preventing subtle hash divergence and ensuring `after_hash` is consistent across the system.
Suggested implementation:
```typescript
import { replayHash, toReplayDocument } from "../../core/history/replay";
```
```typescript
if (subject.located) {
const loaded = await readLocatedItem(subject.located, { schema: settings.schema });
// Use shared toReplayDocument so hashing semantics are consistent with validate/health.
currentItemReplay = toReplayDocument(loaded.document, { schema: settings.schema });
const lastOriginalAfterHash = historyEntries[historyEntries.length - 1]?.after_hash;
matchedChainBefore = replayHash(currentItemReplay) === lastOriginalAfterHash;
}
```
If `toReplayDocument` has a different signature in your codebase (for example it doesn’t take the `{ schema }` option, or it expects a different options shape), adjust the second argument accordingly, or drop it entirely if not needed:
- Either `currentItemReplay = toReplayDocument(loaded.document);`
- Or `currentItemReplay = toReplayDocument(loaded.document, settings.schema);`
Also verify that the existing import path for `replayHash` is correct in your tree; the SEARCH/REPLACE import edit should match exactly whatever import line currently brings in `replayHash`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![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 history-repair command to fix drifted history chains and consolidates shared history replay logic into a new core module. A review comment identifies a redundant item document read in the history-repair command, suggesting an optimization to avoid unnecessary I/O.
|
|
||
| if (changed && !dryRun) { | ||
| if (subject.located) { | ||
| const loaded = await readLocatedItem(subject.located, { schema: settings.schema }); |
There was a problem hiding this comment.
Redundant read of the item document. The item was already read at line 95. Consider storing the result of the first read in a variable accessible to both blocks to avoid unnecessary I/O.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cli/commands/history-repair.ts`:
- Around line 81-107: The code reads history and the located item
(readHistoryEntries, verifyHistoryChain, reanchorHistoryEntries,
readLocatedItem, replayHash) and computes replay reconciliation
(currentItemReplay, matchedChainBefore, reconcileNeeded) before acquiring the
item lock, which risks reconciling/writing against stale state; move the calls
that load/compute the historyEntries, chainBefore, reanchor, and any
readLocatedItem + replayHash comparisons so they run while holding the subject's
item lock (i.e., acquire the same lock used for writes before calling
readHistoryEntries and readLocatedItem), then compute matchedChainBefore and
reconcileNeeded while locked and only release the lock after any
decision/read-modify-write sequence completes; keep checks that don't depend on
mutation outside the lock unchanged and ensure lock is released in all code
paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 6df4cc5f-8386-4c3e-b636-0154102d19f9
📒 Files selected for processing (56)
.agents/pm/chores/pm-pjs5.toon.agents/pm/history/pm-0c9q.jsonl.agents/pm/history/pm-0kjv.jsonl.agents/pm/history/pm-2eb3.jsonl.agents/pm/history/pm-3akm.jsonl.agents/pm/history/pm-54d.jsonl.agents/pm/history/pm-5rh2.jsonl.agents/pm/history/pm-85hm.jsonl.agents/pm/history/pm-8pzn.jsonl.agents/pm/history/pm-axl0.jsonl.agents/pm/history/pm-c3dx.jsonl.agents/pm/history/pm-cakn.jsonl.agents/pm/history/pm-e0b5.jsonl.agents/pm/history/pm-f3q4.jsonl.agents/pm/history/pm-fdla.jsonl.agents/pm/history/pm-g8gj.jsonl.agents/pm/history/pm-imob.jsonl.agents/pm/history/pm-jkip.jsonl.agents/pm/history/pm-lnq3.jsonl.agents/pm/history/pm-lty.jsonl.agents/pm/history/pm-m0fh.jsonl.agents/pm/history/pm-mb4n.jsonl.agents/pm/history/pm-mcli.jsonl.agents/pm/history/pm-nhka.jsonl.agents/pm/history/pm-pjs5.jsonl.agents/pm/history/pm-q4jp.jsonl.agents/pm/history/pm-rnpb.jsonl.agents/pm/history/pm-sh6o.jsonl.agents/pm/history/pm-tq1.jsonl.agents/pm/history/pm-u83w.jsonl.agents/pm/history/pm-w1r6.jsonl.agents/pm/history/pm-wo8.jsonl.agents/pm/history/pm-xk39.jsonl.agents/pm/history/pm-xk8b.jsonl.agents/pm/history/pm-zau.jsonl.agents/pm/issues/pm-85hm.toon.agents/pm/issues/pm-xk39.toon.agents/pm/plans/pm-c3dx.toonCHANGELOG.mddocs/AGENT_GUIDE.mddocs/COMMANDS.mdsrc/cli/commands/completion.tssrc/cli/commands/contracts.tssrc/cli/commands/history-redact.tssrc/cli/commands/history-repair.tssrc/cli/commands/history.tssrc/cli/commands/index.tssrc/cli/commands/restore.tssrc/cli/register-mutation.tssrc/core/history/replay.tssrc/mcp/server.tssrc/sdk/cli-contracts.tstests/integration/release-readiness-runtime.spec.tstests/unit/history-repair-command.spec.tstests/unit/history-replay.spec.tsvitest.config.ts
Comment on lines
+81
to
+107
| const historyEntries = await readHistoryEntries(subject.historyPath, subject.id); | ||
| if (historyEntries.length === 0) { | ||
| throw new PmCliError(`No history entries exist for ${subject.id}; nothing to repair.`, EXIT_CODE.USAGE); | ||
| } | ||
|
|
||
| const chainBefore = verifyHistoryChain(historyEntries); | ||
| const reanchor = reanchorHistoryEntries(historyEntries); | ||
|
|
||
| // Reconcile the replayed chain with the current on-disk item document so the | ||
| // latest after_hash matches what pm validate/health compute for the item. | ||
| let currentItemReplay: ReplayDocument | null = null; | ||
| let currentItemPath: string | null = subject.located?.itemPath ?? null; | ||
| let matchedChainBefore: boolean | null = null; | ||
| if (subject.located) { | ||
| const loaded = await readLocatedItem(subject.located, { schema: settings.schema }); | ||
| currentItemReplay = { | ||
| metadata: loaded.document.metadata as unknown as Record<string, unknown>, | ||
| body: loaded.document.body, | ||
| }; | ||
| const lastOriginalAfterHash = historyEntries[historyEntries.length - 1]?.after_hash; | ||
| matchedChainBefore = replayHash(currentItemReplay) === lastOriginalAfterHash; | ||
| } | ||
|
|
||
| const finalReplay = reanchor.finalDocument; | ||
| const reconcileNeeded = | ||
| currentItemReplay !== null && replayHash(finalReplay) !== replayHash(currentItemReplay); | ||
|
|
There was a problem hiding this comment.
Acquire the item lock before reading history/item state to prevent stale rewrites.
historyEntries, replay reconciliation input, and assignee checks are computed before locking. A concurrent mutation can land between those reads and the eventual write, which can overwrite newer history and reconcile against stale item state.
Suggested restructuring (move read/compute under lock)
- const historyEntries = await readHistoryEntries(subject.historyPath, subject.id);
- ...
- const releaseLock = await acquireLock(...);
+ const releaseLock = await acquireLock(...);
try {
+ const historyEntries = await readHistoryEntries(subject.historyPath, subject.id);
+ // recompute reanchor + reconcile using latest on-disk history/item snapshot
+ // run ownership check on the locked snapshot
+ // then write repaired history
} finally {
await releaseLock();
}Also applies to: 164-211
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/cli/commands/history-repair.ts` around lines 81 - 107, The code reads
history and the located item (readHistoryEntries, verifyHistoryChain,
reanchorHistoryEntries, readLocatedItem, replayHash) and computes replay
reconciliation (currentItemReplay, matchedChainBefore, reconcileNeeded) before
acquiring the item lock, which risks reconciling/writing against stale state;
move the calls that load/compute the historyEntries, chainBefore, reanchor, and
any readLocatedItem + replayHash comparisons so they run while holding the
subject's item lock (i.e., acquire the same lock used for writes before calling
readHistoryEntries and readLocatedItem), then compute matchedChainBefore and
reconcileNeeded while locked and only release the lock after any
decision/read-modify-write sequence completes; keep checks that don't depend on
mutation outside the lock unchanged and ensure lock is released in all code
paths.
…ed canonical replay form
- Hoist the located-item read into a single `readLocatedItem` call reused by both
reconciliation and the ownership/governance check (was read twice). Addresses
gemini-code-assist review.
- Build `currentItemReplay` via the shared `toReplayDocument` canonical replay
form instead of manually constructing `{metadata, body}`, so reconciliation
hashing matches the canonicalization semantics used by pm validate/health and
avoids subtle hash divergence. Addresses sourcery-ai review.
Verified: tsc clean, history replay/repair/activity suites pass (40 tests), and a
fresh temp-dir dogfood (inject chain+item drift -> repair -> verify ok, validate
drifted=0, item content preserved).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Owner
Author
|
Thanks for the reviews — addressed in 6d54705:
All CI gates (coverage 1457 tests + |
|
Tip For best results, initiate chat on the files or code changes.
Would you like me to open a follow-up GitHub issue to track the cross-command lock hardening (read inside the lock across ✏️ Learnings added
|
|
Thanks for the detailed follow-up and for addressing the feedback. Your reasoning regarding the locking model makes sense—maintaining consistency with |
unbraind
added a commit
that referenced
this pull request
May 22, 2026
…0 lock-hardening follow-up - pm-85hm closed: history-repair shipped + 31 legacy streams cleared (pm health ok) - pm-pjs5 closed: replay/patch mechanics single-sourced in core/history/replay.ts - pm-c3dx plan: 7/7 steps complete + design decision and pm-lty discovery recorded - pm-uer0 (new): cross-command read-then-lock hardening follow-up from CodeRabbit review Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced May 24, 2026
This was referenced Jun 11, 2026
This was referenced Jul 18, 2026
This was referenced Jul 23, 2026
This was referenced Aug 2, 2026
This was referenced Aug 9, 2026
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
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.
Why
For months
pm healthandpm validate --check-history-driftreturnedok: falsebecause 31 legacy history streams carried stale hash chains (31 chain + 15 current-item mismatches) left over from old hand-edits, hierarchy repairs, and the 2026-05 redaction one-off scrub.history-redactonly re-anchors a chain when a redaction match occurs, so cleanly-drifted streams could never be repaired. Every agent runningpm healthsaw a non-ok result and couldn't distinguish benign legacy drift from real corruption.What
Add
pm history-repair <id>— the missing peer tohistory-redact:before/afterhashreplace→addon first-write paths; skips unresolvable array-shape ops)after_hashwith the on-disk item document (so--verifyandvalidateboth pass)op=history_repairmarker--dry-run/--author/--message/--forcewith the same ownership + lock governance ashistory-redactWired into the CLI, MCP (
history-repairaction), SDK contracts,contracts, shell completion (bash/zsh/fish), and docs (COMMANDS.md,AGENT_GUIDE.md).Replay dedup (pm-pjs5)
Consolidated the duplicated replay/patch mechanics shared by
history,restore,history-redact, andhistory-repairintosrc/core/history/replay.ts. Each command keeps a thin local error-formatting wrapper to preserve exact CLI error contracts. No behavior change.replayHashnow falls back to a deterministic structural hash for legacy/partial replay states the canonicalizer can't normalize (e.g. a stream whose first entry is a partial update rather than acreate); fully formed documents are unaffected.Result
Ran
history-repairacross all 31 drifted streams:pm health→ok: true(all 9 checks; wasok: falsefor months)pm validate --check-history-drift→ 0 drifted itemsgit diffconfirms only history JSONL changed — no item content touchedVerification
replay.tsat 100% lines/branches/functions/statementstsc --noEmit, static-quality gate, docs-skills gate,check-secrets, and package-first dogfood all passtests/unit/history-replay.spec.ts,tests/unit/history-repair-command.spec.tsverify ok/drifted=0; idempotent no-op on clean streams; propernot_founderror pathRefs pm-85hm, pm-pjs5, pm-xk39.
🤖 Generated with Claude Code
Summary by Sourcery
Introduce an audited history-repair workflow and centralize history replay logic to clear legacy history drift without touching item content.
New Features:
Enhancements:
Documentation:
Tests: