Skip to content

fix(stella-cli): stop a worker mirroring its private board over the lead's tasks rows (#1708) - #1967

Merged
macanderson merged 3 commits into
mainfrom
worktree-fix-1708-worker-board-mirror
Aug 7, 2026
Merged

fix(stella-cli): stop a worker mirroring its private board over the lead's tasks rows (#1708)#1967
macanderson merged 3 commits into
mainfrom
worktree-fix-1708-worker-board-mirror

Conversation

@macanderson

@macanderson macanderson commented Aug 7, 2026

Copy link
Copy Markdown
Owner

What

A sub-session worker's closeout wrote its own private task board into the shared tasks table under the lead's session_id (crates/stella-cli/src/subsession.rs, run_worker). At the table's UNIQUE(session_id, task_id) key that write was pure corruption, twice over:

  1. Ordinal collision — a worker board numbers from "1" in its own namespace, so its task "1" upserted over the lead's unrelated task "1".
  2. /clear seal bypass (A sub: worker that finishes after /clear still mutates the task board of the conversation that was cleared #1692) — the seal that quarantines pre-clear workers lives on the driver (SubSessions::seal_task_boardsession_clear::settle_worker_task); this write ran on the worker's own thread where no seal can reach, so a pre-clear worker repopulated the mirror the user destroyed.

The decision (of the three shapes in #1708)

Drop the write — the shape #1708's investigation identified as the only one that fixes both defects alone:

  • The write's stated purpose ("so tasks queries see sub-agent boards too") is unachievable at this key — neither reader (Store::list_session_tasks, the Observatory's sessions query) can distinguish a worker row from a lead row.
  • The delegation outcome the session cares about is the lead's board row, which the driver already mirrors at both of its own write sites (the lead's turn end in command_deck.rs, worker settlement in session_clear::settle_worker_task).
  • Namespacing fixes only defect 1; routing through the driver fixes only defect 2. Dropping fixes both, with no migration and no cross-thread plumbing — and it makes the /clear seal airtight rather than advisory, because the driver becomes the session mirror's sole writer.

The witness seam (the part #1708's investigation could not find)

The worker's store closeout moves to a new sibling module, crates/stella-cli/src/subsession/closeout.rs — required anyway, since subsession.rs sits 24 lines under the file-size gate's 1500-line ceiling and could not absorb tests. close_worker_execution is the exact production path run_worker calls, and its signature takes no session id, so the closeout structurally cannot address session-keyed rows.

Two witness tests drive that seam against a real in-memory store:

  • a_workers_private_board_never_lands_in_the_sessions_tasks_rows — the lead's mirror survives a worker closeout byte-identical, the table row count is pinned too (so a NULL-session append can't hide), and the execution row still closes (so "fixed by deleting the closeout" can't pass).
  • a_pre_clear_workers_closeout_cannot_repopulate_a_cleared_mirror — after clear_session_tasks (the persisted half of /clear), a worker closeout leaves the mirror exactly as empty as the user made it.

Fail→pass flip, demonstrated: the seam is new, so git stash alone can't show the flip. Instead the pre-#1708 board mirror was temporarily grafted back into close_worker_execution (same write, keyed to the lead's session exactly as run_worker was) — both witnesses fail against the graft with the exact corruption #1708 describes ("the lead's mirror survives a worker closeout untouched" assertion trips), and pass with it removed. The graft was reverted before shipping.

Verification

Measured on this branch with current main merged in (head 73f99926):

  • cargo test -p stella-cli1456 pass, 0 fail, including both new witnesses
  • cargo clippy -p stella-cli --all-targets -- -D warnings — clean
  • cargo fmt --check — clean
  • make guards-fast (all 25 guards) — clean; file-size reports "none grew" against main's regenerated baseline

CI is red for reasons outside this diff

fmt + clippy + test fails on five breaks inherited from main, none in a file this PR touches (the diff is 3 files, all stella-cli):

Break Crate Already covered by
unresolved link to SkipReason::NoResumePoint (daemon/boot.rs:59, added by a2806246 / #1939) stella-cli #1970
too many arguments (8/7) (clippy) stella-pipeline #1971
PassingShell not found stella-pipeline #1971
shell_call_result not found stella-pipeline #1971
non-exhaustive match: ModelCallRole::Research not covered stella-pipeline #1971

The last one is the root cause of three of these: main added a Research variant to stella_protocol::ModelCallRole without updating downstream exhaustive matches. This branch point has no Research variant at all, which is why the break is provably inherited rather than caused.

No competing unbreak is included here deliberately — duplicate PRs racing the same seam is how main gets re-broken. This PR should go green once #1970 and #1971 land; it is already merged up to current main and conflict-free.

No migration: the corrupted rows self-heal — the driver's next lead-board mirror upserts the colliding ordinals back to the lead's state, and /clear deletes the rest; stray rows beyond the lead's ordinal range in existing databases are dead rows in a table whose only production readers are per-session queries the driver now exclusively feeds.

Exemplar for the shape: the pipeline's witness stage gives authored witnesses exactly this lifetime — scaffolding for one run, discarded with it — which is the model applied to a worker's private board here.

Filed, not fixed

Refs #1692, #1631.

Closes #1708

…ead's tasks rows

A sub-session worker's closeout wrote its own task board into the shared
tasks table under the LEAD's session id. At UNIQUE(session_id, task_id)
the worker's ordinal "1" upserted over the lead's unrelated task "1",
and the write ran on the worker's own thread where /clear's driver-side
seal (#1692) cannot reach, so a pre-clear worker repopulated the mirror
the user destroyed.

The fix is subtraction: a worker's private board is scaffolding for its
one run, and the driver already mirrors the lead's board at both of its
own write sites. The worker closeout moves to subsession/closeout.rs —
a seam whose signature takes no session id, so it structurally cannot
address session-keyed rows — and two witness tests drive that production
path against a real store.

Closes #1708

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @macanderson, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@vercel

vercel Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
stella-cli-docs Ignored Ignored Preview Aug 7, 2026 2:24am

@sourcery-ai

sourcery-ai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Refactors worker closeout handling to remove worker task-board mirroring into the shared session tasks table, centralizing session task persistence in the driver and adding a dedicated closeout module with witness tests to prevent regressions in task-board corruption and /clear seal bypass.

Sequence diagram for worker closeout and session task board writes

sequenceDiagram
    participant WorkerThread as run_worker
    participant Closeout as closeout_close_worker_execution
    participant Driver as SubSessions_driver
    participant Store as Store

    WorkerThread->>Closeout: close_worker_execution(execution, registry, files_before, label, cost, persistence_complete)
    Closeout->>Store: record_execution_end

    Driver->>Store: record_task_board(lead_execution_id, session_id, items, now_ms)
    Driver->>Store: clear_session_tasks(session_id)
    Driver->>Store: seal_task_board
    Driver->>Store: settle_worker_task

    note over WorkerThread,Store: Worker closeout no longer calls record_task_board;
    note over Driver,Store: Driver is now the sole writer of session tasks rows
Loading

File-Level Changes

Change Details Files
Move worker execution closeout into a dedicated module that no longer writes to session task-board rows, enforcing that only the driver persists session task mirrors.
  • Introduce new subsession::closeout module holding close_worker_execution and its tests, with documentation of the previous corruption and invariants.
  • Implement close_worker_execution to record only execution audit data via agent::record_execution_end and intentionally omit any session- or task-board writes.
  • Add witness tests that verify worker closeout neither mutates the lead session's task rows nor repopulates a cleared task mirror, while still closing the worker execution row.
crates/stella-cli/src/subsession/closeout.rs
Update worker run path to use the new closeout module and remove inline worker task-board mirroring logic. crates/stella-cli/src/subsession.rs
Clarify /clear seal invariants by documenting that only the driver writes persisted session task-board mirrors.
  • Update session_clear module documentation to state that the /clear seal relies on the driver being the sole writer for persisted task mirrors.
  • Cross-reference the new subsession::closeout behavior to explain why worker closeouts do not write boards, ensuring the seal remains effective.
crates/stella-cli/src/command_deck/session_clear.rs

Assessment against linked issues

Issue Objective Addressed Explanation
#1708 Prevent a sub-session worker from mirroring its private task board into the shared tasks table under the lead session’s session_id, eliminating ordinal collisions where worker task IDs (e.g., "1") overwrite the lead’s tasks.
#1708 Ensure that a worker which predates a /clear cannot repopulate the cleared session task mirror, i.e., worker closeout must not bypass the /clear seal when persisting tasks rows.
#1708 Add witness tests that verify (a) a worker’s private board never overwrites or appends to the lead session’s tasks rows, and (b) a pre-clear worker’s closeout cannot repopulate a task mirror after clear_session_tasks (the /clear delete).

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@macanderson
macanderson merged commit 02f4337 into main Aug 7, 2026
15 checks passed
@macanderson
macanderson deleted the worktree-fix-1708-worker-board-mirror branch August 7, 2026 02:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A worker mirrors its OWN task board into the lead session's tasks rows — ordinal collision, and it bypasses /clear's board seal

1 participant