feat(fleet): surface worker deliverables via summary and saved-session reply - #5946
feat(fleet): surface worker deliverables via summary and saved-session reply#5946gaord wants to merge 2 commits into
Conversation
…n reply
Accumulate streamed content into Completed.summary so receipt notes show a bounded deliverable excerpt instead of 'no verifiable output'.
Emit the real saved-session id in the session_capture stream event, persist it on FleetReceipt, and expose it via the runtime API so a client can resolve the worker's final assistant reply through GET /v1/sessions/{id}.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🔍 Deliverables expand lifecycle labels
Completed event labels now include up to 4,000 characters of worker output. Review downstream status displays that previously consumed short lifecycle labels.
(Refers to this code)
Was this helpful? React with 👍 or 👎 to provide feedback.
| match value.get("type").and_then(serde_json::Value::as_str) { | ||
| Some("content") => { | ||
| if let Some(content) = value.get("content").and_then(serde_json::Value::as_str) { | ||
| answer.push_str(content); |
There was a problem hiding this comment.
🔴 Streamed replies exhaust manager memory
Large streamed replies make answer.push_str retain the entire response for every active worker. The 4,000-character limit runs only after exit, so high-fanout Fleet runs can exhaust manager memory.
Prompt for agents
Bound Fleet worker summary accumulation while processing content events in crates/tui/src/fleet/executor.rs. WorkerStream.answer currently grows for the worker's entire run, while bounded_worker_summary truncates only after termination. Preserve enough state to produce the same redacted 4,000-character summary and truncation marker without retaining all streamed output. Account for redaction patterns that can span content-event boundaries.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if let Some(id) = saved_session_id.as_ref() { | ||
| emit_exec_stream_event(&ExecStreamEvent::SessionCapture { | ||
| content: exec_stream_session_ref(id), | ||
| session_id: id.clone(), |
There was a problem hiding this comment.
🟡 Failed saves advertise nonexistent sessions
When saving fails, SessionCapture.session_id publishes the fallback engine ID as a saved session. The error branch creates no file, so the advertised lookup returns 404.
Prompt for agents
Separate the persisted-session result from the preexisting session or engine ID in run_exec_agent. Emit a recoverable SessionCapture.session_id only when persist_exec_session successfully saved that exact session. Preserve any existing redacted diagnostic breadcrumb behavior needed after save failure, but do not let Fleet persist an ID that GET /v1/sessions/{id} cannot load.
Was this helpful? React with 👍 or 👎 to provide feedback.
| Some("session_capture") => { | ||
| if let Some(id) = value.get("session_id").and_then(serde_json::Value::as_str) | ||
| && !id.trim().is_empty() | ||
| { | ||
| *session_id = Some(id.to_string()); |
There was a problem hiding this comment.
🟡 Worker replies target inaccessible stores
session_id records worker-local IDs without ensuring the runtime API shares that session store. Explicit CODEWHALE_HOME is cleared locally, while SSH workers save remotely. Both receipts advertise unavailable sessions.
Prompt for agents
Make the Fleet saved-session handoff refer to a session available in the runtime API's configured SessionManager. Local Fleet launches currently rebuild the environment without CODEWHALE_HOME, and SSH exec persists on the remote host, but receipts expose the resulting worker-local ID through the manager's local GET /v1/sessions/{id}. Define an explicit transfer or shared-store contract for SSH sessions and propagate the runtime session root for local workers, then persist the receipt ID only after availability is established.
Was this helpful? React with 👍 or 👎 to provide feedback.
| #[serde(rename = "session_capture")] | ||
| SessionCapture { content: String }, | ||
| SessionCapture { | ||
| /// Redacted fingerprint for logs/forensics; never the recoverable id. | ||
| content: String, | ||
| /// The real saved-session id a caller can resolve via | ||
| /// `GET /v1/sessions/{id}` to read the worker's full transcript. | ||
| session_id: String, | ||
| }, |
There was a problem hiding this comment.
🔍 Session event keeps schema version 1
session_capture now carries a recoverable identifier under the unchanged stream schema. Review whether consumers need a version or capability signal for this sensitivity change.
Was this helpful? React with 👍 or 👎 to provide feedback.
| /// Redacted fingerprint for logs/forensics; never the recoverable id. | ||
| content: String, | ||
| /// The real saved-session id a caller can resolve via | ||
| /// `GET /v1/sessions/{id}` to read the worker's full transcript. | ||
| session_id: String, |
There was a problem hiding this comment.
🟥 Session identifiers escape their access boundary
session_id writes a recoverable identifier into the worker log, Fleet ledger, and receipt API. Readers of these broader surfaces can use it to fetch the full transcript.
Was this helpful? React with 👍 or 👎 to provide feedback.
| Some("session_capture") => { | ||
| if let Some(id) = value.get("session_id").and_then(serde_json::Value::as_str) | ||
| && !id.trim().is_empty() | ||
| { | ||
| *session_id = Some(id.to_string()); |
There was a problem hiding this comment.
🟨 Forged events can redirect transcript lookups
Any session_capture line can replace the receipt's session_id without provenance or uniqueness checks. A worker can redirect clients to an unrelated local transcript.
Was this helpful? React with 👍 or 👎 to provide feedback.
Hmbown
left a comment
Hmbown
left a comment
There was a problem hiding this comment.
Reviewed at 8f76be063. The problem is real — "worker exited successfully but produced no verifiable output" on a task that wrote a full report is a bad receipt, and it should be fixed. I'm requesting changes on the mechanism, not the goal: as written the excerpt is taken from the wrong end of the stream, the accumulator is unbounded, and the character count in the receipt is not the number it claims to be.
Tests I ran
Detached worktree, RUST_MIN_STACK=16777216:
cargo test -p codewhale-tui --lib -- fleet::executor fleet::task_spec— 38 passed; 0 failedcargo test -p codewhale-tui --lib -- fleet_receipt— 6 passed; 0 failedcargo test -p codewhale-protocol --lib -- fleet— 24 passed; 0 failed
All green. Everything below is from reading, not from a failing test.
1. The excerpt is the beginning of the run, not the deliverable
observe_worker_stream_line accumulates every content event (executor.rs, new block), and bounded_worker_summary then keeps the first 4,000 chars. For a report task the first 4,000 characters of the stream are the model thinking out loud on its way to the answer; the deliverable is at the end. So the receipt will usually show the opening of the run and call it "the deliverable".
The exec side already has the right value. In exec_agent.rs the terminal metadata event carries visible_final_answer_chars: summary.output.chars().count() — summary.output is the final assistant reply, already computed, already bounded by the output ceiling. Emitting a bounded excerpt of summary.output from exec (or the char count plus a bounded tail) would give the executor the real deliverable with no accumulation at all, and would delete the accumulator, bounded_worker_summary, and the double parse below in one move.
2. The accumulator is unbounded
Some("content") => {
if let Some(content) = value.get("content").and_then(Value::as_str) {
answer.push_str(content);
}
}No cap. WorkerStream::answer grows to the full size of the worker's streamed text — megabytes for a long run — and is held for the process's lifetime, per worker, across the whole fleet, so that 4,000 characters of it can be used at terminal. If you keep the accumulate-in-executor approach, stop appending once answer.len() passes the bound you actually need (plus a truncated: bool so the ellipsis stays honest).
3. The character count in the receipt note is wrong for exactly the cases it matters
task_spec.rs, new arm:
"worker produced {} characters of deliverable: {}",
summary.chars().count(),
bounded_receipt_excerpt(summary),input.summary has already been through bounded_worker_summary, which truncates to 4,000 and appends "...". So every deliverable longer than 4,000 characters reports "worker produced 4003 characters of deliverable" — a fixed number presented as a measurement. This repo is unusually careful about receipts being true; a count that silently saturates is the kind of thing someone will later debug for an hour. Either carry the real pre-truncation count alongside the excerpt, or drop the count and just show the excerpt.
4. Second copy of the bound-and-redact helper
bounded_worker_summary (executor.rs) and bounded_receipt_excerpt (task_spec.rs) are the same function twice: redact_secrets, take N chars, append "...". The text is consequently redacted twice on the way to the note. One helper, one call site, one cap.
5. Every worker stream line is now JSON-parsed twice
observe_worker_stream_line does serde_json::from_str::<Value>(line) and matches on value["type"], then immediately calls map_exec_stream_line(line) (executor.rs:366) which does serde_json::from_str and matches on value["type"] again. That is a second copy of the parse and of the dispatch, on the hot path for every line of every worker. Parse once and pass the Value down, or fold the two new arms into map_exec_stream_line — "content" is already a match arm there.
6. session_id now means two different things in the same event schema
After this PR the exec stream emits both of these:
metadata.session_id— stillexec_stream_session_ref(id), i.e. the redacted fingerprint (exec_agent.rs,ExecStreamMeta)session_capture.session_id— the raw recoverable id (this PR)
Same field name, opposite meaning, same stream. And metadata.resume_command still renders the literal codewhale exec --resume <redacted-session-id> (lib.rs:12202), which is now a redaction that protects nothing while remaining useless to the caller. Please pick one: either name the new field something that cannot be confused with the fingerprint (saved_session_id), or make the three surfaces consistent.
Related: the PR deletes assert!(!capture_json.contains(raw_session_id)), which was the guard on a deliberate invariant. I don't think the id is secret — text mode already prints session: {truncate_id(id)} and load_session_by_prefix resolves it — so I'm not calling this a leak. But it is an intentional invariant being retired, and that deserves a line in the PR body rather than a deleted assertion.
7. Docs
docs/AGENT_RUNTIME.md:144-148 documents the codewhale exec --output-format stream-json event vocabulary, and docs/zh_hans/AGENT_RUNTIME.md:76 mirrors it. Both should mention that session_capture now carries session_id, since that is the field an external caller has to know about to resolve GET /v1/sessions/{id}. Right now the feature is undiscoverable outside this diff.
Smaller things
- The summary is attached only to
Completed. A worker that fails after writing most of its report loses the text entirely — arguably that is when you most want it. "worker produced {} characters of deliverable: {}"is a new user-visible string on a surface this PR touches, added as a raw literal.crates/tui/locales/AGENTS.mdsays "new and touched surfaces use typedMessageIdkeys". The neighbouring notes inverify_task_resultare all raw English too, so this is consistent with its surroundings rather than a regression — flagging it so the decision is deliberate rather than inherited.partial()setsfailure_kind: None, so this text does not reachFleetAlertEvent::verifier_failedand does not get shipped to alert sinks. Good — I checked, and it means the new content stays on the receipt surface.- DCO:
0d7c29e40has noSigned-off-by:trailer.Check Signed-off-byis advisory in.github/workflows/dco.ymlso CI stays green, but CONTRIBUTING asks for it —git commit --amend -s.
The saved-session half of this (persist the id, expose it on the receipt, resolve the full transcript through the existing sessions API) is the right shape and I'd take it on its own. It's the summary half that needs another pass.
2 participants
Summary
Two related changes so a completed Fleet task no longer reports a meaningless receipt when it produced only text:
Bounded summary: the executor accumulates streamed content events and attaches a redacted, bounded excerpt to
Completed.summary. When a task has no scorer and no file artifact,verify_task_resultsurfaces that deliverable in the receipt notes instead of "no verifiable output".Saved-session reply:
codewhale execnow emits the real saved-session id in itssession_capturestream event (the log fingerprint stays redacted). The executor captures it,FleetReceiptpersists it, and the runtime API exposes it so a client can resolve the worker full final assistant reply via GET /v1/sessions/{id}.Testing
Note: main currently has 5 pre-existing nonminimal_bool clippy errors, unrelated to this change.