Harden lifecycle cleanup and verification boundaries - #926
Conversation
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if not decision.accepted or decision.coverage is Coverage.BOUNDED: | ||
| detail = ( | ||
| decision.detail | ||
| if not decision.accepted | ||
| else ( | ||
| "Inline exact verification cannot bind a bounded scope; the " | ||
| "checker must report exhaustive or not-applicable coverage." | ||
| ) | ||
| ) |
There was a problem hiding this comment.
🔍 Inline exact guard covers only BOUNDED coverage, not other unverifiable decisions
The new guard fails closed for an accepted decision with Coverage.BOUNDED, but an accepted decision with Coverage.RESTRICTED/Coverage.SAMPLED, a non-replayable Method, or Arithmetic.FLOATING_HEURISTIC still falls through to the accepted path. In that case _commit_verification_record persists an InlineExactVerificationRecord first, and only afterwards does the Assurance model validator (src/jacobian/contracts/results.py:104-140) reject Verification.VERIFIED, so the ValueError is swallowed by _RECOVERABLE_VERIFICATION_ERRORS and the caller sees an ERROR result while the record artifact already exists in the store. No in-tree checker produces those combinations today (src/jacobian_checkers/exact_domain_operations.py only emits NOT_APPLICABLE/EXHAUSTIVE), so this is currently unreachable, but the same defensive reasoning that motivated the BOUNDED guard applies to the other unverifiable combinations.
Was this helpful? React with 👍 or 👎 to provide feedback.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cdd91e5a8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| input=InputValidation( | ||
| status=InputStatus.REJECTED, errors=(decision.detail,) | ||
| status=InputStatus.REJECTED, errors=(detail,) |
There was a problem hiding this comment.
When an authorized checker accepts the exact claim but reports Coverage.BOUNDED, the request has already passed schema and binding validation, yet this branch marks its input as rejected. This conflates input validity with insufficient coverage and causes consumers to diagnose a valid request as malformed; keep the input accepted while reporting the bounded decision as UNKNOWN and UNVERIFIED through the conclusion, assurance, and execution detail.
AGENTS.md reference: AGENTS.md:L140-L143
Useful? React with 👍 / 👎.
| error.add_note( | ||
| f"state database handle cleanup also failed: {close_error}" | ||
| ) |
There was a problem hiding this comment.
When checkpointing and handle cleanup both fail, converting close_error to a note discards its type, traceback, and identity; it also turns a secondary KeyboardInterrupt or SystemExit into an ordinary StateDatabaseError. Raise both failures in an appropriate exception group so callers and diagnostics can inspect every teardown failure rather than retaining only its string.
Useful? React with 👍 / 👎.
1 participant
Summary
Tightens the cleanup and verification boundaries identified in the repository hotspot audit.
Why
Several nested resource owners previously stopped on
KeyboardInterruptor discarded a secondary close failure. That could leave sibling resources running or hide useful shutdown diagnostics. Polynomial capability construction and inline verification also had paths where checker availability or bounded coverage did not produce the clearest fail-closed result.Validation
make test-unit— 869 passedmake test-process— 235 passedmake test-composition— 481 passedmake lint typecheck— passed; C901 baseline now has 0 violationsmake check-static— passedmake security-audit— no known vulnerabilitiesmake duplicate-code— 0 clonesSuggested review order