Repository navigation
feat(sdk): strengthen context and evidence contracts - #981
Conversation
Deliver a coupled SDK and CLI contract tranche across material omission receipts, scoped extension preflight activation, public claim-race recognition, truthful merge preference evidence, versioned history hashes, and typed assurance mutation refusals. Preserve legacy merge receipts and unversioned history readers, expose the new primitives through supported SDK entrypoints, add exact regression and packed-consumer coverage, document the contracts, and close the canonical pm/GitHub lineage with generated changelog evidence.
Reviewer's GuideStrengthens SDK and CLI context/evidence contracts by versioning history item hashes, tightening merge and omission receipts, formalizing assurance mutation refusal semantics, scoping extension preflight activation, enforcing lean MCP mutation envelopes, and exposing a stable claim-race predicate on the public SDK surface. Sequence diagram for versioned history hash verification and drift scansequenceDiagram
actor Client
participant HistoryReplay
participant DriftScan
Client->>HistoryReplay: verifyHistoryChainWithVersion(entries)
loop for each entry
alt explicit item_hash_version present
HistoryReplay->>HistoryReplay: [check SUPPORTED_HISTORY_ITEM_HASH_VERSIONS]
opt [unsupported version]
HistoryReplay-->>Client: { ok: false, errors: ["verify_failed:unsupported_item_hash_version:..."] }
end
end
HistoryReplay->>HistoryReplay: replayHash(replay, candidateVersion)
HistoryReplay->>HistoryReplay: hashDocumentForVersion(replayToItemDocument(replay), candidateVersion)
end
HistoryReplay-->>Client: { ok, errors, item_hash_version? }
Client->>DriftScan: scanHistoryDrift(workspace)
DriftScan->>DriftScan: verifyHistoryStream(entries)
DriftScan->>HistoryReplay: verifyHistoryChainWithVersion(entries)
HistoryReplay-->>DriftScan: { ok, item_hash_version? }
DriftScan->>DriftScan: hashDocumentForVersion(itemDocument, item_hash_version)
DriftScan-->>Client: DriftScanResult with item_hash_version and driftedItems
Sequence diagram for material omission receipts derived from output field groupssequenceDiagram
actor Agent
participant GetQuery as runGet
participant OutputProj as outputProjection
participant Omission as attachOutputOmissionReceipt
Agent->>GetQuery: runGet(pmRoot, id, options)
GetQuery->>GetQuery: itemMaterialFieldGroups(metadata, body)
GetQuery->>OutputProj: registerOutputMaterialFieldGroups(result, groups)
GetQuery-->>Agent: result
Agent->>Omission: attachOutputOmissionReceipt(command, result)
Omission->>Omission: resolveOutputOmissionReceipt(command, result)
Omission-->>Agent: result with omission_receipt
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Review limit reached
Next review available in: 29 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR records release closeout for several PM items and adds SDK behavior for versioned history hashes, merge receipt semantics, output materiality, claim detection, assurance errors, scoped extension ownership, and lean MCP mutation responses. ChangesVersioned history integrity
SDK contracts and projections
Activation and assurance behavior
Release records and documentation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
Create and claim the canonical review/merge coordination item, connect it to all coupled implementation items, and record the published PR lineage plus release/review runbooks.
![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, and left some high level feedback:
- The
normalizeAssuranceMutationhelper currently reclassifies allTypeErrors as usage refusals, which may mask unexpected internal type bugs; consider narrowing the check to only validation-originated errors (e.g., by tagging or originating function) to avoid overcatching. - The filtering logic in
resolveGetReceiptcombiningmaterialGroups,itemGroups, andObject.hasOwnis hard to reason about and subtly changes omission behavior; splitting this into clearly named predicates or intermediate variables would make the materiality rules and edge cases easier to audit. - The history hash version comparison helpers (e.g.,
compareJsonValuesusingJSON.stringify) rely on stringified JSON ordering and default fallbacks; if ordering or representation changes, hashes will drift, so it may be safer to centralize and document the canonicalization assumptions or reuse a shared stable stringify to avoid accidental changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `normalizeAssuranceMutation` helper currently reclassifies all `TypeError`s as usage refusals, which may mask unexpected internal type bugs; consider narrowing the check to only validation-originated errors (e.g., by tagging or originating function) to avoid overcatching.
- The filtering logic in `resolveGetReceipt` combining `materialGroups`, `itemGroups`, and `Object.hasOwn` is hard to reason about and subtly changes omission behavior; splitting this into clearly named predicates or intermediate variables would make the materiality rules and edge cases easier to audit.
- The history hash version comparison helpers (e.g., `compareJsonValues` using `JSON.stringify`) rely on stringified JSON ordering and default fallbacks; if ordering or representation changes, hashes will drift, so it may be safer to centralize and document the canonicalization assumptions or reuse a shared stable stringify to avoid accidental changes.
## Individual Comments
### Comment 1
<location path="tests/unit/sdk/assurance-action.spec.ts" line_range="152-149" />
<code_context>
+ it("types malformed, referenced, and unauthorized mutations as usage refusals", async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Add an explicit assertion on the typed refusal context to fully lock the assurance mutation contract
The new tests verify the correct `PmCliError` and `EXIT_CODE.USAGE` vs `GENERIC_FAILURE` behavior. Since `normalizeAssuranceMutation` now sets a structured context `{ code: "invalid_argument_value", reason: "assurance_mutation_refused" }` for validation `TypeError`s, please extend one expectation to assert on `error.context` as well (e.g. `toMatchObject({ exitCode: EXIT_CODE.USAGE, context: { code: "invalid_argument_value", reason: "assurance_mutation_refused" } })`) so the typed refusal contract is fully covered, not just the exit code.
Suggested implementation:
```typescript
it("types malformed, referenced, and unauthorized mutations as usage refusals", async () => {
await withTempPmPath(async ({ pmPath }) => {
const global = { path: pmPath };
await seedRegistry(pmPath);
await expect(
runAssuranceAction(
{
action: "put",
kind: "assertion",
id: "malformed-floor",
definition: {
```
```typescript
},
),
).rejects.toMatchObject({
exitCode: EXIT_CODE.USAGE,
context: {
code: "invalid_argument_value",
reason: "assurance_mutation_refused",
},
});
```
1. Ensure this expectation is specifically on the path that exercises a validation TypeError produced by `normalizeAssuranceMutation`, so the structured refusal context is asserted where the contract applies.
2. If there are multiple expectations in this test that all hit the same TypeError path, you may optionally mirror the `context` assertion on those as well for consistency, but the core contract is covered by at least one explicit `context` check as shown.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Merging this PR will degrade performance by 1.76%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | createQueryFingerprint |
1.4 ms | 1.5 ms | -1.76% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing feat/sdk-context-evidence-contracts (c893687) with main (9455e9b)
Preserve the strict 3,400-line source ceiling after adding the assurance refusal producer by rendering generated string arrays up to 100 columns deterministically. Add boundary coverage and record the exact-head preflight remediation on pm-ajj4r7.
|
@greptileai please review the exact current head cc74fa7. |
|
@coderabbitai full review |
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Cover the generated catalog branch above the compact 100-column boundary with a real multi-source fixture and record exact 100/100/100/100 remediation evidence on pm-ajj4r7.
|
@greptileai please re-review the exact current head 2f31fb9 after the coverage fix. |
|
@coderabbitai full review |
|
Tag expected assurance declaration and policy refusals explicitly so CLI and MCP transports do not mask unexpected TypeErrors. Clarify material omission ownership, document legacy hash byte compatibility, and assert the complete typed refusal context while retaining exact repository-wide coverage.
|
@greptileai Please review the exact current head 1bb1af4, including the typed assurance mutation-refusal boundary and its negative control. |
|
@coderabbitai full review Please review the exact current head 1bb1af4 after the assurance boundary remediation. |
|
|
|
Useful review. Commit 1bb1af4 narrows normalization to a dedicated AssuranceMutationRefusalError, adds an unexpected-TypeError negative control, clarifies resolveGetReceipt ownership, and documents why v1 must retain JSON.stringify byte ordering for immutable hash compatibility. |
|
Acknowledged: the guide accurately identifies the broad SDK/context/evidence changes and was used to structure the exact-head review. |
|
Acknowledged as a provider limitation, not approval: CodeRabbit hit its review limit and supplied no findings. A new exact-head full review was requested for 1bb1af4. |
|
Acknowledged: this is another rate-limit notice without a review. It is recorded as unavailable feedback, and exact-head review was re-requested. |
|
Addressed on the current head: the missing multiline generator branch has a real fixture, and local canonical coverage is exactly 100/100/100/100 across 52,193 statements, 38,348 branches, 10,745 functions, and 50,414 lines. |
|
Reviewed and classified as non-actionable benchmark noise: createQueryFingerprint and src/sdk/pagination.ts are unchanged by this PR; the reported 1.76% shift was on an older head and below a 0.1 ms rounded delta. Exact-head CodSpeed remains required before merge. |
|
Acknowledged as an unavailable review provider: this invocation was rate limited and produced no findings, so it is not counted as approval. |
|
Acknowledged: despite the initial status line, the action ended rate limited and did not review 1bb1af4. This remains a documented limitation, not approval. |
|
CodSpeed exact-head update acknowledged: the edited report now shows all 11 benchmarks untouched for 1bb1af4, superseding its older transient signal. Reaction corrected to 👍. |
Regenerate the error catalog and full runtime contract fixture after moving assurance mutation normalization into its dedicated typed boundary. Record the exact CI diagnosis and verification on the active PR delivery item.
|
@greptileai Please review the exact current head 1b31551, including the typed assurance refusal boundary and regenerated error provenance contracts. |
|
|
|
Acknowledged as unavailable feedback: the exact-head 1b31551 request was rate limited and produced no findings, so it is not approval. |
|
CodeRabbit summary edit acknowledged: the exact-head 1b31551 attempt remains rate limited with no review findings. This is recorded as a provider limitation, not approval. |
|
CodSpeed exact-head edit acknowledged: all 11 benchmarks remain untouched on 1b31551. |
|
@coderabbitai full review Please perform the final full review of exact head 1b31551 now that the stated rate-limit window has elapsed. |
|
@greptileai Final exact-head review request for 1b31551; please report any remaining actionable findings before merge. |
|
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 @.agents/pm/history/pm-hfqju5.jsonl:
- Around line 19-23: Reopen and reclaim pm-hfqju5, preserving its append-only
history, and keep it in_progress until PR `#981` has merged-head verification;
then add current delivery evidence before running pm close followed by pm
release. Regenerate .agents/pm/issues/pm-zryb9d.toon through pm after reopening
and reclaiming pm-zryb9d, applying the same deferred-transition and evidence
requirements; both affected sites require these lifecycle updates rather than
direct manual history edits.
In `@src/core/history/history.ts`:
- Around line 231-236: Update hashDocumentForVersion to validate that version is
a supported HistoryItemHashVersion before calling canonicalHashDocument or
hashing; reject any unsupported runtime value instead of implicitly treating it
as version 2, while preserving existing behavior for valid versions.
In `@src/sdk/governance/assurance.ts`:
- Around line 686-690: Update the validation around the retire branch in the
assurance mutation logic to verify that definition.retire_reason is a string
before invoking trim, routing non-string or missing values through
AssuranceMutationRefusalError with the existing invalid_argument_value
classification. Add an action-level regression test covering a numeric
retire_reason and assert the structured refusal response.
In `@src/sdk/query/get.ts`:
- Around line 159-175: Update itemMaterialFieldGroups in
src/sdk/query/get.ts:159-175 to include claim_state only when the source item
contains non-default claim information, not merely because the field group
exists. In tests/unit/sdk/output-projection.spec.ts:331-366, add an
unclaimed-item regression case asserting no claim_state omission and retain the
omission expectation for a fixture with material claim data.
🪄 Autofix
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 Plus
Run ID: d96cc062-85fa-45c3-9976-97a41e9e065e
📒 Files selected for processing (54)
.agents/pm/chores/pm-fb0lkg.toon.agents/pm/history/pm-2htk4p.jsonl.agents/pm/history/pm-ajj4r7.jsonl.agents/pm/history/pm-fb0lkg.jsonl.agents/pm/history/pm-gok2km.jsonl.agents/pm/history/pm-hfqju5.jsonl.agents/pm/history/pm-qckpnq.jsonl.agents/pm/history/pm-v0a0un.jsonl.agents/pm/history/pm-zryb9d.jsonl.agents/pm/issues/pm-2htk4p.toon.agents/pm/issues/pm-gok2km.toon.agents/pm/issues/pm-hfqju5.toon.agents/pm/issues/pm-qckpnq.toon.agents/pm/issues/pm-v0a0un.toon.agents/pm/issues/pm-zryb9d.toon.agents/pm/tasks/pm-ajj4r7.toonCHANGELOG.mddocs/MERGE_SAFETY.mddocs/README.mddocs/SDK_CONTEXT_EVIDENCE_CONTRACTS.mdscripts/generate-error-code-catalog.mjsscripts/smoke-codex-plugin-mcp.mjssdk/public-surface.jsonsrc/cli/main.tssrc/core/history/drift-scan.tssrc/core/history/history.tssrc/core/history/replay.tssrc/sdk/core.tssrc/sdk/generated-error-code-catalog.tssrc/sdk/governance/assurance-action.tssrc/sdk/governance/assurance-mutation-error.tssrc/sdk/governance/assurance.tssrc/sdk/index.tssrc/sdk/lifecycle/claim.tssrc/sdk/merge/driver.tssrc/sdk/merge/receipts.tssrc/sdk/merge/three-way.tssrc/sdk/output-projection.tssrc/sdk/query/get.tssrc/sdk/runtime-primitives.tssrc/types.tstests/fixtures/contracts/full.jsontests/unit/cli/extension-preflight-activation.spec.tstests/unit/core/history/drift-scan.spec.tstests/unit/core/history/history-hash-version.spec.tstests/unit/scripts/generate-error-code-catalog.spec.tstests/unit/scripts/smoke-codex-plugin-mcp.spec.tstests/unit/sdk/agent-runtime-primitives.spec.tstests/unit/sdk/assurance-action.spec.tstests/unit/sdk/assurance.spec.tstests/unit/sdk/claim-public-surface.spec.tstests/unit/sdk/merge-direction-contract.spec.tstests/unit/sdk/merge-receipts.spec.tstests/unit/sdk/output-projection.spec.ts
Reject unsupported history hash epochs before producer hashing, keep malformed assurance retirement metadata inside the typed refusal boundary, and stop billing default unclaimed state as material context. Add source-backed TDD coverage and record exact review remediation evidence on pm-ajj4r7.
|
The requested full review completed and produced four concrete findings. They have all been evaluated, reacted to, answered inline, and resolved; three fixes are in c893687 and the lifecycle recommendation is declined with live PM evidence. |
|
Reviewed all four actionable comments from this review. Commits through c893687 implement the three valid contract fixes with 7,235 tests at exact 100/100/100/100 and an 88-command isolated dogfood pass; the PM lifecycle recommendation is documented as declined because the dedicated delivery item owns merge closeout. |
|
@coderabbitai full review\n@greptileai please review\n\nFinal remediation head: c893687. Please re-review the three CodeRabbit contract fixes (hash epoch producer validation, non-string assurance retire_reason refusal, source-material claim-state omission) and confirm no new findings. All prior bot threads have been reacted to, answered, and resolved. |
|
@coderabbitai full review |
|
@greptileai please review exact head c893687 |
|
|
|
|
This final exact-head review request was rate limited and produced no findings. It is recorded as unavailable feedback, not approval; the remediation head is instead covered by all hosted gates, prior actionable review remediation, exact 100/100/100/100 coverage, and isolated dogfood. |
|
The bot identified the exact remediation head but the requested full review was rate limited before analysis. This is an explicit provider limitation, not approval; no new findings were supplied. |
|
Reviewed empty CodeRabbit review container PRR_kwDORSgRDc8AAAABJJ3sRA; it introduced no standalone findings. Its companion inline confirmation/withdrawal has been reacted to and answered on the resolved thread for exact head c893687. |
|
Reviewed empty CodeRabbit review container PRR_kwDORSgRDc8AAAABJJ4Dxg; it introduced no standalone findings. Its companion inline confirmation/withdrawal has been reacted to and answered on the resolved thread for exact head c893687. |
|
Reviewed empty CodeRabbit review container PRR_kwDORSgRDc8AAAABJJ4E7A; it introduced no standalone findings. Its companion inline confirmation/withdrawal has been reacted to and answered on the resolved thread for exact head c893687. |
|
CodeRabbit summary edit at c893687 reviewed: its linked-issues, scope, title, and description checks pass. The 55.56% docstring warning is not actionable because the repository's authoritative static gate measures 100% source/export/member docstring coverage and the exact-head hosted Gates (static) job passed. |
|
CodSpeed exact-head edit reviewed: createQueryFingerprint is untouched by this PR, the report explicitly detected different runtime environments, the rounded shift is 0.1 ms / 1.76%, 10 benchmarks are untouched, and the hosted CodSpeed job passed. Reaction records the noisy regression signal without treating it as a code defect. |
|
Codecov exact-head edit acknowledged: all modified coverable lines and tests pass; the independent canonical suite is 7,235/7,235 at exact 52,196/52,196 statements, 38,355/38,355 branches, 10,746/10,746 functions, and 50,417/50,417 lines. |
|
Reviewed final empty CodeRabbit confirmation container PRR_kwDORSgRDc8AAAABJJ4W7A; its companion inline acknowledgement confirms the addressed contract and carries a thumbs-up reaction. No new finding or code change is requested. |
|
Reviewed final empty CodeRabbit confirmation container PRR_kwDORSgRDc8AAAABJJ478g; its companion inline acknowledgement confirms the addressed contract and carries a thumbs-up reaction. No new finding or code change is requested. |
|
Reviewed final empty CodeRabbit confirmation container PRR_kwDORSgRDc8AAAABJJ4_1w; its companion inline acknowledgement confirms the addressed contract and carries a thumbs-up reaction. No new finding or code change is requested. |
Outcome
This broad SDK/CLI tranche makes context and evidence contracts more truthful, portable, and token-efficient across every supported surface:
isAlreadyClaimedErroris a supported, bundle-safe SDK primitiveCanonical pm items
preferredis the constant "ours" and contradictsretained/discardedunder stable_value_order #974 truthful merge receiptsalready_claimed_by#975 public claim-race predicateCloses #969
Closes #971
Closes #974
Closes #975
Closes #976
Verification
Release policy
v2026.8.11already exists, so this PR must not create a second automatic release today. After merge, the immutable existing npm/GitHub release will be verified independently; the next eligible daily release remains fail-closed while recent high-severity Sentry issues are unresolved.Summary by Sourcery
Strengthen SDK and CLI context, history, merge, assurance, and claim contracts for more truthful, portable, and version-compatible evidence across runtime surfaces.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores:
Summary by cubic
Strengthens SDK and CLI context/evidence contracts to cut tokens, clarify merge receipts, and harden history verification. Tracks PR #981 delivery and docs; tightens history epoch gating and omission billing.
New Features
pm getonly charge for material groups; empty/default fields no longer add cost (pm show --depth brief returns MORE output than --depth standard: the omission receipt costs more than the omitted fields #969).isAlreadyClaimedErrorfrom supported SDK entrypoints for safe, bundle-proof checks (SDK: isAlreadyClaimedError is declared in typings but not exported from any public subpath, forcing extensions to hard-codealready_claimed_by#975).requested_preferencefrom retained values; legacypreferredstill read; driver result usesrequested_preference(Merge receipt:preferredis the constant "ours" and contradictsretained/discardedunder stable_value_order #974).item_hash_versionwith default v2; replay/verify accept versions; drift cache v4; backward-compatible with v1 streams (history_drift disagrees across pm versions: 2+ tests_add ops on one item are drifted by 2026.8.7 and clean by 2026.8.10, and history-repair cannot satisfy both #976).invalid_argument_value/assurance_mutation_refused) across CLI/SDK/MCP; MCP smoke enforces a leanpm_createenvelope.Bug Fixes
sdk/governance/assurance-mutation-errorboundary and updated provenance.Written for commit c893687. Summary will update on new commits.