Repository navigation
fix(permits): enforce expiry in the guarded reservation update; fold the 2026-08 research into docs/ip - #298
Conversation
…isional #285 and #288 added verified, primary-source research to the repository and changed no file in docs/ip/. The package was therefore ranking mechanisms against an art landscape the project had already superseded. This corrects that, and assembles a filing-ordered specification. ## The prior art the package was missing `draft-farley-acta-signed-receipts` is the sharpest of it: Ed25519 over JCS-canonicalized JSON, namespaced receipt types, and from -02 a `spending_authority` type — reaching mechanisms 1, 3 and 4 at once. It was nowhere in docs/ip, including the IDS list. Also absent: protect-mcp (recorded as the closest competitor, emitting receipts verifiable without calling the issuer), jamjet (signed chained receipts plus budget enforcement), latch (idempotency plus budget caps), TraceAgent, and microsoft/agent-governance-toolkit. Separately, 02- ranked the guarded conditional UPDATE first without naming the textbook technique it instantiates. Optimistic concurrency control (Kung & Robinson, ACM TODS 6(2), June 1981, pp. 213-226) is now stated outright, along with the fact that this repository uses the same technique a second time in audit_chain.py — which an examiner may find. The distinction that survives is narrow and is stated narrowly: a domain-cap predicate rather than a version equality test, a zero-row outcome classified into an actionable reason, and no retry loop. ## The ranking changed as a result The settlement mechanisms held; the evidence mechanisms weakened, because the evidence half of this category filled in during 2026 while the package was being drafted. Offline verification drops from second to fourth: what survives is the six-state taxonomy specifically — a key the verifier does not hold resolving to UNKNOWN_KEY rather than to a tampering verdict — not "offline verification," which is now table stakes. Set A and Set C carry matching drafting notes. ## The SDK-release questions got narrower One comfortable assumption did not survive: no workflow or script in this repository downloads its own release assets, so the mechanical explanation that would have made the four download counts self-evidently benign is not present in the tree. The count is unexplained rather than explained. Recorded from the authenticated API instead: the release and both assets were produced by github-actions[bot]; updated_at equals published_at, so the release has not been edited since 2026-08-07; and both asset SHA-256 digests, which are what lets anyone later establish what was in the artifact on that date. Repository visibility could NOT be determined from this environment — an unauthenticated probe traverses a proxy, and a private repo returns 404 regardless, so neither code is evidence. That is now stated rather than assumed. ## New: 07-provisional-specification.md 03- and 05- assembled into one continuous 112(a) written description in filing order. Code citations are wrapped in [[brackets]] so counsel can find and strip them; [not implemented] markers are retained deliberately. Abstract is 146 words against the 150-word cap of 37 CFR 1.72(b). ## Validation - 40 file:line citations across docs/ip verified against the tree - 0 broken internal links - scripts/check_doc_references.py: 249 refs across 242 files resolve - ruff clean; pytest 1666 passed, 19 skipped - Kung & Robinson citation verified against dblp and the ACM DOI rather than asserted from memory Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVvJpNRU9wp3WXowfeqtPU
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_0b137be8-13c6-4965-b5b7-6fbf7c274734) |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughSummary by CodeRabbit
WalkthroughPermit reservations now enforce expiry atomically, use a shared UTC timestamp, and classify expiry races. Regression tests cover local and remote paths. Intellectual-property documents add the updated behavior, research, evidence, claims, and provisional specification. ChangesPermit expiry and filing documentation
Estimated code review effort: 4 (Complex) | ~45 minutes Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
… open The authenticated API reports private: true as of 2026-08-17. That settles the present state and only the present state — the API exposes current visibility, not history, and the interval since 2026-08-07 is the part that bears on whether the SDK sdist was ever publicly available. Stated that way rather than as a clean answer, and the counsel question updated to say what the API can and cannot close. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NVvJpNRU9wp3WXowfeqtPU
|
CI is red here, and it is not this PR. Flagging once rather than pushing at it. Every CI run in the repository is failing 4–5 seconds after start, including runs that have nothing to do with this branch:
Run 772 is a push to That pattern — instant failure, no logs, repo-wide, plus a second billing-metered service reporting a limit — is what a GitHub Actions spending/minutes limit looks like, not a test failure. Worth checking the account's Actions billing settings; nothing in the code will fix it. This PR is documentation only: no code, no migration, no API surface. It was verified locally against the merged tree before pushing — I'll re-check periodically and report when Actions recovers. Not pushing empty commits to retrigger in the meantime — that would just burn more of whatever quota is exhausted. Generated by Claude Code |
An expired permit keeps status="active" in storage — expiry is a dynamic
check, not a stored state (app/routers/me.py:102). The guarded UPDATE
predicated only on status and the budget cap, so it did not test validity.
A permit could therefore pass the read-time expiry check in validation,
cross expires_at, and still satisfy the guarded write — reserving budget
and dispatching under a permit that was no longer valid. The row lock does
not close that window on engines where it is a silent no-op, which is the
same reason the cap needs a database-evaluated predicate. Both reservation
paths had it: the local one and the upstream MCP dispatch path.
Second consequence, less obvious: the re-read that classifies a lost race
looked only at stored status, so permit_expired was unreachable from it. An
expired permit still read as "active" and the denial fell through to
permit_budget_exceeded — telling a caller with budget remaining that it was
out of money.
The fix adds `expires_at > :now` to the guarded predicate on both paths and
classifies expiry before budget in the re-read. One `now` is bound for the
predicate and the classification so the two cannot disagree. Naive-UTC
comparison matches the column type and follows the existing pattern in
reconcile_expired_permits.
## The regression tests fail without the fix
test_reserve_refuses_permit_that_expired_after_validation and
test_remote_prepare_refuses_permit_that_expired_after_validation force the
window open rather than waiting for it: validation runs for real against a
live permit, the row is then expired out of band from a separate session,
and only then does the guarded write run.
Verified against the pre-fix code:
> assert validation.allowed is False
E assert True is False
Both paths allowed the reservation under an expired permit. The upstream
test additionally asserts the denial leaves no prepared attempt, since a
prepared attempt with no reservation behind it is the inconsistency the
state machine exists to prevent.
## Docs
docs/ip 03-, 05- and 07- described this as the only remaining gap in the
reservation mechanism and marked it [not implemented]. Updated: the SQL
snippet and step-4 classification now carry the expiry term, the §7/§VIII
limitation is struck through as closed, and FIG. 2 no longer tells the
draftsperson to omit the permit_expired terminal — it now instructs drawing
all three, ordered status → expiry → budget to match the code.
## Validation
- pytest 1668 passed, 19 skipped
- both new tests confirmed to fail against pre-fix code (output above)
- ruff check clean; mypy clean across 172 source files
- check_doc_references.py: 249 refs across 242 files resolve
- 40 docs/ip file:line citations verified; 0 broken internal links
This touches permits and metering, which AGENTS.md marks security-critical,
so the change ships with negative-path tests on both affected paths.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NVvJpNRU9wp3WXowfeqtPU
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_341262a3-c1e0-4429-81e0-b79bf333345b) |
PetrefiedThunder
changed the title
|
Correcting my earlier comment: this PR is no longer documentation-only.
What it fixes. An expired permit keeps Both paths had it. There was also a quieter second effect: the lost-race re-read looked only at stored Reproduced before fixing. Reverting the two service files and re-running the new tests: Both paths allowed the reservation under an expired permit. The tests force the window open rather than waiting for it — validation runs for real against a live permit, the row is expired out of band from a separate session, then the guarded write runs. This closes a gap the IP package itself documented as the only one remaining in the reservation mechanism, marked CI note, unchanged. Actions is still failing repo-wide — the newest run at the time of writing is 787 (02:51:08, 4 seconds, on an unrelated branch), and Given that this now touches permits and metering with no CI coverage available, I'd treat the local evidence as necessary but not sufficient — worth re-running the trust gate before merge once Actions recovers. Generated by Claude Code |
|
GitHub Actions has recovered, and the gates I said were missing have now run — green. Run 31996530115 on
That resolves the caveat in my previous comment. The Validation section of the description still says the trust gate could not be run. That was true when written and is now stale — treat this comment as the correction; I'll leave the description otherwise as-is rather than rewriting history around it. Also merged Cursor Bugbot is still returning Generated by Claude Code |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…ath; make dispatch-budget release once-only without a row lock; refuse SQLite in production (#304) Three fixes from the audit findings #302 listed as not addressed. Each was verified against current main and each ships a regression test confirmed to fail against the unfixed source. 1. reserve_budget spent against expired permits. #298 added the expiry predicate to authorize_and_reserve's guarded update, but reserve_budget is a second reservation path and did not get it. An expired permit keeps status="active" in storage (expiry is a dynamic check, not a stored state), so the WHERE clause matched and budget moved. Adds the same expires_at > now term, plus precise denial reasons (permit_expired / permit_not_active) instead of reporting every miss as budget_exceeded. Engine-independent: this was wrong on PostgreSQL too. Test: an active permit whose expires_at is moved into the past is refused with permit_expired and spent_credits stays 0. Against the unfixed source the reservation succeeds (DID NOT RAISE). 2. release_dispatch_budget_once was not once-only. It read budget_released_at under with_for_update=True and then decremented the permit. SQLAlchemy silently drops FOR UPDATE on engines that do not support it, so two concurrent callers both observed NULL and both released — the permit was credited back twice for one dispatch. The claim is now a guarded UPDATE (NULL -> now, rowcount == 1 wins); only the winner decrements. The decrement itself was already atomic; it was the claim that was not. Test: two concurrent releases must yield exactly one True and exactly one decrement. Against the unfixed source both return True. 3. SQLite was a permitted production durable-state backend. STATE_BACKEND=memory was refused in production-like environments but sqlite was not, and auto-resolution would select it from a SQLite DATABASE_URL — the ordinary local-development setting. That is the deployment-level enabler for every remaining FOR UPDATE-is-a-no-op finding in the money path. Both the explicit and the auto path now fail closed. Tests: production-like + STATE_BACKEND=sqlite with SQLITE_URL set, and auto-resolution from a sqlite DATABASE_URL, both raise DurableStateConfigError. test_sqlite_without_url_refuses_production_like previously asserted the refusal reason was the missing SQLITE_URL, which implied a configured URL would be accepted. Its assertion is updated to the stricter reason; the behaviour it guards (production refuses this config) is unchanged. Also: the docs sweep in test_onboarding_contract.py walked .claude/worktrees/, a gitignored path holding full checkouts, so a copy of an already-excluded file failed the sweep for anyone using a git worktree there. Excluded .claude/. Tests: 1711 passed, 32 skipped; production_trust 6 passed; ruff clean; mypy clean across 172 source files; demo and red-team trust-plane gates --assert clean. Not addressed here: the remaining read-modify-write findings (wallet balance, reclaim_child_wallet double credit, refund reconciliation clobbering reservations, velocity counters) and the local-governed reconciler crash window. Fix 3 removes their production reachability; converting them to guarded updates is separate work. Claude-Session: https://claude.ai/code/session_01FsXYfTHDLiFNQ24R5JPN7H Co-authored-by: Claude <noreply@anthropic.com>
1. Permit expiry was not enforced by the guarded update
An expired permit keeps
status = "active"in storage — expiry is a dynamic check, not a stored state (app/routers/me.py:102). The guardedUPDATEpredicated only onstatusand the budget cap, so it did not test validity.A permit could pass the read-time expiry check during validation, cross
expires_at, and still satisfy the guarded write — reserving budget and dispatching under a permit that was no longer valid. The row lock doesn't close that window on engines where it is a silent no-op, which is the same reason the cap needs a database-evaluated predicate in the first place. Both reservation paths had it:PermitService.authorize_and_reserve()and the upstreamauthorize_reserve_and_prepare().There was a second, quieter consequence. The re-read that classifies a lost race looked only at stored
status, sopermit_expiredwas unreachable from it — an expired permit still read asactive, and the denial fell through topermit_budget_exceeded, telling a caller with budget remaining that it was out of money.The fix adds
expires_at > :nowto the guarded predicate on both paths and classifies expiry before budget in the re-read. Onenowis bound for both the predicate and the classification so the two cannot disagree. Naive-UTC comparison matches the column type and follows the existing pattern inreconcile_expired_permits.The regression tests fail without the fix
test_reserve_refuses_permit_that_expired_after_validationandtest_remote_prepare_refuses_permit_that_expired_after_validationforce the window open rather than waiting for it: validation runs for real against a live permit, the row is expired out of band from a separate session, and only then does the guarded write run.Verified against the pre-fix code:
Both paths allowed the reservation under an expired permit. The upstream test additionally asserts the denial leaves no prepared attempt, since a prepared attempt with no reservation behind it is exactly the inconsistency the state machine exists to prevent.
2. The IP package was ranking against superseded art
#285 and #288 added verified, primary-source research to this repository and changed no file in
docs/ip/.draft-farley-acta-signed-receiptsis the sharpest of it: Ed25519 over JCS-canonicalized JSON, namespaced receipt types, and from-02aspending_authorityreceipt type — reaching mechanisms 1, 3 and 4 at once. It appeared nowhere in the package, including the IDS list. Also absent: protect-mcp (recorded as the closest competitor, emitting receipts verifiable without calling the issuer), jamjet (signed chained receipts plus budget enforcement), latch (idempotency plus budget caps), TraceAgent, and microsoft/agent-governance-toolkit. All of §6 is marked as propagated and not independently re-verified — counsel must read the primaries.Separately,
02-ranked the guarded conditionalUPDATEfirst without naming the textbook technique it instantiates: optimistic concurrency control (Kung & Robinson, ACM TODS 6(2), June 1981, pp. 213–226, verified against dblp and the ACM DOI). The distinction that survives is stated narrowly — a domain-cap predicate rather than a version equality test, a zero-row outcome classified into an actionable reason, and no retry loop.The ranking changed as a result:
3. The SDK-release questions got narrower
No workflow or script in this repository downloads its own release assets, so the mechanical explanation that would have made the four download counts self-evidently benign is not in the tree. The count is unexplained rather than explained.
Recorded from the authenticated API instead: both assets produced by
github-actions[bot];updated_at == published_at, so the release has not been edited since 2026-08-07; both asset SHA-256 digests; andprivate: trueas of 2026-08-17 — which settles the present state and says nothing about the interval, which is the part that matters and is an audit-log question.4. New:
07-provisional-specification.md03-and05-assembled into one continuous §112(a) written description in filing order. Code citations wrapped in[[brackets]]so counsel can strip them;[not implemented]markers retained deliberately. Abstract is 146 words against the 150-word cap of 37 CFR 1.72(b).The expiry fix above closed one of those
[not implemented]markers —03-,05-and07-all described it as the only remaining gap in the reservation mechanism. FIG. 2 previously instructed the draftsperson not to draw apermit_expiredterminal; it now instructs drawing all three, ordered status → expiry → budget to match the code.Type of Change
Validation
pytest -q— 1668 passed, 19 skippedruff check .— cleanmypy app/— clean across 172 source filespython scripts/check_doc_references.py— 249 refs across 242 files resolvedocs/ipfile:linecitations verified; 0 broken internal linkspython scripts/export_openapi.py --check— N/A, no API surface changepython scripts/generate_sim_inventory.py --check— N/ASECURITY_LIMITATIONS.md— the docs changes remove overclaims, and the code change closes a limitation the package documentedmake trust-release-gatecould not be run: all GitHub Actions in this repository are currently failing 4–5 seconds after start with no logs, repo-wide, including pushes tomainand unrelated dependabot PRs. See the comment below. Everything above was verified locally against the merged tree.Risk
Raised from low, because this is no longer documentation-only. One behavioural change on a hot path: a reservation against a permit that crosses
expires_atmid-flight now denies where it previously succeeded. That is the intended correction — the permit was invalid — but it is a denial callers did not previously see, and it carries a reason code (permit_expired) that was previously unreachable from this path. No migration, no API surface change. Callers already handlepermit_expiredfrom validation, so the reason is not new to the contract, only newly reachable from the reservation re-read.