Repository navigation
Expose supported Operator checker recovery - #588
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 46 minutes. View limit details
✨ Finishing Touches
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 |
17320d1 to
cbdb97c
Compare
# Conflicts: # .ci/behavior-ownership/partition.v1.json # backend/app/modules/authorization/catalogue.py # backend/app/modules/checkers/router.py # mcp_server/contracts/authorization_context_get.json
# Conflicts: # backend/tests/test_api_rate_controls.py
# Conflicts: # backend/tests/test_api_rate_controls.py
# Conflicts: # backend/tests/test_api_rate_controls.py
durutheguru
left a comment
There was a problem hiding this comment.
Review on behalf of the repository owner. Authorization, replay, cursor binding and rollback hold up, and this PR is the user of the operations.checker_retry_source.list audit pair that #589's migration admits. One fix before merge:
Should fix — a source that was already retried returns a retryable 503 instead of a concealed 404.
After Operator A retries source R, a second POST for R with a fresh Idempotency-Key passes the task lock, _source_from_run and authority prepare. _reserve_after_task_lock then raises CheckerExecutionUnavailable("checker_retry_source_unavailable") (execution_coordination.py:108), and checker_retry.py:394 maps every CheckerExecutionUnavailable to CheckerRetryOperationUnavailable → 503 checker_recovery_unavailable, retryable: true. No duplicate run is created, but clients will retry a permanently stale source forever, which contradicts the description ("stale … sources stay concealed as 404"). Suggested fix:
except CheckerExecutionUnavailable as exc:
if str(exc) == "checker_retry_source_unavailable":
raise CheckerRetrySourceUnavailable("checker_retry_source_unavailable") from exc
raise CheckerRetryOperationUnavailable("checker_retry_reservation_unavailable") from excplus a regression test: retry R, then retry R again with a new key → 404, no new run, receipt or outbox event.
Optional.
_append_retry_event: anOutboxIdempotencyConflictonchecker-retry:<source_run_id>is deterministic but reported asretryable: true.discover()marks a sourceeligiblefrom the fence alone; a source whose task has leftevaluation_pendingis listed as eligible and then refused with 404.discover(): a malformed row (UUID / pydanticValidationError) surfaces as 500, while the retry path normalises the same failure to 503.
durutheguru
left a comment
There was a problem hiding this comment.
Approved on behalf of the repository owner. 0464524c8 fixes the superseded-source mapping: execution_coordination.py now raises CheckerRetrySourceUnavailable, checker_retry.py re-raises it as the concealed 404, and the authority test covers it. The optional notes (retryable OUTBOX key conflict, fence-only eligibility in discovery, 500 on malformed discovery rows) can be follow-ups. Merging once Backend CI against main passes.
Combine the checker-retry and project-management read families in the catalogue, dispatch and runtime union; recount inventories and recompute the partition, lane, MCP and debt-ledger digests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The retained
CheckerRetryServicecould safely supersede an exact finalized post-submit infrastructure failure, but Operators had no supported way to discover eligible sources or invoke that owner. This change adds project-scoped discovery and retry routes backed by fresh system Operator authority, signed bounded pagination, exact source/currentness checks, immutable receipt replay, concealed denials, and caller-root rollback/restaging.The implementation extends the existing CHECKERS retry transaction. It binds the URL project during source selection before any foreign TASK lock, returns the same privacy-bounded source projection from discovery and retry, and keeps queued, running, business-completed, stale, and unresolved
UNKNOWNeffects out of retry eligibility. GET uses the canonical authorization-read rate control; POST uses the existing admin-mutation rate control and rejects non-human authority before CHECKERS or TASK lookup.Prepare-time denials bind the original source/project, current identity link, and system scope before canonical denial evidence is retained. Internal retained-evidence, reservation, replay, and typed OUTBOX inconsistencies return bounded retryable
503after rollback. Absent, foreign, stale, ineligible, and unauthorized sources stay concealed as404; same-project request-key reuse with changed source or reason remains409. Exact same-key replay freshly revalidates Operator authority, validates the retained original allow, and creates no second run, receipt, outbox event, initiation audit, or allow decision.PR #589 is merged in
mainat60e6302f54b614e9e285f7a9f4e8b32909caa415and owns the shared operational-read audit migration, schema fingerprint, external-checker registry API, and full protected-route baseline. This branch is reconciled to that exact main ancestry, preserves the canonical migration and fingerprint byte-for-byte, and adds no sibling revision.Validation for this reviewed candidate:
667e5a053: exact prepared denial; complete Operator discovery/retry/replay/revocation; real OUTBOX-conflict rollback; and auto-acceptance stale-source refusal. Isolated database, role, and MinIO cleanup all completed. The external-review repair at0464524c8then passed the complete public Operator journey 1/1 in 62.84 seconds, proving same-key replay and fresh-key superseded-source404with unchanged durable retry facts; owned cleanup completed.git diff --check, and the workflow-exact test-structure validator pass.0464524c8, and final evidence review passed atf71782657; main-relative freshness/docs replay passed at5cbf4f3f4.This PR is ready for human review. Merge eligibility remains blocked until the new exact-head required Backend aggregate passes and all review conversations are resolved. Source-specific uncertain-delivery reconciliation remains a later bounded change.
Related to #581.
Summary by CodeRabbit