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 6 commits into
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| 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 publish stale sessions
When persist_exec_session fails, saved_session_id falls back to latest_session_id, yet SessionCapture publishes it as saved. The ID can reference an unchanged session or no file, so clients cannot retrieve the final reply.
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.
🟥 Worker logs expose transcript access handles
session_capture writes the raw saved_session_id into durable worker logs and receipts. Readers of either surface can use that handle to retrieve 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.
…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}.
… once, saved_session_id Maintainer follow-up on Hmbown#5946 (original by @gaord, preserved below as c58c74912). Keeps the saved-session half; reworks the summary half per review: 1. The excerpt now travels in the exec terminal event: the terminal `metadata` receipt carries `visible_final_answer_excerpt` (bounded, secret-redacted) next to the REAL pre-bound count `visible_final_answer_chars` of `summary.output` — the final reply, not the opening of the run. 2. The fleet executor's frame accumulator is deleted: nothing streams assistant text into per-worker memory anymore; `WorkerStream` only records the terminal receipt's answer. 3. Exactly one bound-and-redact helper, `exec_stream_final_answer_excerpt` (crates/tui/src/lib.rs, 4,000 chars); the executor-side `bounded_worker_summary` and task_spec-side `bounded_receipt_excerpt` duplicates are deleted. 4. The terminal frame is parsed exactly once: `parse_exec_terminal_*` take `&serde_json::Value`, and `WorkerStream::observe_line` parses each line once for route evidence, final answer, session capture, and payload mapping (`map_exec_stream_value`). 5. `session_capture.session_id` renamed to `saved_session_id` everywhere, and protocol `FleetReceipt.session_id` to `saved_session_id`; all consumers updated (runtime_api receipt JSON, manager, task_spec, ledger/alerts/control tests); `metadata` stays fingerprint-only and `metadata.resume_command` now names the field instead of pretending to redact one. 6. Docs updated in docs/AGENT_RUNTIME.md and docs/zh_hans/AGENT_RUNTIME.md. 7. The excerpt also surfaces on FAILED outcomes: it stays on `FleetWorkerTerminalEvent.final_answer` whatever the outcome, and a no-scorer failed/cancelled receipt keeps the text in its score notes; lifecycle event labels show a 160-char excerpt and worker inspection summaries bound notes to 240 bytes while payloads/receipts keep the full excerpt. Gates (RUST_MIN_STACK=16777216, shared target dir): - cargo fmt --all: clean, no changes - cargo clippy --workspace --all-targets --all-features --locked -D warnings -A clippy::uninlined_format_args -A clippy::too_many_arguments -A clippy::unnecessary_map_or: pass, 0 warnings - cargo test -p codewhale-tui --lib --locked -- fleet::executor fleet::task_spec fleet::manager terminal_mode_tests::exec_stream runtime_api::tests::fleet_receipt: 100 passed, 0 failed - cargo test -p codewhale-protocol --locked: 85 passed, 0 failed - cargo test -p codewhale-tui --lib --locked: 11856 passed, 0 failed, 13 ignored (one earlier run had 1 unrelated tmux clipboard flake that passes in isolation; a mass-failure run in between was shared-target-dir cross-worktree contamination, not this change) Signed-off-by: CodeWhale Bot <bot@codewhale.net>
8f76be0 to
4de6dc1
Compare
|
Maintainer follow-up pushed to |
| if let Some(answer) = parse_exec_terminal_final_answer(&value) { | ||
| self.final_answer = Some(answer); | ||
| } |
There was a problem hiding this comment.
🔍 Description names obsolete data flow
The PR description says the executor accumulates streamed content. The implementation now reads visible_final_answer_excerpt from terminal metadata instead.
Was this helpful? React with 👍 or 👎 to provide feedback.
Signed-off-by: CodeWhale Bot <bot@codewhale.net>
# Conflicts: # CHANGELOG.md # crates/tui/CHANGELOG.md
… cumulative stream Devin follow-up on Hmbown#5946: in a multi-step turn ExecSummary::output accumulates every streamed delta, including pre-tool commentary from earlier steps, so the terminal metadata could present progress text as the deliverable. Derive visible_final_answer_chars/excerpt from the last assistant-like message of the persisted session; the cumulative output stays only as the fallback when the session carries no assistant text. Signed-off-by: Ben Gao <bengao168@msn.com>
|
Follow-up pushed (
Local gates: fmt clean; |
| fn exec_stream_final_answer_text(messages: &[Message]) -> Option<String> { | ||
| let text = messages | ||
| .iter() | ||
| .rev() | ||
| .find(|message| message.role.is_assistant_like())? |
There was a problem hiding this comment.
🟡 Resumed turns reuse old replies
When a resumed turn produces no assistant message, exec_stream_final_answer_text selects the previous turn's reply. Terminal metadata then exposes that stale reply as the current deliverable.
Prompt for agents
exec_stream_final_answer_text searches the entire session history, including messages loaded before the current resumed turn. Scope final-answer extraction to assistant output authored during the current turn. The boundary must handle tool-result messages correctly and return None when the current turn produced no visible assistant text, allowing metadata to omit the excerpt rather than reusing an earlier reply. Add coverage for a resumed transcript whose new turn fails before producing assistant output.
Was this helpful? React with 👍 or 👎 to provide feedback.
| let text = messages | ||
| .iter() | ||
| .rev() | ||
| .find(|message| message.role.is_assistant_like())? |
There was a problem hiding this comment.
🔍 Interrupted fragments become deliverables
is_assistant_like includes InterruptedAssistant, so failed turns can surface incomplete text as a final deliverable. Review whether receipts need an explicit incomplete marker.
Was this helpful? React with 👍 or 👎 to provide feedback.
Preserve the contributor/security branch and all five Computer Use commits through 050f916 as separate parents. Resolve build.rs by retaining both complete initialization paths: loop-built localization data for small-stack safety and the bundled macOS Computer Use helper. The other five overlapping source files retain the MCP authority fixes and live tool discovery together. Format the final raw-key PTY test line missed by its preceding commit. Verified staged source tree: 314bd33 Local macOS evidence on this combined source: - cargo nextest run --workspace --all-features --locked --profile ci --test-threads 8: 14,621 passed, 0 failed, 15 skipped; 1 slow, 2 leaky process-output classifications, with no retry. Includes the six-case unbracketed offline-queue PTY regression for #5999. - Focused ACP/MCP/queue/CU discovery/shutdown regressions: 20 passed, 0 failed. - cargo clippy --workspace --all-targets --all-features --locked -- -D warnings: passed. - Workspace doctests: 3 passed, 0 failed, 8 ignored. - Final cargo fmt --all -- --check and git diff --cached --check: passed. - Vendored Computer Use tests: 130 passed, 0 failed. All 19 runtime files exactly match upstream eb5eac7, including verified type delivery and session re-lease. Development-only c9ab245 scanner was not vendored. - Combined web package: 411 tests passed, 0 failed; typecheck, facts and docs passed. Same web application source previously passed webpack production build (785 static pages) and lint (0 errors, 2 existing image warnings). - Core root npm test && npm run check:web is unavailable: package.json has neither script. Default Turbopack build cannot follow the existing external node_modules symlink; supported webpack build passed with font downloads. Tests use the isolated test-home helper, a per-tree Cargo target, CARGO_INCREMENTAL=0, and four compiler jobs. Computer Use helper signing is ad-hoc for these local artifacts. Initial sandbox-only fixture failures and the PTY fixture setup corrections remain in separate logs; no negative pre-fix PTY replay is claimed. Not claimed: exact-commit hosted CI, native Windows /pin acceptance, release packaging/signing/notarization, deployment, or customer acceptance. PR #6002 remote head 13c0f4a still has its older failing ACP check; this local source passes the exact regression. PR #5946 remains deferred because its incoming final-answer extraction can reuse a prior resumed turn's assistant reply. No public push, tag, remote merge, release, deployment, or provider spend.
Preserve gaord's #5946 history and expose the current worker reply in bounded terminal metadata and Fleet receipts. Repair stale resumed-turn extraction, omit capture claims after failed saves, and validate local session links against a fresh parent-assigned identity in the Runtime's existing store. Remote-only transcripts retain excerpts without claiming local availability. Worker metadata is bounded/redacted at the ingestion boundary as well. Validation: production cargo check passed. Focused all-feature nextest: 56 passed, 0 failed (12,031 tests filtered out). Includes actual child-process capture success, missing-store and forged-ID cases, and current-turn reply coverage. Extracted upstream answer helper fails the resumed-turn probe (0 passed/1 failed); the repaired helper passes (1 passed/0 failed). Formatting and diff checks passed. Full combined release gates follow after remaining contributor fixes; no new hosted CI or provider execution claimed. Local integration only; no main update, public push, remote merge or release.
2 participants
Summary
Two related changes so a completed Fleet task no longer reports a meaningless receipt when it produced only text:
Bounded deliverable excerpt: the terminal
codewhale execmetadatareceipt carriesvisible_final_answer_excerpt— a bounded, secret-redacted excerpt of the final assistant reply — next to the real pre-boundvisible_final_answer_charscount. The executor reads that receipt (never the streamedcontentdeltas) and attaches the excerpt toCompleted.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 undersaved_session_id(the log fingerprint stays redacted). The executor captures it,FleetReceiptpersists it, and the runtime API exposes it so a client can resolve the worker's full final assistant reply via GET /v1/sessions/{id}.Testing
Note: main currently has pre-existing nonminimal_bool clippy errors (subagent/mod.rs, apply.rs, fleet_roster.rs), unrelated to this change.
Maintainer follow-up
All credit for the feature and the original implementation goes to @gaord — the original commit is preserved below the follow-up. Per the review, the saved-session half was kept as-is and the summary half was reworked (head
4de6dc17f, branch rebased on currentmain):codewhale execmetadatareceipt carriesvisible_final_answer_excerpt, a bounded, secret-redacted excerpt of the final assistant reply, next to the real pre-boundvisible_final_answer_charscount.exec_stream_final_answer_excerptincrates/tui/src/lib.rs(4,000 chars); the executor- and task_spec-side duplicates are gone.parse_exec_terminal_*take&serde_json::Valueand the incremental reader maps route evidence, final answer, session capture, and ledger payload from one parse.session_capture.session_idis renamedsaved_session_id(protocolFleetReceipt.session_idtoo, plus every consumer: runtime API receipt JSON, manager, task_spec, tests). This deliberately retires the old "no raw id anywhere in the capture" invariant: the raw recoverable id now lives in exactly one field,session_capture.saved_session_id, while themetadatareceipt stays fingerprint-only andmetadata.resume_commandnames that field instead of a fake redaction.docs/AGENT_RUNTIME.mdanddocs/zh_hans/AGENT_RUNTIME.md.Gates (all run on
4de6dc17f):cargo fmt --allclean;cargo clippy --workspace --all-targets --all-features --locked -D warnings ...clean; targeted tui tests 100 passed / 0 failed;cargo test -p codewhale-protocol --locked85 passed / 0 failed; fullcargo test -p codewhale-tui --lib --locked11876 passed / 0 failed / 13 ignored.Contributor follow-up (Devin round 2)
Addressed on top of
4de6dc17f(headf94684cd6, merged currentmainfirst):ExecSummary::outputaccumulates every streamed delta, including pre-tool commentary from earlier steps, so the terminal metadata could present progress text as the deliverable. New helperexec_stream_final_answer_textderives the answer from the last assistant-like message of the persisted session (latest_messages); the cumulative output is only a fallback when the session carries no assistant text. Covered byexec_stream_final_answer_text_is_the_last_assistant_reply(pre-tool commentary + tool result + distinct final answer) andexec_stream_final_answer_text_requires_assistant_text.main). They also collided with main; the merge resolution keeps main's side.Gates on
f94684cd6:cargo fmt --all -- --checkclean;cargo test -p codewhale-tui --lib -- exec_stream_final_answer3 passed / 0 failed;cargo test -p codewhale-tui --lib -- fleet::executor fleet::task_spec fleet_receipt49 passed / 0 failed (one load-flake rerun per #5929, green on rerun and in isolation);cargo test -p codewhale-protocol --lib -- fleet24 passed / 0 failed.CI note: the
Test (windows-latest)leg ond3b333b06failed twomcp_boot_*engine tests — this branch does not touch that code, they pass on main's latest run, and the symptoms match the #5929 load-flake tracking issue (same class as the buildkite retrigger commit already on the branch).No-Issue: fleet worker deliverable surfacing tracked by this PR