[shaurya703]

Summary

CompletionRouter kept a record's full result (text, omitted_chars and the artifact reference) for as long as the record existed, so a delivered child's output, or the withdrawn completions of a deleted parent, sat in the store until compact(retain) dropped the record. The only workaround was a short retention in a boot-time compact.

This adds CompletionRecord::settle (crate-private), which moves a record to a terminal state, bumps updated_at and empties result, and uses it on every settling path:

• mark_delivered → Delivered
• tombstone → Tombstoned
• cancel_parent → Tombstoned for every withdrawn pending record
• mark_failed → GaveUp; the returned record still carries the full result for the host's give-up policy, only the stored copy is emptied

The task id, parent key, status, attempts and timestamps stay, which is all that dedupe, tombstones and compaction read.

The issue describes a JSONL completion log; that store has since been replaced by DriverCompletionStore, which upserts one document per task id, so there is no growing log to compact on a size threshold. I left that part out: the per-task document now holds the emptied record, and compact(retain) remains the way to drop the dedupe entries. The tasks README still described the removed JSONL log, so its CompletionStore bullet now describes the driver store and the new settle behaviour.

API Or Behavior Changes

Behaviour change, no public API change: a settled CompletionRecord read back from a CompletionStore (get, list) has result == CompletionResult::default(). Pending records and the records mark_failed returns are unchanged. RecordOutcome / TombstoneOutcome semantics (dedupe after delivery, suppression after tombstone or cancel, survival across a reopen) are unchanged and covered by the existing tests.

Tests

New in crates/tinyagents-tasks/src/completions/router_tests.rs:

• a_delivered_record_keeps_its_bookkeeping_but_not_its_result
• tombstone_and_cancel_parent_drop_the_withdrawn_result
• the_give_up_writer_gets_the_full_result_and_the_store_does_not

New in crates/tinyagents-tasks/src/completions/driver_tests.rs:

• a_settled_record_is_persisted_without_its_result (asserts on the raw persisted document for delivered, tombstoned, gave-up and cancelled records)
• settled_records_reopen_without_their_result_and_stay_deduped (SQLite reopen)

Commands run locally:

• cargo fmt --check
• cargo clippy -p tinyagents-tasks --all-targets --all-features -- -D warnings
• cargo test -p tinyagents-tasks (112 passed)
• cargo test -p tinyagents-orchestration completion (37 passed; the subagent driver is the router's in-tree consumer)
• cargo clippy --all-targets -- -D warnings (full workspace not run locally)
• cargo build --all-targets / --all-features (full workspace not run locally)
• cargo test / cargo test --all-features (full workspace not run locally)

Documentation

• crates/tinyagents-tasks/src/README.md: CompletionStore bullet rewritten for the driver store and the settle behaviour; mark_failed bullet notes the returned-vs-stored result.
• CHANGELOG.md: new unreleased entry.
• Doc comments on CompletionRecord::result, tombstone, cancel_parent, mark_delivered, mark_failed and compact.

Closes #354

Summary by CodeRabbit

• Bug Fixes
  • Settled completion records now clear their stored results while retaining their state, identifiers, and attempt counts. This applies to delivered, tombstoned, cancelled, and gave-up records.
  • Delivered and gave-up results remain available in the immediate output. Duplicate detection and tombstone behavior are unchanged.
• Documentation
  • Updated completion storage guidance to describe stored records, skipped unreadable documents, and the distinction between returned give-up records and their stored copies.

[tinysweeper]

Tiny Sweeper review

Reviewed head: 17aa7ed61eeb720ba619dd686236d819d121a9a4

Verdict: Ready for maintainer review · Priority: none
Coverage: Reviewed 6 of 6 files.
Pull request purpose (title): fix(tasks): drop a c…

Diagnostics and supporting details

Full summary

Pull request purpose (title): fix(tasks): drop a completion's result once the record settles. Tiny Sweeper completed its review; deterministic results follow.

Review snapshot

Change surface	Files	Review signal	Count
Production	2	Active findings	0
Tests	2	Noted findings	0
Documentation	2	Resolved findings	0
Configuration	0	Pending checks/questions	0

Completeness: Complete
Test assessment: Test coverage is assessed from changed tests and lane evidence; execution is not claimed without trusted check data.

What changed

This iteration continues the fix(task) work on the completions subsystem of the tinyagents-tasks crate: `types.rs` documents that `result` is emptied once a record settles and adds a `pub(crate) settle` helper; `router.rs` routes `tombstone`, `cancel_parent` and `mark_delivered` through `settle`, and reworks `mark_failed` to take the result out via `std::mem::take` before storing the settled record and restoring it on the returned value, with updated doc comments for those methods and for `compact` (which frees the dedupe entries too, so the retention window must outlive any child's usefulness to its parent). The README's driver-store section is rewritten to describe the document store and result-dropping on settle, replacing the removed-JSONL-log prose, and the changelog gains an unreleased entry. The critique lane's summary reports 1 finding while its `findings` list is empty, so that finding cannot be characterised from the evidence. Relative to the prior reviewed head, no trusted push delta was available (latest_push_delta unavailable), and cached prompt differences cannot be treated as a proven delta; the visible changes include driver-test refactors to use shared `COMPLETIONS`/`task_id_key` helpers, an added `finished_at` stagger for deterministic claim ordering, persisted-document field-name changes (`record.result` → top-level `result`, `taskId` → `task_id`, `parentKey` → `parent_key`), removal of a stale blank line between changelog entries, and a `restart_recovery_note` assertion dropped from one router test — all consistent with the whole-pull-request diff but not independently confirmed as new in this push.

Features

• Added — CompletionRecord::settle centralises terminal-state transitions and drops the result: Centralises the settle-to-terminal-state transition with a `debug_assert_ne!` guard against settling to `Pending`; empties the result so a delivered or withdrawn child's output does not outlive the parent's interest in it. Dedupe, tombstones and compaction keep reading the surviving bookkeeping. (crates/tinyagents-tasks/src/completions/types.rs impl CompletionRecord {, crates/tinyagents-tasks/src/completions/types.rs pub struct CompletionRecord {)
• Modified — tombstone and cancel_parent withdraw records without their result: Tombstoned pending records (including a parent-flag stub when the completion has not arrived) are stored with id and state but no result, so a deleted parent's content does not wait for compaction. (crates/tinyagents-tasks/src/completions/router.rs impl CompletionRouter {, crates/tinyagents-tasks/src/completions/types.rs pub struct CompletionRecord {)
• Modified — mark_delivered stores the settled record without its result: Delivered records keep id, state and attempts for dedupe but drop text, artifact reference and omitted-character count; dedupe and the tombstone guard continue to work on the emptied record. (crates/tinyagents-tasks/src/completions/router.rs impl CompletionRouter {, crates/tinyagents-tasks/src/completions/types.rs pub struct CompletionRecord {)
• Modified — mark_failed returns the full result to the caller but stores an emptied record: The host's give-up writer is the result's last reader: it receives the full record while the stored copy keeps none, so failed output does not persist in the store either. Pending records and the records `mark_failed` returns are unchanged; `RecordOutcome`/`TombstoneOutcome` semantics are unchanged. (crates/tinyagents-tasks/src/completions/router.rs impl CompletionRouter {, crates/tinyagents-tasks/src/completions/types.rs pub struct CompletionRecord {)
• Internal refactor — compact doc note about settled-record results and dedupe entries: Clarifies that settled records' results already went at settle time and that compact frees the dedupe entries too, advising a retention window longer than any child can outlive its parent's interest. (crates/tinyagents-tasks/src/completions/router.rs impl CompletionRouter {, crates/tinyagents-tasks/src/completions/types.rs pub struct CompletionRecord {)
• Modified — README driver-store section rewrite and changelog entry for settled-result dropping: Aligns documentation with the new settle semantics; the description lane confirmed the README bullet, changelog entry and doc comments match what the diff does, including the removed-JSONL note and the stored-vs-returned distinction. (crates/tinyagents-tasks/src/README.md hand-off and nothing product-specific:, CHANGELOG.md Fixes (the two deferred tinyagents#463 findings):, CHANGELOG.md)

Tests

• router unit test (in-memory store) — Asserts a delivered record keeps state, attempts, task_id, parent_key, agent_id, status, label and timestamps while its result is the default, and that dedupe (`Duplicate`), the tombstone guard (`AlreadyFinal`) and `pending_for` still behave correctly afterwards.: Pins the delivered-record invariants: bookkeeping survives, result is emptied, dedupe and the tombstone guard still work on the emptied record. (crates/tinyagents-tasks/src/completions/router_tests.rs async fn mark_failed_on_a_settled_record_is_ignored() {)
• router unit test (in-memory store) — Asserts `tombstone` and `cancel_parent` leave withdrawn records tombstoned with default results, that re-recording them is suppressed, and that another parent's pending record is untouched.: Covers the withdrawn-path result drop and confirms cancellation is scoped to the parent. (crates/tinyagents-tasks/src/completions/router_tests.rs async fn mark_failed_on_a_settled_record_is_ignored() {)
• router unit test (in-memory store) — Asserts the record returned by `mark_failed` carries the full recorded result while the stored copy has a default result and matching state/attempts/updated_at; also checks no re-claim, dedupe on re-record, and that `mark_failed` on the settled record is ignored.: Pins the mark_failed split-return contract: the caller gets the full payload, the store does not. (crates/tinyagents-tasks/src/completions/router_tests.rs async fn mark_failed_on_a_settled_record_is_ignored() {)
• driver persistence test (document store) — Persists records with text, artifact and omitted_chars, settles them via `mark_delivered`, `mark_failed`, `tombstone` and `cancel_parent`, and asserts the persisted documents show the terminal state with empty text, zero omitted_chars and no artifact, plus that the store reads back default results.: Exercises the persistence layer across all four settle paths in a single flow. (crates/tinyagents-tasks/src/completions/driver_tests.rs async fn delivered_and_tombstoned_stay_deduped_after_a_reopen() {)

Findings

No active actionable findings.

Before merge

None.

Agent review details

critique

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: The critique lane reviewed the changed files and, apart from its summary line, listed no findings in its findings array.
• Lane summary: Reviewed 5 files; 1 finding. _The code index is behind this pull request (indexed at `f73ec4ed10f5`), so retrieved context may be out of date._

security

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Lane summary: The change empties a completion record's result when it settles (delivered, tombstoned, gave up) while preserving the bookkeeping dedupe and tombstones need, and the give-up path restores the full result for the host's final writer. The added tests cover the in-memory router, the give-up handoff, and the durable driver/sqlite reopen paths; I found no reachable misbehaviour or regressions introduced by the diff. 2 files were not security-reviewed: CHANGELOG.md (prose or tabular data), crates/tinyagents-tasks/src/README.md (prose or tabular data). _The code index is behind this pull request (indexed at `f73ec4ed10f5`), so retrieved context may be out of date._

tests

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: The stated invariant — the stored copy loses its result while id, state and attempts stay — is pinned by tests that would fail if it regressed; the tests lane found no reportable defect.
• Lane summary: Adds a `settle` helper that clears a completion's result when it reaches a terminal state, and the diff backs it with real tests: in-memory router tests covering delivered/tombstoned/`cancel_parent`/give-up paths (including that `mark_failed` still hands the host the full result while storing an empty one), durable-document assertions in `driver_tests`, and a reopen test proving dedupe and tombstones survive with no payload. The stated invariant — that the stored copy loses its result while id, state and attempts stay — is pinned by tests that would fail if it regressed. _The code index is behind this pull request (indexed at `f73ec4ed10f5`), so retrieved context may be out of date._

commits

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Positive: No sensitive content in committed files per the commits lane.
• Lane summary: No sensitive content in committed files. _The code index is behind this pull request (indexed at `f73ec4ed10f5`), so retrieved context may be out of date._

description

• Conclusion: Success
• Scope reviewed: all assigned evidence
• Lane summary: The change settles completion records through a new `CompletionRecord::settle` that clears the result on the delivered, tombstoned, cancelled and gave-up paths (the give-up writer still gets the full record back), and the added router/driver tests, README bullet, CHANGELOG entry and doc comments all match what the diff does. The description accurately covers the behaviour change, the removed-JSONL note and the stored-vs-returned distinction; I found nothing to report. _The code index is behind this pull request (indexed at `f73ec4ed10f5`), so retrieved context may be out of date._

Evidence and run details

• Models: ladder/vectors-oai3, deepseek-v4.1-flash, glm-5.3-flash
• Spend: $0.005191
• Tokens: 604861 input · 63858 output · 139392 cached · 580 embedding
• Updated: 2026-10-21T16:45:55Z

Head	State	Pass summary
0bfbe19fecdd	ready for maintainer review	0 active finding(s), 0 resolved finding(s) (at 2026-10-15T09:38:35Z)
17aa7ed61eeb	ready for maintainer review	0 active finding(s), 0 resolved finding(s) (at 2026-10-21T16:45:55Z)

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 6 billable files and costs up to $1.50.}

Or wait 20 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: b6e82abb-5d34-49a2-85e8-af3e5d45370f

📥 Commits

Reviewing files that changed from the base of the PR and between 0bfbe19 and 17aa7ed.

📒 Files selected for processing: 6

• CHANGELOG.md
• crates/tinyagents-tasks/src/README.md
• crates/tinyagents-tasks/src/completions/driver_tests.rs
• crates/tinyagents-tasks/src/completions/router.rs
• crates/tinyagents-tasks/src/completions/router_tests.rs
• crates/tinyagents-tasks/src/completions/types.rs

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: b5364c08-7cf1-406e-b1cc-06a4fdf2139c

📥 Commits

Reviewing files that changed from the base of the PR and between 3dbf2ef and 0bfbe19.

📒 Files selected for processing: 6

• CHANGELOG.md
• crates/tinyagents-tasks/src/README.md
• crates/tinyagents-tasks/src/completions/driver_tests.rs
• crates/tinyagents-tasks/src/completions/router.rs
• crates/tinyagents-tasks/src/completions/router_tests.rs
• crates/tinyagents-tasks/src/completions/types.rs

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

📝 Walkthrough

Settled completion records now clear their results while retaining bookkeeping for deduplication and tombstones. mark_failed stores a cleared record but returns the full result to the caller. Tests cover router behavior, driver persistence, and reopening.

Changes: Completion settlement

Layer / File(s)	Summary
Terminal settlement and result handling crates/tinyagents-tasks/src/completions/types.rs, crates/tinyagents-tasks/src/completions/router.rs, crates/tinyagents-tasks/src/completions/router_tests.rs, CHANGELOG.md, crates/tinyagents-tasks/src/README.md	A shared method sets terminal state and timestamp, then clears the result. Router settlement paths use it. mark_failed stores the cleared record and returns the full-result record. Router tests cover result clearing and retained bookkeeping.
Driver persistence and reopen behavior crates/tinyagents-tasks/src/completions/driver_tests.rs, crates/tinyagents-tasks/src/README.md	Driver tests verify that settled records retain identifying fields and terminal state while results are cleared, including after reopening. The README describes the driver collection and stored give-up records.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: senamakel

Merge Risk: ⚪ Minimal · up to 0bfbe

The supplied evidence identifies no actionable merge-blocking risk. Normal validation remains appropriate before merging.

Security architecture risk: 🟡 Moderate · up to 0bfbe

Discarding completed outputs reduces unnecessary retention. However, failed outputs can lose their recoverable copy before the recipient receives them if a batch partially fails or execution stops during handoff. No new privilege escalation or cross-tenant exposure was established.

Retained concerns

• Medium · reliability · inferred: Give-up settlement deletes the persisted result before completing ownership transfer to the host. For two eligible records, if the first write succeeds and the second fails, mark_failed returns an error without the first result, while that record is already GaveUp with an empty result. Retrying skips it. Interruption after persistence but before durable host consumption has the same recovery gap. The base had the same partial-batch sequencing but retained the full persisted result. Successful-return tests and earlier claim copies are counterevidence against universal loss, but neither establishes a recoverable handoff for callers relying on this return contract.

Security review details

Security Blast Radius

• observed — The inspected persistence scope is completion documents in a host-supplied, tenant-scoped document handle. Each write replaces the document keyed by task ID. The new settlement helper is crate-private rather than a new externally callable authority surface.

Trust Boundaries and Controls

• observed — Record admission continues to check parent cancellation, tombstones, cross-parent task-ID collisions, and duplicate IDs. Settlement preserves the identity and state those controls use. Host acknowledgement and give-up policy remain host responsibilities; these methods do not introduce a new authentication mechanism.

Resilience and Maintainability Implications

• inferred — The give-up recovery concern is a failure-containment issue, not an established attacker-driven exploit. Its scope is results successfully finalized before a later batch failure or interruption. An independently retained host copy can mitigate loss, but recovery from the completion store itself is newly unavailable.

Hardening Proposals

• proposed — Make give-up result ownership transfer recoverable before clearing the durable payload, through an acknowledged handoff or equivalent bounded durable mechanism. Preserve completed handoffs across partial batch errors without restoring indefinite result retention.

Pre-merge checks: 5

Passed checks: 5 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: clearing a completion's result when the record settles.
Linked Issues check		Issue #354 requires clearing stored completion results when records become Delivered, Tombstoned, or GaveUp. CompletionRecord::settle clears the full CompletionResult and preserves the dedup…
Out of Scope Changes check		The implementation changes, regression tests, README updates, and changelog entry all document or verify issue #354. The README correction reflects the current DriverCompletionStore and explains the…
Docstring Coverage		Docstring coverage is 89.47% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 4 files. (2 skipped: 2 …

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{A rabbit checks the settled store,
No result lingers there anymore.
IDs and states stay in their place,
While give-up keeps its full-result trace.
The pending paths still hop along,
Through reopen, dedupe stays strong.}

_{Comment @coderabbitai help to get the list of available commands.}

[tinysweeper]

tinysweeper found nothing blocking. Approving.

             $0.0087 · 319,155 in / 14,796 out · 36,480 cached (11%) · ladder/vectors-oai3, glm-5.3-flash · 549 embedded
critique:    $0.0047 · 153,741 in / 9,837 out  · 32,000 cached (21%) · glm-5.3-flash
security:    $0.0005 · 49,838 in  / 381 out    · 0 cached (0%)       · glm-5.3-flash
tests:       $0.0023 · 78,378 in  / 1,523 out  · 0 cached (0%)       · glm-5.3-flash
description: $0.0003 · 28,309 in  / 904 out    · 4,480 cached (16%)  · glm-5.3-flash

[tinysweeper]

tinysweeper found nothing blocking. Approving.

Diagnostics

Total spend: $0.0052