feat(server): agent issue-state write path (RelayBoardCall + transition executor) - #206
Open
seal-agent wants to merge 3 commits into
Open
Conversation
…on executor) Implements the agent write leg of the Compass board (SEA-1728 Part 5, T3-a of the frozen `compass-agent-primary-lifecycle` design). An agent calls `Board(SetIssueState)` on the AgentGateway; the Runner forwards it to the Server as `RelayBoardCall(session_id, call)`; the Server resolves the session to the caller agent account and runs the one shared compare-and-transition on the issue's canonical lifecycle state. ### What lands - `Hub.RelayBoardCall` (`relay_board.go`) + the `BoardCaller` seam — the board sibling of the lifecycle/comms relay legs, sharing their trust model exactly: fail-closed guard order (nil-caller `CodeUnavailable` before resolution; unbound session `CodeNotFound`, never a stale/admin account; delegate under the **resolved** caller account). A tool-level failure is returned in-band as `BoardCallError`; only a resolution miss / no-caller is a Connect error. - The single transition executor (`server/board.go`, `boardService.SetIssueState`) every state producer will share — the frozen compare-and-transition (`compass-issue-model/design.md:513-521`): under a serialized transition lock, read current truth, reject an `UNSPECIFIED` target, no-op-no-publish on same-state (ARCHIVED included), else commit to Postgres, read back, record+publish on the projection, and (nil-safe) mirror outbound (ARCHIVED elided). - `TransitionSource` (`kind` agent|tracker|auto + optional actor) fully defined so the PR-B poll driver and PR-C tracker ingestion add producers without changing the executor signature. Only `SourceAgent` is exercised here; the caller AccountID is recorded for attribution, no scope rejection ships (single-trust-domain MVP). - `IssueProjection.RecordAndPublish` — a state-only record+publish (the step-5 tail of `PublishIssueUpdate` without the forge upsert, which would demand forge fields and could not carry the state column). - Wired post-construction at `sinks.go` (`hub.SetBoardCaller`), the nil-safe hub-sink idiom; the outbound tracker mirror is left nil this PR (PR-C wires the real forge-tracker write). ### Not in this PR The forge/tracker poll driver (PR-B, SEA-1810), tracker-status ingestion / reverse mapping / echo-suppression / the real outbound mirror (PR-C), the `UpdateIssueState` `CompassService` handler (frozen record drops it), any proto or compass-ui change. ### Tests Red-first security tests (`relay_board_test.go`) mirroring the lifecycle leg (unbound->NotFound reaching the caller 0 times; nil-caller->Unavailable before resolution; resolved-account delegation; tool error in-band; call_id echo; unset-oneof invalid_argument). Default-lane executor tests (`board_test.go`) over a fake store + real projection: commit+publish on a real transition, reject UNSPECIFIED, unknown-issue NotFound, no-op-no-publish, nil-safe mirror (fires / nil no-op / ARCHIVED elided). Build+vet+scoped tests+gating lint green. Spec-impact: none. Refs SEA-1728 Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
This was referenced Aug 7, 2026
Addresses the sole review's one medium finding on the agent write path: three error-injection fields on the test fakes were declared but never exercised, leaving the executor's commit-failure, post-commit read-back-failure, and mirror-failure branches uncovered. Adds three default-lane cases (red-first) that drive each branch and assert the mapped Connect code plus the fan-out invariant: a commit failure maps to CodeInternal with no publish; a read-back failure maps to CodeInternal after the commit landed (setWrites==1) but before any publish, pinning the commit-then-read-back ordering; a mirror failure surfaces as CodeInternal after the publish already fired, pinning the publish-before-mirror ordering. Adds a `readBackErr` fake field gated on setWrites>0 to isolate the post-commit read. Also folds the two low findings as PR-C deferral comments at the mirror call site (error-after-commit ordering; the mirror must move off transitionMu once it does real network I/O). No production behavior change. Spec-impact: none. Refs SEA-1728 Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
Round-2 review flagged fakeIssueStore.getErr as the last dead injection field: read in GetIssue but assigned by no test. The prior commit wired the post-commit read-error case through the new readBackErr field, orphaning getErr. Coverage is unaffected by its removal — the pre-commit ErrNotFound arm is pinned by TestSetIssueStateUnknownIssueIsNotFound and transitionStoreError's default arm by the readBackErr test (both GetIssue call sites route through the same mapper). Spec-impact: none. Refs SEA-1728 Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is part of a stack containing 3 PRs:
mainImplements the agent write leg of the Compass board (SEA-1728 Part 5, T3-a of the frozen
compass-agent-primary-lifecycledesign). An agent callsBoard(SetIssueState)on the AgentGateway; the Runner forwards it to the Server asRelayBoardCall(session_id, call); the Server resolves the session to the caller agent account and runs the one shared compare-and-transition on the issue's canonical lifecycle state.What lands
Hub.RelayBoardCall(relay_board.go) + theBoardCallerseam — the board sibling of the lifecycle/comms relay legs, sharing their trust model exactly: fail-closed guard order (nil-callerCodeUnavailablebefore resolution; unbound sessionCodeNotFound, never a stale/admin account; delegate under the resolved caller account). A tool-level failure is returned in-band asBoardCallError; only a resolution miss / no-caller is a Connect error.server/board.go,boardService.SetIssueState) every state producer will share — the frozen compare-and-transition (compass-issue-model/design.md:513-521): under a serialized transition lock, read current truth, reject anUNSPECIFIEDtarget, no-op-no-publish on same-state (ARCHIVED included), else commit to Postgres, read back, record+publish on the projection, and (nil-safe) mirror outbound (ARCHIVED elided).TransitionSource(kindagent|tracker|auto + optional actor) fully defined so the PR-B poll driver and PR-C tracker ingestion add producers without changing the executor signature. OnlySourceAgentis exercised here; the caller AccountID is recorded for attribution, no scope rejection ships (single-trust-domain MVP).IssueProjection.RecordAndPublish— a state-only record+publish (the step-5 tail ofPublishIssueUpdatewithout the forge upsert, which would demand forge fields and could not carry the state column).sinks.go(hub.SetBoardCaller), the nil-safe hub-sink idiom; the outbound tracker mirror is left nil this PR (PR-C wires the real forge-tracker write).Not in this PR
The forge/tracker poll driver (PR-B, SEA-1810), tracker-status ingestion / reverse mapping / echo-suppression / the real outbound mirror (PR-C), the
UpdateIssueStateCompassServicehandler (frozen record drops it), any proto or compass-ui change.Tests
Red-first security tests (
relay_board_test.go) mirroring the lifecycle leg (unbound->NotFound reaching the caller 0 times; nil-caller->Unavailable before resolution; resolved-account delegation; tool error in-band; call_id echo; unset-oneof invalid_argument). Default-lane executor tests (board_test.go) over a fake store + real projection: commit+publish on a real transition, reject UNSPECIFIED, unknown-issue NotFound, no-op-no-publish, nil-safe mirror (fires / nil no-op / ARCHIVED elided). Build+vet+scoped tests+gating lint green.Spec-impact: none. Refs SEA-1728
Co-authored-by: Matt Wilkinson matt@sealedsecurity.com