Hand intake's ledger to admission - #734
Conversation
…ue handoff Admission was built against its own Records and Ledger interfaces; intake's ledger had no revision to guard a verdict with and nowhere to keep one. The ledger now implements both: migration 4 adds the revision and the verdict columns, a verdict commits through the ledger's one state-changing write in a transaction that decides admitted or queued, and RunAdmission takes from the queue intake offers to.
There was a problem hiding this comment.
🟡 Changes recommended
The concurrent integration test assumes an output ordering that the implementation does not guarantee, making CI potentially flaky.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Connects intake’s durable ledger to admission with revision-guarded verdict commits and persisted decision metadata.
Changes:
- Adds verdict storage, revisions, migration, retention cleanup, and transactional commits.
- Wires intake’s queue and shared NDJSON writer into admission.
- Adds integration, concurrency, migration, and lifecycle tests.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
File summaries
| File | Description |
|---|---|
internal/connector/ledger.go |
Adds admission metadata migration and index. |
internal/connector/ledger_events.go |
Adds decision persistence and revisioned transitions. |
internal/connector/ledger_admission.go |
Implements the intake/admission adapter and transactional commit. |
internal/connector/ledger_admission_test.go |
Tests adapter behavior and concurrency. |
internal/connector/intake.go |
Updates stranded-event recovery documentation. |
internal/connector/admission/run.go |
Updates intake seam documentation. |
internal/connector/admission/event.go |
Updates event documentation. |
internal/connector/admission/doc.go |
Documents the completed integration. |
internal/connector/admission/commit.go |
Updates ledger implementation documentation. |
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 3
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The ledger's one state write now owns blocked_at, retry_at and dropping the snapshot on a move to blocked or discarded, so SetState cannot leave them stale. A conversation is live when a record on it is admitted or dispatched, as admission's contract says; a queued record alone would stall its key. The conversation index is unconditional so the liveness read can use it. The handoff test asserts verdict lines by event id, not by arrival order.
There was a problem hiding this comment.
🟡 Changes recommended
The conversation-liveness implementation conflicts with the PR’s stated queued-record contract and must be reconciled.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
A queued record alone does not make a conversation live, per admission's Ledger contract, and deciding a later event neither moves nor re-decides it. The lifecycle comment names that definition and says a blocked record returns to work with content only through a new verdict.
There was a problem hiding this comment.
🔵 Needs a closer look
The durable schema migration and concurrent transactional lifecycle changes warrant final human review despite strong test coverage.
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
- Review effort level: Balanced (auto)
Note
Copilot is running an experiment and ran this review at Balanced.
Intake writes every feed event to its ledger and admission knows how to decide one, but nothing connects them. Admission was built against its own seam, because it landed before intake did, and intake's ledger had nothing that seam needs. There was no revision to guard a verdict with, so one event could get two verdicts or an older decision could overwrite a newer one. There was also nowhere to keep what admission decided.
Context: 15 Admission: metadata gate, trigger matrix, verdict, which left this adapter for after Take the account event feed into a durable ledger merged. Follows Decide which feed events are the agent's business: admission.
Dispatch and the
basecamp_connectMCP domain read what admission decided, so this join is what unblocks them.What changes:
NextBlockedRetryreads: when a run of blocks began, the last attempt, and a throttle's deadline. Nothing has shipped a ledger yet, but the schema still changes through a migration.SetStateand admission's commit both use it, so a verdict can't take an edge the lifecycle doesn't have, and any move stales a decision loaded before it.RunAdmissionconnects intake's queue toadmission.Run, sharing intake's line writer.Admission's gate and intake's recovery logic are untouched. Every move keeps the blocked-retry inputs consistent, and moving to blocked or discarded drops the snapshot. Blocked records are stored with what their retry schedule needs, but nothing re-offers them on that schedule yet. That runner belongs with the lifecycle work, along with
redispatch.