feat(sdk): ship history time travel, rich reads, and runtime integrity#549
Conversation
Promote audited history maintenance and verified replay into the public SDK, add immutable get --at reads, and expose bounded child and normalized schedule projections across CLI, SDK, and MCP. Add atomic bundle-generation manifests, torn-install recovery, extension copy containment, handled Sentry control-flow classification, and exact reliability-gate behavior. Ratchet coverage and docstring quality to 100%, preserve zero duplication, refresh contracts/docs/changelog, and close the nine linked pm items with packed-package and live telemetry evidence.
Keep immutable history reads strict while restore retains latest-at-or-before timestamp behavior and command-specific diagnostics. Avoid workspace corpus scans for normal built-in leaf reads while preserving automatic container/custom-type rollups and explicit children projection for every type. Record Greptile remediation, linked verification, and regenerated pm-changelog output.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @unbraind, your pull request is larger than the review limit of 150000 diff characters
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (81)
📝 WalkthroughWalkthroughThis PR promotes history maintenance and point-in-time reads into the SDK, adds bounded schedule and child projections, hardens CLI bundle integrity detection and extension copying, tightens quality gates, and expands release, integration, and regression coverage. ChangesHistory SDK and historical reads
CLI integrity and release hardening
Tracking and supporting records
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant SDK
participant History
participant Filesystem
User->>CLI: pm get <id> --at target
CLI->>SDK: getItemAt(id, target)
SDK->>Filesystem: read history stream
Filesystem-->>History: history entries
SDK->>History: resolve target and replay patches
History-->>SDK: verified historical document
SDK-->>CLI: reconstructed result metadata
CLI-->>User: JSON or Toon historical projection
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
|
@greptileai please review the full current head, including the local Greptile remediation commit. |
|
/gemini review |
|
@coderabbitai full review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 49 minutes. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR promotes history and richer read features into the CLI and SDK. The main changes are:
Confidence Score: 5/5This PR appears safe to merge based on the reviewed changed paths. The historical children projection issue is addressed by explicit validation, and the reviewed history replay, compaction, bundle integrity, extension containment, MCP, and runtime input paths have consistent guardrails. No files require special attention.
 T-Rex LogsWhat T-Rex did
Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant CLI as pm get / PmClient.get
participant History as getItemAt / readHistoryEntries
participant Replay as history projection replay
participant Projections as schedule/children/claim_state
User->>CLI: "get <id> [--at target] [--fields]"
alt historical read
CLI->>History: resolve item and history target
History->>Replay: replay entries through selected version/timestamp
Replay-->>History: canonical reconstructed document
History-->>CLI: document + as_of metadata
CLI->>CLI: reject tree/children historical workspace projections
else current read
CLI->>CLI: locate current item
CLI->>Projections: attach body/linked/claim_state/schedule
opt children projection
CLI->>Projections: scan bounded corpus and build rollup
end
CLI->>CLI: record context usage touch
end
CLI-->>User: projected GetResult
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant CLI as pm get / PmClient.get
participant History as getItemAt / readHistoryEntries
participant Replay as history projection replay
participant Projections as schedule/children/claim_state
User->>CLI: "get <id> [--at target] [--fields]"
alt historical read
CLI->>History: resolve item and history target
History->>Replay: replay entries through selected version/timestamp
Replay-->>History: canonical reconstructed document
History-->>CLI: document + as_of metadata
CLI->>CLI: reject tree/children historical workspace projections
else current read
CLI->>CLI: locate current item
CLI->>Projections: attach body/linked/claim_state/schedule
opt children projection
CLI->>Projections: scan bounded corpus and build rollup
end
CLI->>CLI: record context usage touch
end
CLI-->>User: projected GetResult
Reviews (11): Last reviewed commit: "fix(changelog): include the 2026.7.14 re..." | Re-trigger Greptile |
Greptile SummaryThis PR adds public history time-travel and richer read surfaces across the CLI and SDK. The main changes are:
Confidence Score: 4/5One contained issue needs to be fixed before merging. The main history replay, projection, bundle integrity, and extension containment changes look coherent. The new historical
T-Rex LogsWhat T-Rex did
Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant CLI as pm get
participant SDK as getItemAt
participant History as history JSONL
participant Replay as verified replay
participant Usage as context usage ledger
User->>CLI: "pm get <id> --at <target>"
CLI->>SDK: getItemAt(id, target)
SDK->>History: readHistoryEntries(historyPath, id)
History-->>SDK: history entries
SDK->>Replay: resolve target and hash-verified replay
Replay-->>SDK: canonical historical item
SDK-->>CLI: reconstructed item + as_of metadata
CLI-->>User: render result
CLI->>Usage: "recordContextUsageTouches(id, intent=get)"
Note over CLI,Usage: This write currently also runs for --at historical reads
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant CLI as pm get
participant SDK as getItemAt
participant History as history JSONL
participant Replay as verified replay
participant Usage as context usage ledger
User->>CLI: "pm get <id> --at <target>"
CLI->>SDK: getItemAt(id, target)
SDK->>History: readHistoryEntries(historyPath, id)
History-->>SDK: history entries
SDK->>Replay: resolve target and hash-verified replay
Replay-->>SDK: canonical historical item
SDK-->>CLI: reconstructed item + as_of metadata
CLI-->>User: render result
CLI->>Usage: "recordContextUsageTouches(id, intent=get)"
Note over CLI,Usage: This write currently also runs for --at historical reads
|
|
Acknowledged Gemini quota notice 4964492991: no review ran because the daily quota is exhausted; no code conclusion is inferred from this notice. |
|
Acknowledged CodeRabbit rate-limit notice 4964493463: the bot could not start a paid review in this round; the current head remains covered by local release gates and the other hosted checks. |
|
Acknowledged Gemini quota notice 4964495919: this duplicate command response confirms the same external quota blocker and contains no actionable code finding. |
|
Acknowledged CodeRabbit completion response 4964496233: the response reports completion under the rate limit and contains no inline or actionable findings. |
|
Acknowledged Codecov report 4964508279: all modified coverable lines are covered and all tests passed; this agrees with the local 100/100/100/100 release gate. |
|
Acknowledged Greptile finding 4964511738: the mutation-free historical-read issue is valid. |
|
Acknowledged Greptile finding 4964512082: this duplicate exact-head report identifies the same valid P1. The focused regression and implementation fix are complete locally and are passing ESLint plus strict static quality; full proof will run before the update is pushed. |
|
Acknowledged Sourcery review 4690177327: the PR intentionally exceeds the reviewer size cap because this delivery consolidates nine closely related SDK/CLI items on one verification surface; Sourcery provided no code finding. |
Skip derived context-usage ledger writes for reconstructed pm get --at reads while retaining feedback for current reads. Add byte-for-byte ledger immutability coverage and record the hosted Greptile remediation with linked 100/100/100/100 verification.
Use the shared EXIT_CODE.USAGE contract for invalid numeric and string runtime inputs, add direct SDK regression coverage, record the Greptile remediation in pm-4a7m, and regenerate the changelog with pm-changelog 2026.7.14.
Align history-compact scope-conflict failures with EXIT_CODE.USAGE, assert the MCP message and exit-code contract, record the exact-head review remediation, and retain pm-changelog 2026.7.14-1 metadata.
Correct the live PM-CLI-16 state after tracing a new event to an external media-server consumer still on 2026.7.13, record the resolved group and clean reliability gate, and regenerate the package-owned changelog.
Replace the absolute external-consumer path with host-neutral evidence through pm comments and audited history redaction, preserve hash-chain validity, and record the passing secret and live reliability gates.
|
@greptileai Please re-review the complete current head 7261948. Prior P1 historical-read mutation and local SDK usage-error findings are fixed; please report any remaining actionable finding. |
|
/gemini review |
|
@coderabbitai full review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
@coderabbitai full review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 10 minutes. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Return an explicit usage error when point-in-time reads request workspace-level children, cover the regression, record Windows nightly provenance in the existing canonical items, and refresh the package-owned changelog.
|
@greptileai please review the full current head dcc7780, including the historical-children remediation. |
|
/gemini review |
|
@coderabbitai full review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 59 minutes. |
Regenerate the package-owned changelog after merging the 2026.7.14 release commit so pull-request merge refs and clean checkouts produce identical output.
|
Acknowledged current-head bot status: Gemini is quota-blocked, while CodeRabbit accepted the full-review trigger. The only actionable Greptile thread was fixed, tested, replied to inline, and resolved; the branch has now also merged the 2026.7.14 release head to eliminate merge-ref changelog drift. |
|
@greptileai please review the full current head 619f337 after the release-head changelog reconciliation. |
|
/gemini review |
|
@coderabbitai full review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 53 minutes. |
|
Acknowledged the final bot-trigger responses: Gemini remains externally quota-blocked and CodeRabbit accepted the current-head full review. No new actionable thread has been posted; all existing inline findings are answered and resolved. |
Regenerate with the newly fetched v2026.7.14 tag so local, pull-request merge-ref, and CI changelog output remain byte-identical.
|
@greptileai please review the full current head cdcb3bb, including the tagged-release changelog reconciliation. |
|
/gemini review |
|
@coderabbitai full review |
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 50 minutes. |
|
Final review closeout: every bot comment/review node has been up-voted, the actionable Greptile P1 was answered inline and resolved, Greptile is green, all GitHub/security/coverage gates are green, and CodeRabbit has no finding but remains externally rate-limited beyond the 20-minute review window. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 `@docs/SDK.md`:
- Around line 169-172: Remove the inline “Tracked:” banner from the SDK
documentation and use the repository’s pm docs collection mechanism to associate
these documentation changes with the active pm item. Preserve the surrounding
documentation content and existing link conventions.
In `@src/cli/bundle-integrity.ts`:
- Around line 91-122: Update inspectCurrentBundleIntegrity so each manifest file
is read through one guarded fs.readFileSync operation instead of a separate
existsSync check followed by an unguarded read. Catch a read failure and return
"file_missing", while preserving the existing path validation, hash comparison,
and diagnostic results for successfully read files.
In `@src/sdk/history-compact.ts`:
- Around line 792-832: Update runHistoryCompactBulk to validate options.ids,
options.scope, and options.allOver with the shared assertHistoryCompactTarget
before determining mode or selecting targets. Preserve the existing selection
behavior for valid inputs, while rejecting conflicting selectors instead of
silently prioritizing ids.
In `@src/sdk/history-redact.ts`:
- Around line 269-294: Simplify applyLiteralRule by removing the manual indexOf
counting loop and derive replacements from the split result used for
replacement. Preserve the empty-literal guard and unchanged return when no
matches exist, while ensuring the replacement still applies all literal
occurrences.
- Around line 296-310: Harden applyRegexRule so SDK-supplied RegexRule patterns
cannot monopolize execution through catastrophic backtracking. Add an
appropriate complexity safeguard—such as validating regex complexity, limiting
the value length, or isolating execution with a timeout—before running both
value.matchAll and value.replace, while preserving normal replacement counts and
returned values for safe inputs.
- Around line 513-547: Update resolveHistoryRedactNextItem to validate
canonical.metadata.type against the original subject’s type before calling
getItemPath or serializing the rewritten document. Reject type changes with the
existing PmCliError/EXIT_CODE.USAGE pattern, while preserving the current
behavior for unchanged types and metadata.id validation.
In `@tests/unit/extensions/extension-copy-safety.spec.ts`:
- Around line 34-52: Update the symlink setup in the test “stages a symlinked
source whose real install destination is nested inside it” to remain
cross-platform: use a Windows-compatible junction when running on win32, while
preserving the directory symlink on other platforms, or skip the case on win32.
Keep the existing copyExtensionDirectoryWithoutSelfNesting assertions unchanged.
In `@tests/unit/sdk/history-read-primitives.spec.ts`:
- Around line 229-233: Remove the redundant array spread in the sort comparison
within the rollup sample assertion, sorting the fresh array returned by
rollup.sample.map((row) => row.id) directly while preserving the existing
localeCompare ordering and expectation.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 6405437b-3708-4498-8ae4-ec9aeb655e19
📒 Files selected for processing (81)
.agents/pm/chores/pm-fb3i.toon.agents/pm/extensions/.managed-extensions.json.agents/pm/features/pm-4a7m.toon.agents/pm/features/pm-hib1.toon.agents/pm/history/pm-4a7m.jsonl.agents/pm/history/pm-7071.jsonl.agents/pm/history/pm-8myl.jsonl.agents/pm/history/pm-b24b.jsonl.agents/pm/history/pm-f38n.jsonl.agents/pm/history/pm-fb3i.jsonl.agents/pm/history/pm-hib1.jsonl.agents/pm/history/pm-kl19.jsonl.agents/pm/history/pm-ksv2.jsonl.agents/pm/history/pm-u5zr.jsonl.agents/pm/history/pm-wfvq.jsonl.agents/pm/history/pm-x1g5.jsonl.agents/pm/history/pm-y4z5.jsonl.agents/pm/issues/pm-7071.toon.agents/pm/issues/pm-8myl.toon.agents/pm/issues/pm-b24b.toon.agents/pm/issues/pm-f38n.toon.agents/pm/issues/pm-kl19.toon.agents/pm/issues/pm-ksv2.toon.agents/pm/issues/pm-u5zr.toon.agents/pm/issues/pm-wfvq.toon.agents/pm/issues/pm-x1g5.toon.agents/pm/issues/pm-y4z5.toonCHANGELOG.mddocs/COMMANDS.mddocs/RELEASING.mddocs/SDK.mdeslint-suppressions.jsonpackage.jsonpackages/pm-governance-audit/extensions/governance-audit/runtime-utils.tspackages/pm-governance-audit/extensions/governance-audit/sdk.tsscripts/bundle-cli.mjsscripts/release/sdk-import-boundary-baseline.jsonscripts/release/sentry-telemetry-gate.mjsscripts/release/static-quality-gate.mjssrc/cli.tssrc/cli/bundle-integrity.tssrc/cli/commands/create.tssrc/cli/commands/extension.tssrc/cli/commands/get.tssrc/cli/commands/history-compact.tssrc/cli/commands/history-redact.tssrc/cli/commands/history-repair.tssrc/cli/commands/history.tssrc/cli/commands/restore.tssrc/cli/help-content.tssrc/cli/register-list-query.tssrc/core/history/projection.tssrc/core/history/read.tssrc/core/sentry/helpers.tssrc/core/shared/errors.tssrc/mcp/tool-definitions.tssrc/sdk/cli-contracts/flag-contracts.tssrc/sdk/history-compact.tssrc/sdk/history-mcp.tssrc/sdk/history-read.tssrc/sdk/history-redact.tssrc/sdk/history-repair.tssrc/sdk/index.tssrc/sdk/item-children.tssrc/sdk/item-schedule.tssrc/sdk/runtime-input.tssrc/sdk/runtime.tstests/fixtures/contracts/full.jsontests/integration/release-automation-contract.spec.tstests/integration/release-readiness-runtime.spec.tstests/unit/cli/cli-bundle-integrity.spec.tstests/unit/cli/register-list-query.spec.tstests/unit/commands/get-append-command.spec.tstests/unit/commands/history-activity-command.spec.tstests/unit/commands/history-repair-command.spec.tstests/unit/core/sentry/sentry-control-flow.spec.tstests/unit/extensions/extension-copy-safety.spec.tstests/unit/mcp/mcp-server-branch-residual.spec.tstests/unit/scripts/bundle-cli.spec.tstests/unit/scripts/release/sentry-telemetry-gate.spec.tstests/unit/sdk/history-read-primitives.spec.ts
| Tracked: [pm-4a7m](../.agents/pm/features/pm-4a7m.toon), | ||
| [pm-hib1](../.agents/pm/features/pm-hib1.toon), | ||
| [pm-y4z5](../.agents/pm/issues/pm-y4z5.toon), and | ||
| [pm-x1g5](../.agents/pm/issues/pm-x1g5.toon). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Inline "Tracked:" banner may conflict with this file's established linking convention.
Per prior guidance specific to this file, tracker linkage should go through the pm docs collection mechanism rather than an inline banner in the document body. As per coding guidelines, "Link documentation changes back to the active pm item with pm docs."
🤖 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 `@docs/SDK.md` around lines 169 - 172, Remove the inline “Tracked:” banner from
the SDK documentation and use the repository’s pm docs collection mechanism to
associate these documentation changes with the active pm item. Preserve the
surrounding documentation content and existing link conventions.
Sources: Coding guidelines, Learnings
| function inspectCurrentBundleIntegrity( | ||
| initialSnapshot: CliBundleManifestSnapshot, | ||
| cliEntrypointPath: string, | ||
| ): CliBundleIntegrityDiagnostic["reason"] | undefined { | ||
| const currentSnapshot = readCliBundleManifestSnapshot(cliEntrypointPath); | ||
| if (!currentSnapshot) { | ||
| return "manifest_missing"; | ||
| } | ||
| if ( | ||
| currentSnapshot.manifest.generation !== | ||
| initialSnapshot.manifest.generation | ||
| ) { | ||
| return "generation_changed"; | ||
| } | ||
| const bundleRoot = path.dirname(currentSnapshot.manifest_path); | ||
| for (const entry of currentSnapshot.manifest.files) { | ||
| const filePath = path.resolve(bundleRoot, entry.path); | ||
| if (!filePath.startsWith(`${bundleRoot}${path.sep}`)) { | ||
| continue; | ||
| } | ||
| if (!fs.existsSync(filePath)) { | ||
| return "file_missing"; | ||
| } | ||
| const digest = createHash("sha256") | ||
| .update(fs.readFileSync(filePath)) | ||
| .digest("hex"); | ||
| if (digest !== entry.sha256) { | ||
| return "hash_mismatch"; | ||
| } | ||
| } | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unguarded file read can mask the genuine error the diagnostic exists to preserve.
fs.existsSync followed by fs.readFileSync is a TOCTOU: exactly during the "concurrent rebuild" scenario this module targets, the file can disappear between the check and the read, causing readFileSync to throw. That exception isn't caught anywhere in diagnoseCliBundleIntegrityFailure or in runCliWithBundleIntegrity's catch block, so it propagates out and replaces the original, more informative startup error instead of being treated as an inconclusive diagnosis.
🛡️ Proposed fix: collapse the check+read into one guarded read
const bundleRoot = path.dirname(currentSnapshot.manifest_path);
for (const entry of currentSnapshot.manifest.files) {
const filePath = path.resolve(bundleRoot, entry.path);
if (!filePath.startsWith(`${bundleRoot}${path.sep}`)) {
continue;
}
- if (!fs.existsSync(filePath)) {
- return "file_missing";
- }
- const digest = createHash("sha256")
- .update(fs.readFileSync(filePath))
- .digest("hex");
- if (digest !== entry.sha256) {
- return "hash_mismatch";
- }
+ let fileContents: Buffer;
+ try {
+ fileContents = fs.readFileSync(filePath);
+ } catch {
+ return "file_missing";
+ }
+ const digest = createHash("sha256").update(fileContents).digest("hex");
+ if (digest !== entry.sha256) {
+ return "hash_mismatch";
+ }
}This also removes the race outright rather than narrowing the window, and stays compatible with existing test expectations (file_missing is still returned when the output is removed).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function inspectCurrentBundleIntegrity( | |
| initialSnapshot: CliBundleManifestSnapshot, | |
| cliEntrypointPath: string, | |
| ): CliBundleIntegrityDiagnostic["reason"] | undefined { | |
| const currentSnapshot = readCliBundleManifestSnapshot(cliEntrypointPath); | |
| if (!currentSnapshot) { | |
| return "manifest_missing"; | |
| } | |
| if ( | |
| currentSnapshot.manifest.generation !== | |
| initialSnapshot.manifest.generation | |
| ) { | |
| return "generation_changed"; | |
| } | |
| const bundleRoot = path.dirname(currentSnapshot.manifest_path); | |
| for (const entry of currentSnapshot.manifest.files) { | |
| const filePath = path.resolve(bundleRoot, entry.path); | |
| if (!filePath.startsWith(`${bundleRoot}${path.sep}`)) { | |
| continue; | |
| } | |
| if (!fs.existsSync(filePath)) { | |
| return "file_missing"; | |
| } | |
| const digest = createHash("sha256") | |
| .update(fs.readFileSync(filePath)) | |
| .digest("hex"); | |
| if (digest !== entry.sha256) { | |
| return "hash_mismatch"; | |
| } | |
| } | |
| return undefined; | |
| } | |
| function inspectCurrentBundleIntegrity( | |
| initialSnapshot: CliBundleManifestSnapshot, | |
| cliEntrypointPath: string, | |
| ): CliBundleIntegrityDiagnostic["reason"] | undefined { | |
| const currentSnapshot = readCliBundleManifestSnapshot(cliEntrypointPath); | |
| if (!currentSnapshot) { | |
| return "manifest_missing"; | |
| } | |
| if ( | |
| currentSnapshot.manifest.generation !== | |
| initialSnapshot.manifest.generation | |
| ) { | |
| return "generation_changed"; | |
| } | |
| const bundleRoot = path.dirname(currentSnapshot.manifest_path); | |
| for (const entry of currentSnapshot.manifest.files) { | |
| const filePath = path.resolve(bundleRoot, entry.path); | |
| if (!filePath.startsWith(`${bundleRoot}${path.sep}`)) { | |
| continue; | |
| } | |
| let fileContents: Buffer; | |
| try { | |
| fileContents = fs.readFileSync(filePath); | |
| } catch { | |
| return "file_missing"; | |
| } | |
| const digest = createHash("sha256").update(fileContents).digest("hex"); | |
| if (digest !== entry.sha256) { | |
| return "hash_mismatch"; | |
| } | |
| } | |
| return undefined; | |
| } |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 114-114: Filesystem path is not a string literal; a request-/variable-derived path can enable path traversal. Validate and normalize the path before use.
Context: fs.readFileSync(filePath)
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(detect-non-literal-fs-filename-typescript)
🤖 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/bundle-integrity.ts` around lines 91 - 122, Update
inspectCurrentBundleIntegrity so each manifest file is read through one guarded
fs.readFileSync operation instead of a separate existsSync check followed by an
unguarded read. Catch a read failure and return "file_missing", while preserving
the existing path validation, hash comparison, and diagnostic results for
successfully read files.
| function applyLiteralRule( | ||
| value: string, | ||
| literal: string, | ||
| replacement: string, | ||
| ): { value: string; replacements: number } { | ||
| if (literal.length === 0) { | ||
| return { value, replacements: 0 }; | ||
| } | ||
| let cursor = 0; | ||
| let replacements = 0; | ||
| while (cursor <= value.length) { | ||
| const index = value.indexOf(literal, cursor); | ||
| if (index === -1) { | ||
| break; | ||
| } | ||
| replacements += 1; | ||
| cursor = index + Math.max(1, literal.length); | ||
| } | ||
| if (replacements === 0) { | ||
| return { value, replacements: 0 }; | ||
| } | ||
| return { | ||
| value: value.split(literal).join(replacement), | ||
| replacements, | ||
| }; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Simplify applyLiteralRule: drop the redundant manual counting pass.
The indexOf loop only re-derives what value.split(literal).length - 1 already gives you, then the code still does a separate split/join for the actual replacement — two full scans for one answer.
♻️ Proposed simplification
function applyLiteralRule(
value: string,
literal: string,
replacement: string,
): { value: string; replacements: number } {
if (literal.length === 0) {
return { value, replacements: 0 };
}
- let cursor = 0;
- let replacements = 0;
- while (cursor <= value.length) {
- const index = value.indexOf(literal, cursor);
- if (index === -1) {
- break;
- }
- replacements += 1;
- cursor = index + Math.max(1, literal.length);
- }
- if (replacements === 0) {
- return { value, replacements: 0 };
- }
- return {
- value: value.split(literal).join(replacement),
- replacements,
- };
+ const parts = value.split(literal);
+ if (parts.length === 1) {
+ return { value, replacements: 0 };
+ }
+ return { value: parts.join(replacement), replacements: parts.length - 1 };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function applyLiteralRule( | |
| value: string, | |
| literal: string, | |
| replacement: string, | |
| ): { value: string; replacements: number } { | |
| if (literal.length === 0) { | |
| return { value, replacements: 0 }; | |
| } | |
| let cursor = 0; | |
| let replacements = 0; | |
| while (cursor <= value.length) { | |
| const index = value.indexOf(literal, cursor); | |
| if (index === -1) { | |
| break; | |
| } | |
| replacements += 1; | |
| cursor = index + Math.max(1, literal.length); | |
| } | |
| if (replacements === 0) { | |
| return { value, replacements: 0 }; | |
| } | |
| return { | |
| value: value.split(literal).join(replacement), | |
| replacements, | |
| }; | |
| } | |
| function applyLiteralRule( | |
| value: string, | |
| literal: string, | |
| replacement: string, | |
| ): { value: string; replacements: number } { | |
| if (literal.length === 0) { | |
| return { value, replacements: 0 }; | |
| } | |
| const parts = value.split(literal); | |
| if (parts.length === 1) { | |
| return { value, replacements: 0 }; | |
| } | |
| return { value: parts.join(replacement), replacements: parts.length - 1 }; | |
| } |
🤖 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/sdk/history-redact.ts` around lines 269 - 294, Simplify applyLiteralRule
by removing the manual indexOf counting loop and derive replacements from the
split result used for replacement. Preserve the empty-literal guard and
unchanged return when no matches exist, while ensuring the replacement still
applies all literal occurrences.
| function applyRegexRule( | ||
| value: string, | ||
| rule: RegexRule, | ||
| replacement: string, | ||
| ): { value: string; replacements: number } { | ||
| const regex = new RegExp(rule.source, rule.flags); | ||
| const matches = [...value.matchAll(regex)]; | ||
| if (matches.length === 0) { | ||
| return { value, replacements: 0 }; | ||
| } | ||
| return { | ||
| value: value.replace(regex, replacement), | ||
| replacements: matches.length, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file around the cited lines and the parser site.
wc -l src/sdk/history-redact.ts
sed -n '180,340p' src/sdk/history-redact.ts
# Find tests or helpers related to regex redaction rules.
rg -n "parseRegexRule|applyRegexRule|safe-regex|recheck|ReDoS|history-redact|regex rule" -S src test tests . || trueRepository: unbraind/pm-cli
Length of output: 36766
Guard history-redact regexes against catastrophic backtracking.
parseRegexRule only checks that the pattern compiles, and applyRegexRule then runs it over every history string with no complexity bound or timeout. Since this is exposed through the SDK, a crafted pattern can still hang a long-lived caller during redaction; add a regex-complexity check, input-size cap, or worker/timeout isolation.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 300-300: Do not use variable for regular expressions
Context: new RegExp(rule.source, rule.flags)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
[warning] 300-300: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(rule.source, rule.flags)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
🤖 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/sdk/history-redact.ts` around lines 296 - 310, Harden applyRegexRule so
SDK-supplied RegexRule patterns cannot monopolize execution through catastrophic
backtracking. Add an appropriate complexity safeguard—such as validating regex
complexity, limiting the value length, or isolating execution with a
timeout—before running both value.matchAll and value.replace, while preserving
normal replacement counts and returned values for safe inputs.
Source: Linters/SAST tools
| function resolveHistoryRedactNextItem(params: { | ||
| pmRoot: string; | ||
| subject: HistorySubject; | ||
| settings: Awaited<ReturnType<typeof readSettings>>; | ||
| typeToFolder: Record<string, string>; | ||
| finalDocument: ReplayDocument; | ||
| }): HistoryRedactNextItem { | ||
| if (!hasItemMetadata(params.finalDocument)) { | ||
| return { raw: null, path: null, document: null }; | ||
| } | ||
| const canonical = canonicalDocument( | ||
| replayToItemDocument(params.finalDocument), | ||
| { schema: params.settings.schema }, | ||
| ); | ||
| if (canonical.metadata.id !== params.subject.id) { | ||
| throw new PmCliError( | ||
| `history-redact would change item id from ${params.subject.id} to ${canonical.metadata.id}; narrow your patterns.`, | ||
| EXIT_CODE.USAGE, | ||
| ); | ||
| } | ||
| return { | ||
| document: canonical, | ||
| path: getItemPath( | ||
| params.pmRoot, | ||
| canonical.metadata.type, | ||
| params.subject.id, | ||
| "toon", | ||
| params.typeToFolder, | ||
| ), | ||
| raw: serializeItemDocument(canonical, { | ||
| format: "toon", | ||
| schema: params.settings.schema, | ||
| }), | ||
| }; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== locate relevant symbols ==\n'
rg -n "resolveHistoryRedactNextItem|history-redact|canonicalDocument\\(|getItemPath\\(|metadata.type|metadata.id" src/sdk/history-redact.ts src -g '!**/dist/**' -g '!**/build/**'
printf '\n== file outline ==\n'
ast-grep outline src/sdk/history-redact.ts --view expanded || true
printf '\n== read target region ==\n'
sed -n '430,590p' src/sdk/history-redact.ts
printf '\n== inspect call sites and related types ==\n'
rg -n "resolveHistoryRedactNextItem\\(|HistoryRedactNextItem|typeToFolder|currentItem|subject\\.type|subject\\.id" src -g '!**/dist/**' -g '!**/build/**'Repository: unbraind/pm-cli
Length of output: 39292
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("src/sdk/history-redact.ts")
text = p.read_text()
start = text.find("function resolveHistoryRedactNextItem")
print(text[start:text.find("function", start+1) if text.find("function", start+1) != -1 else len(text)])
PYRepository: unbraind/pm-cli
Length of output: 1293
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== inspect redaction + replay conversion ==\n'
sed -n '290,460p' src/sdk/history-redact.ts
printf '\n---\n'
sed -n '1840,1975p' src/core/item/item-format.ts
printf '\n---\n'
sed -n '1,180p' src/core/history/replay.ts
printf '\n== search for type normalization / guards in history-redact flow ==\n'
rg -n "metadata\\.type !==|metadata\\.type ===|change item type|type from|type.*narrow your patterns|replayToItemDocument\\(|toReplayDocument\\(" src/sdk src/core -g '!**/dist/**' -g '!**/build/**'Repository: unbraind/pm-cli
Length of output: 14676
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== redactHistoryEntry and patch traversal ==\n'
sed -n '372,460p' src/sdk/history-redact.ts
printf '\n== replayToItemDocument and canonicalDocument signatures ==\n'
rg -n "export function replayToItemDocument|export function canonicalDocument|interface ReplayDocument|interface ItemDocument|metadata:|type:" src/core/item/item-format.ts src/core/history/replay.ts -n
sed -n '1,140p' src/core/history/replay.ts
printf '\n---\n'
sed -n '1880,1945p' src/core/item/item-format.tsRepository: unbraind/pm-cli
Length of output: 10191
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== history-redact write path ==\n'
sed -n '800,920p' src/sdk/history-redact.ts
printf '\n== normalizeItemMetadata definition area ==\n'
rg -n "function normalizeItemMetadata|export function normalizeItemMetadata|normalizeItemMetadata\\(" src/core/item/item-format.ts
sed -n '1460,1565p' src/core/item/item-format.tsRepository: unbraind/pm-cli
Length of output: 8295
Guard metadata.type before writing the rewritten item. resolveHistoryRedactNextItem() only rejects metadata.id changes, but the canonicalized type also feeds getItemPath(). A redaction that mutates type would move the item to a different folder without failing.
🤖 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/sdk/history-redact.ts` around lines 513 - 547, Update
resolveHistoryRedactNextItem to validate canonical.metadata.type against the
original subject’s type before calling getItemPath or serializing the rewritten
document. Reject type changes with the existing PmCliError/EXIT_CODE.USAGE
pattern, while preserving the current behavior for unchanged types and
metadata.id validation.
| it("stages a symlinked source whose real install destination is nested inside it", async () => { | ||
| const root = await mkdtemp(path.join(os.tmpdir(), "pm-extension-copy-safety-")); | ||
| tempRoots.push(root); | ||
| const source = path.join(root, "source"); | ||
| const sourceAlias = path.join(root, "source-alias"); | ||
| const destination = path.join(source, ".agents", "pm", "extensions", "demo"); | ||
| await mkdir(source, { recursive: true }); | ||
| await writeFile(path.join(source, "manifest.json"), "manifest\n", "utf8"); | ||
| await symlink(source, sourceAlias, "dir"); | ||
|
|
||
| await _testOnly.copyExtensionDirectoryWithoutSelfNesting( | ||
| sourceAlias, | ||
| destination, | ||
| cp, | ||
| ); | ||
| expect(await readFile(path.join(destination, "manifest.json"), "utf8")).toBe( | ||
| "manifest\n", | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate the windows spec-exemption mechanism and check if this new spec is covered.
rg -n "windows" -i --glob '!node_modules' -g '*.mjs' -g '*.json' -g '*.ts' -C2
rg -n "extension-copy-safety" -iRepository: unbraind/pm-cli
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the test file and any windows-spec-exemption mechanism in the repo.
git ls-files | rg 'extension-copy-safety\.spec\.ts|windows|exempt|skip.*windows|spec.*exempt|ci.*windows|pm-u5zr'
echo '---'
if git ls-files | rg -q 'extension-copy-safety\.spec\.ts'; then
file=$(git ls-files | rg 'extension-copy-safety\.spec\.ts')
echo "FILE: $file"
wc -l "$file"
echo '--- OUTLINE ---'
ast-grep outline "$file" --view expanded || true
echo '--- CONTEXT ---'
cat -n "$file" | sed -n '1,220p'
fi
echo '---'
# Search for any explicit windows guards or exemption lists.
rg -n -i 'windows|Developer Mode|symlink|exemption|skip.*win|win32|platform' . --glob '!node_modules' --glob '!dist' --glob '!build'Repository: unbraind/pm-cli
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- pm-u5zr issue ---'
sed -n '1,240p' .agents/pm/issues/pm-u5zr.toon
echo '--- pm-u5zr history ---'
sed -n '1,240p' .agents/pm/history/pm-u5zr.jsonl | head -n 40
echo '--- CI / exemption search ---'
rg -n -i 'spec exemption|windows.*exempt|exempt.*windows|itOnPosix|it.skip|process\.platform !== "win32"|process\.platform === "win32"' \
tests .github src scripts docs .agents \
--glob '!node_modules' --glob '!dist' --glob '!build' \
-g '*.ts' -g '*.mjs' -g '*.json' -g '*.md' -g '*.toon' -g '*.yml' -g '*.yaml'
echo '--- windows regression job mention of extension tests ---'
sed -n '220,260p' tests/integration/ci-workflow-contract.spec.tsRepository: unbraind/pm-cli
Length of output: 26616
Avoid a plain directory symlink here on Windows
symlink(source, sourceAlias, "dir") will fail on Windows runners without Developer Mode/admin. Use a win32 junction or skip this case on win32 so the suite stays cross-platform.
🤖 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 `@tests/unit/extensions/extension-copy-safety.spec.ts` around lines 34 - 52,
Update the symlink setup in the test “stages a symlinked source whose real
install destination is nested inside it” to remain cross-platform: use a
Windows-compatible junction when running on win32, while preserving the
directory symlink on other platforms, or skip the case on win32. Keep the
existing copyExtensionDirectoryWithoutSelfNesting assertions unchanged.
| expect(rollup.sample.map((row) => row.id)).toEqual( | ||
| [...rollup.sample.map((row) => row.id)].sort((left, right) => | ||
| left.localeCompare(right), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Redundant array spread before .sort().
.map() already returns a fresh array, so spreading it again before .sort() is unnecessary.
♻️ Proposed simplification
expect(rollup.sample.map((row) => row.id)).toEqual(
- [...rollup.sample.map((row) => row.id)].sort((left, right) =>
+ rollup.sample.map((row) => row.id).sort((left, right) =>
left.localeCompare(right),
),
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect(rollup.sample.map((row) => row.id)).toEqual( | |
| [...rollup.sample.map((row) => row.id)].sort((left, right) => | |
| left.localeCompare(right), | |
| ), | |
| ); | |
| expect(rollup.sample.map((row) => row.id)).toEqual( | |
| rollup.sample.map((row) => row.id).sort((left, right) => | |
| left.localeCompare(right), | |
| ), | |
| ); |
🤖 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 `@tests/unit/sdk/history-read-primitives.spec.ts` around lines 229 - 233,
Remove the redundant array spread in the sort comparison within the rollup
sample assertion, sorting the fresh array returned by rollup.sample.map((row) =>
row.id) directly while preserving the existing localeCompare ordering and
expectation.
Source: Linters/SAST tools
…backfill, frontier notes - file pm-albl for GH-557 (contract layer intercepts -h/--help before variadic-positional handlers) under pm-ugqx - backfill estimate/risk/confidence on pm-d4ns/pm-evav/pm-topu/pm-zt1c (clears missing_estimate warning) - frontier notes on pm-usfg (remaining slices 3mna -> oxrw -> oslr) and pm-4k6b (catalog-to-shipped map for PR#546/#549/#554/#556/#559) - files-missing 162 drift comment on pm-j8z6; pass summary on pm-doxj Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VDJ1m5HDPDBVp8BBAixtnP
1 participant
Summary
PmClient.pm get --at, type-agnostic bounded child summaries, schedule facets, and token-efficient projections while preserving constant-cost normal reads for built-in leaf types.Tracked pm items
Documented follow-ups: pm-f38n and pm-kl19.
Verification
pnpm release:gatesReview notes
The shared history resolver deliberately supports two policies: immutable reads reject timestamps beyond the stream, while restore keeps latest-entry-at-or-before semantics. The public child SDK primitive remains type-agnostic; the CLI avoids automatic workspace scans for built-in leaf types unless
childrenis explicitly projected.Summary by cubic
Adds verified, mutation‑free time‑travel reads and promotes audited history maintenance into the public SDK. Also ships richer
getprojections (children rollups, normalized schedule) and startup bundle integrity checks across CLI, SDK, and MCP.New Features
historyRedact,historyRepair,historyCompact, andgetItemAt;PmClient.getsupportsoptions.atfor point‑in‑time reads (pm-4a7m, pm-hib1).pm get --at <version|timestamp>for immutable reads; children rollups (counts, samples, continuation) for any parent type; schedule facet for Meeting/Event/Reminder (pm-hib1, pm-y4z5, pm-x1g5).bundle_integrity_torn_install) without masking real module failures (pm-wfvq).Bug Fixes
children,schedule, andclaim_state; historical--atreads now return a usage error when requesting workspace‑level children.CommandErroras handled so Sentry doesn’t open exception groups (pm-8myl, pm-ksv2, pm-7071).pm get --atskips derived context‑usage ledger writes; strict range checks remain, andrestorekeeps latest‑at‑or‑before semantics.EXIT_CODE.USAGE; changelog regenerated to include the 2026.7.14 release boundary.Written for commit cdcb3bb. Summary will update on new commits.