Give taskd a wire contract and bound its event history - #40
Merged
Conversation
Every handler serialized the runtime's internal structs directly (`serde_json::to_value(record)` on `TaskRecord`/`TaskEvent`/ `EvaluationReport`), so the persisted, in-process representation *was* the public API by accident: renaming an internal field, adding one, or adding an event variant silently changed what clients receive (`docs/reviews/architecture-review.md` §3). Add `wire.rs` between the two. The views borrow their data, own no logic, and name every field explicitly, so an internal rename is now a compile error in one file instead of a silent API break. Plan, capabilities, and outcomes stay as the `andromeda-core` contract types they already are — those carry their own versioning and are the shared vocabulary of the whole system, so mirroring them would duplicate a contract rather than insulate one. This commit changes no bytes on the wire, and says so twice: a golden test spells out the full document for a task exercising every construct and every event kind, and a second test asserts the view still reproduces the internal serialization exactly. From here the wire format can only change by editing the golden literal. `chrono` moves from dev- to normal dependency for the timestamp fields; it is already a normal dependency of core and runtime, so `Cargo.lock` is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`TaskRecord.events` is append-only and unbounded — every evaluate,
transition, grant, and outcome adds one, and an `evaluated` event embeds
a decision per action — so returning all of them made response size
unbounded. `GET /v1/tasks` was the worst case: it returned complete
records, multiplying one task's whole history by the number of tasks.
Give the reads a shape:
- `GET /v1/tasks/{id}` returns the 50 most recent events plus
`event_count`, the true total, so truncation is visible rather than
silent. `?events=<n>` widens the window, clamped to a hard maximum of
1000 at the single point where the response is built. A mistyped
parameter is a 400 rather than a silent fallback to the default, the
same rule the request bodies already follow.
- `GET /v1/tasks` returns a summary projection — id, state, revision,
intent, first/last event timestamps, and counts — with no event bodies
at all. This changes the listing's shape, so the docs change with it.
Measured, not claimed: for one task with a thousand events the full
record is 224 390 B and its summary 297 B, and the summary does not grow
with history at all. Over HTTP with 50 evaluations the listing goes from
20 770 B to 314 B.
Nothing else in the tree consumes these endpoints: `andromeda-ci-verify`
only calls `/healthz`, and `andromeda-cli` is not a taskd client — it
opens `FileTaskStore` in-process (architecture review #5), so its own
`task list` output is unaffected.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`TaskState::Draft` was produced by nothing and reached by no edge:
`create` picks `Ready` or `AwaitingApproval` after evaluating the plan,
the CLI's state enum never offered it, and no transition in the machine
targets it — so `{"to": "draft"}` was always refused. A state a
security-relevant machine can never be in is contract noise that every
client reading `state` still has to handle, so remove it along with its
three outgoing edges rather than invent a purpose for it.
Pin what is left: `the_transition_matrix_is_pinned` asserts allow/reject
for every ordered pair of states, and `the_state_list_covers_the_whole_
machine` proves the list it iterates cannot silently miss a variant (the
exhaustive match makes a new state a compile error). On the wire side, a
test asserts `draft` no longer parses at all.
No persisted record can carry `"state": "draft"` — nothing could write
one — so dropping the variant orphans no task.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.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
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.
Implements architecture-review finding #3 (taskd serves internal structs as wire DTOs, and returns unbounded event history) and removes the unreachable
Draftstate (engagement-summary #11).Verification: do the findings still hold?
Re-checked against
main(276014c) before writing any code. All three still held:serde_json::to_value(...)on the internal type:create_task,get_task,grant_capabilities,record_outcome,transition_taskonTaskRecord;evaluate_taskonEvaluationReport;list_tasksonVec<TaskRecord>+Vec<ListWarning>.get/listreturned the completeeventsvector with no projection, pagination, or ceiling, andlistdid it for every task at once.Draftunreachable — confirmed.create()yields onlyReadyorAwaitingApproval; no edge inTaskState::transitionhasDraftas a target, so{"to": "draft"}was always rejected; the CLI'sCliTaskStatenever offered it. Its only occurrences were the variant, its three outgoing edges, one test fixture instore.rs, and the README diagrams.Note on numbering: the brief refers to
Draftas architecture-review finding #8, but #8 in that document is the deadCapability.single_usefield.Draftis raised indocs/reviews/engagement-summary.mditem 11.single_useis untouched here.A. Wire DTO layer
New
crates/andromeda-taskd/src/wire.rs(private to the crate) holds borrowed view structs plus explicitFrommappings. Handlers now serialize only these. Because each mapping names every field, an internal rename is a compile error in one file instead of a silent API break.Plan, capabilities, and outcomes stay as the
andromeda-corecontract types they already are — those carry their own versioning (ActionPlan.schema_version) and are the shared vocabulary of the whole system, so mirroring them would duplicate a contract rather than insulate one. The golden test covers them anyway: it locks the entire document, nested core fields included.No version envelope was added. The brief's constraint ("do not change the current wire format… pins the existing shape, not a redesign") and the review's envelope suggestion are in tension; the constraint won. The
/v1path prefix plus/healthz'sapi_versionremain the version marker, andwire.rsis now the one place an envelope could be introduced deliberately.The lock test fails on an internal rename — demonstrated, not asserted. Two temporary renames, both reverted:
andromeda_core::Evidence.summary→text. The DTO passes this through, so the golden test is the guard, and it fired:outcomes[].evidence[]carried"text": "listing matched"against a lock that says"summary".TaskEventKind::Granted.plan_fully_granted→fully_granted. Two layers fired in sequence: first the DTO layer refused to compile (error[E0026]: variant Granted does not have a field named plan_fully_grantedatwire.rs:220) — i.e. the API cannot change without someone editing the wire file. Then, after mechanically following the compiler and renaming the DTO field too (the careless-refactor path), the golden test failed on exactly one key: wire"fully_granted"vs locked"plan_fully_granted".B. Bounded event history — wire format change
TaskRecord.eventsis append-only; every evaluate/transition/grant/outcome appends, and anevaluatedevent embeds a decision per action.GET /v1/tasks/{id}now returns the 50 most recent events (wire::DEFAULT_EVENT_LIMIT) plusevent_count, the true total, so truncation is visible rather than silent.?events=<n>widens it, clamped towire::MAX_EVENT_LIMIT= 1000 at the single point where the response is built.?events=0is legal. A mistyped parameter (?event=5) is a 400bad_requestrather than a silent fallback to the default — the same rule the request bodies follow (deny_unknown_fields).GET /v1/tasksnow returns a summary projection and no event bodies at all:task_id,state,revision,intent_summary,requested_by,created_at/updated_at(first/last event),action_count,capability_count,event_count,outcome_count. The{"tasks": [...], "warnings": [...]}envelope and the corrupt-record warning behaviour are unchanged.event_count.Measured improvement (from the test output, not estimates):
wire::tests::the_listing_projection_is_constant_in_the_event_count)GET /v1/tasksafter 50 evaluations (list_returns_summaries_instead_of_full_records)The summary grew by 3 bytes between 1 and 1000 events (the digits of
event_count), i.e. it is constant in the history size, and the listing multiplies that constant by the number of tasks instead of multiplying each task's whole history.Consumers of the changed shape — grepped the whole repo for
v1/tasks,"tasks",list_detailed, andTaskListing. Hits: the two READMEs, the taskd/runtime sources,docs/development/task-control-plane.md,docs/development/getting-started.md, and the review documents. Specifically:os/files/usr/libexec/andromeda-ci-verifycalls only/healthz— the image does not read/v1/tasks, so it cannot contradict the docs. The other threelibexecscripts make no HTTP calls at all.crates/andromeda-cliis not a taskd client (architecture review Build installable Andromeda Developer Preview #5): it opensFileTaskStorein-process and has no HTTP client, soandromeda task listis a separate surface and is unchanged.Docs updated in the same PR:
docs/development/task-control-plane.md(new "wire 契约与内部类型分离" and "读取形状与事件上界" sections, API table) and both READMEs' API tables.C.
Draft— removedNo concrete near-term use exists: nothing produces it, no edge targets it, and the CLI never offered it. A state a security-relevant machine can never be in is contract noise that every client reading
statestill has to branch on, so the variant and its three outgoing edges are gone.What replaced it, so the machine stays pinned:
the_transition_matrix_is_pinnedasserts allowed/rejected for every ordered pair of states against an explicit 15-edge list.the_state_list_covers_the_whole_machineproves that list cannot silently miss a variant: the match is exhaustive (a new state is a compile error) and each element must sit at the index its own variant names."draft"no longer deserializes at all, so{"to": "draft"}fails to parse rather than failing a transition check.No persisted record can carry
"state": "draft"— nothing could ever write one — so dropping the variant orphans no task. Both README state diagrams and the control-plane doc now showAwaitingApproval/Readyas the only entry states.Validation
cargo fmt --all -- --check: clean.cargo clippy --workspace --all-targets --locked -- -D warnings: clean.cargo test --workspace --locked: 252 passed, 0 failed (core 49, policy 68, runtime 48, hardware 26, cli 15, taskd 44+1+1).chronomoves from dev- to normal dependency inandromeda-taskd; it was already a normal dependency ofandromeda-core/-runtimeand a dev-dependency here, soCargo.lockis byte-identical (git diff origin/main -- Cargo.lockis empty).Out of scope
Storage-side cost is untouched:
liststill reads whole record files from disk (architecture review #4 — the response is what this PR bounds, not the read).Capability.single_use(the actual finding #8) is untouched.