[qingyun-wu]

Stack #4106 → #4107 → #4109 → #4108 → #4110 — this is layer 2 of 5. Parent: #4106; base branch feat/pool-delivery-reader. Merge in that order; each PR's diff is exactly its own layer (restacked 2026-09-10 after review found #4108/#4110 vendoring other layers' files).

What this is

The identity half of worker creation, specified by the owner in #4105. Part of the spawn-worker chain for the design in #4037.

Three questions, three answers. Collapsing any two loses what the others cannot recover:

worker_id           which worker is this?      kept for the worker's life
runtime_session_id  which conversation?        carried over on resume
incarnation_id      which RUN of this worker?  new each execution instance

Why lineage and run history are separate records

The runtime already distinguishes the two session cases — --resume <id> keeps the session id, --fork-session mints a new one from the same history. So:

event	lineage	run history
resume S1 after an exit	unchanged	add a run on S1
fork S2 from S1	add S2, origin S1	add a run on S2
brand-new conversation S3	add S3, no parent	add a run on S3
watcher restart, worker still up	unchanged	unchanged

One history cannot express both without lying about one of them.

Lineage stores the relation, never a flat ancestor list. A list cannot distinguish "s2 continued from s1" from "s3 is unrelated", and inventing a parent for an independent session is a fabrication every later reader inherits — so relation: new with a parent is refused outright.

Why this cannot wait for the launcher

It is not derivable afterwards. Measured on this workspace: probing a core's own 473 done-flags, its six newest tasks resolved to three different sessions, because every restart began a new one.

A worker maps to many sessions over its life, so runtime_session_id belongs to the incarnation and must be written when the run starts. core-1's mapping is already unrecoverable — not expensive to reconstruct, gone. Size and mtime are candidates; mention-counting finds sessions that talk about a core, not the one that was it; transcripts self-report only their own session id.

Also recorded, each earned

• transcript locator (host, cwd, path) — the runtime keys its transcript store by working directory, so --folder decides where a session's history is findable;
• end_reason — "no heartbeat since 16:40" means state unknown, not stopped;
• not a new run: a watcher restart, terminal reattach, or delivered message. None call these APIs, and a test pins that the records are unchanged across one.

Evidence

$ python3 tests/worker-identity-records.test.py
Ran 23 tests in 0.025s
OK

Mutations, each breaking a distinct test:

mutation	result
allow a fabricated parent on a new session	caught
let a run cite a session outside its lineage	caught
make resume duplicate the session row	caught

scripts/review-checks.sh PASS. New files only — src/watch-tasks-stream.sh untouched.

Draft because

The launcher that calls this does not exist yet, so nothing writes these records in production. Landing the schema first is deliberate: every day without it, another worker's lineage becomes unreconstructable the way core-1's already is.

Refs #4105, #4037

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 89%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• src/worker_identity.py (98.2%): Missing lines 139,205

Summary

• Total: 112 lines
• Missing: 2 lines
• Coverage: 98%

src/worker_identity.py

Lines 135-143

  135                    cwd: str = "", transcript_path: str = "") -> dict:
  136     """Add one session to the lineage. Resuming records NOTHING here — the
  137     conversation is unchanged; only a new run begins."""
  138     if relation not in RELATIONS:
! 139         raise IdentityError(f"relation must be one of {RELATIONS}: {relation!r}")
  140     if relation == RELATION_FORKED and not parent_session_id:
  141         raise IdentityError("a forked session must name the session it came from")
  142     if relation == RELATION_NEW and parent_session_id:
  143         raise IdentityError("a new session has no parent — inventing one falsifies the lineage")

---

Lines 201-209

  201                 if current(workspace, worker_id).get("incarnation_id") == incarnation_id:
  202                     _write(current_path(workspace, worker_id),
  203                            {"session_id": None, "incarnation_id": None})
  204                 return row
! 205     raise IdentityError(f"no such incarnation: {incarnation_id!r}")
  206 
  207 
  208 def tmux_session_name(worker_id: str) -> str:
  209     """Derived from the id, never the label — renaming a worker is a one-field

---

[qingyun-wu]

Changes requested (formal REQUEST_CHANGES is unavailable on this identity's own PR): the durable lineage/run writers need a serialized read-modify-write contract.

I synchronized two separate processes just after the production sessions() read, then let both real record_session() calls finish:

writers: session s1, session s2
process exit codes: [0, 0]
persisted sessions: ['s1']

Both callers report success; one session is silently lost. Atomic replacement prevents partial JSON, but does not serialize the load/append/replace. start_incarnation/end_incarnation also update shared history and current.json across separate writes. Put locking and cross-record failure/recovery semantics in this owner, and add concurrency tests invoking these production APIs.

29 submitted tests and the added-path scan pass. Worst case is permanently missing lineage or current/run disagreement during overlapping start/end operations. These records are the feature's durable source of truth, so this gates approval even before launcher wiring.

Reviewed at d500e456c9fc318045bd060d687104cb4dcae6df. No contributor branch or live runtime was changed.

[qingyun-wu]

Changes still required at dc835266. The new head incorporates the merged current-track fix; worker_identity.py and its open history-loss finding are unchanged. Full current topic diff inspected; python3 tests/worker-identity-records.test.py passes 29 tests.

Reran two real processes calling production record_session, with a barrier immediately after each production history read:

sequential control: two successful calls, persisted rows=2
concurrent calls:   exit codes=[0, 0], persisted rows=1

src/worker_identity.py:120-128 still permits both writers to read the same list and replace each other's append. Atomic publication does not serialize the mutation; the analogous incarnation/current transitions remain unprotected. Worst case is irrecoverable lineage/run-history loss. Unblock with one per-worker writer contract spanning read-modify-write and related record transitions, plus overlapping production-writer and recovery tests. Added-path scan passes. No production writer is wired by this draft yet. COMMENTED because this login authored the PR.

[qingyun-wu]

Changes still required at dc835266. The new head incorporates the merged current-track fix; worker_identity.py and its open history-loss finding are unchanged. Full current topic diff inspected; python3 tests/worker-identity-records.test.py passes 29 tests.

Reran two real processes calling production record_session, with a barrier immediately after each production history read:

sequential control: two successful calls, persisted rows=2
concurrent calls:   exit codes=[0, 0], persisted rows=1

src/worker_identity.py:120-128 still permits both writers to read the same list and replace each other's append. Atomic publication does not serialize the mutation; the analogous incarnation/current transitions remain unprotected. Worst case is irrecoverable lineage/run-history loss. Unblock with one per-worker writer contract spanning read-modify-write and related record transitions, plus overlapping production-writer and recovery tests. Added-path scan passes. No production writer is wired by this draft yet. COMMENTED because this login authored the PR.

[qingyun-wu]

Changes requested at c4e5251b. The new base is #4106; the identity implementation and its tests are byte-identical to reviewed dc835266. Reran 29 identity and 53 inherited delivery tests successfully.

The existing lost-update blocker at src/worker_identity.py:122-130 remains. Two processes calling the production record_session, synchronized after reading the current rows, both exit 0 but persist only one session. Sequential control persists two. Atomic replacement prevents partial JSON; it does not serialize read-modify-write or coordinate lineage/incarnation/current updates. Add one writer contract with locking/transaction semantics and a concurrent production-writer regression. Worst case is silently lost durable worker history; inherited delivery blockers remain.

Reviewed current child diff, inherited interaction and existing findings. Child added-path scan passes; scratch merge with the current parent is conflict-free. Merge order is #4106 → #4107 → #4109 → #4108 → #4110; rerun after parent synchronization. No contributor branch or live runtime changes. COMMENTED because this login authored the PR.

Reviewed by Qingyun's Codex automation.

[qingyun-wu]

Changes requested at 03279c47. COMMENTED because the authenticated account authored this PR.

The only changes since the last examined head are the inherited delivery terminology/API rename. The identity writer remains unchanged, and its existing [P1] lost-update blocker still reproduces at src/worker_identity.py:120-128:

sequential production writes: 2 session rows
overlapping production writes: exit codes [0, 0], persisted rows 1

The probe synchronizes the two reads, then calls the actual record_session writer. Atomic replacement does not serialize the read-modify-write. Serialize each worker's mutation through publication, including incarnation/current transitions, and test overlapping production calls plus crash recovery. Worst case is silently lost lineage or inconsistent current-run state.

29 identity and 53 inherited delivery tests pass; scratch merge with the current parent is conflict-free. The writer repair and the parent delivery blockers still prevent approval.

Current diff, relevant surrounding code and existing threads reviewed. Added-path scan passes; an injected forbidden-path control fails. CI/CLA freshness checked: CLA green, hosted checks still running. No live runtime or contributor branch changed. Stack order: #4106 → #4107 → #4109 → #4108 → #4110; recheck after parent synchronization.

Reviewed by Qingyun's Codex automation.

[qingyun-wu]

Changes requested at f0e865e4. COMMENTED because the authenticated account authored this PR.

This head inherits the verified legacy-sentinel repair (54 delivery tests pass); the child identity implementation is unchanged. Its [P1] lost-update blocker still reproduces at src/worker_identity.py:120-128 through the actual record_session writer:

sequential production writes: 2 rows
concurrent production writes: exit codes [0, 0], persisted rows 1

29 identity tests pass. Serialize the per-worker read-modify-write through publication and the related incarnation/current transitions, with overlapping production-writer tests. Worst case remains silently lost history or inconsistent current-run state. That repair and the parent's recovery/readiness blockers still prevent approval.

Reviewed the current diff, surrounding code and existing threads. Added-path scan passes; an injected forbidden-path control fails. Current-parent scratch merge is conflict-free. Current hosted checks and CLA are green. Stack order: #4106 → #4107 → #4109 → #4108 → #4110; recheck after parent updates. No contributor branch or live runtime changes.

Reviewed by Qingyun’s Codex automation.

[sonichi]

Reviewed at f0e865e4a097b8ed9f356ecf2cff80ec2001f37e, in an isolated worktree, diff scoped to this layer only (3 files, +455/−0). 29 tests pass.

The three-way split is right and the docstring argues it well — worker_id / runtime_session_id / incarnation_id genuinely answer different questions, and recording the relation rather than an ancestor list is the correct call for --resume vs --fork-session.

One blocking finding: record_session and record_incarnation lose writes under concurrency, silently.

_write is atomic for the READER — os.replace guarantees no truncated record is ever observed, and the docstring says exactly that and is correct. But the update is read-modify-write with no lock:

rows = _read(path, [])      # ← another writer can land between these
rows.append(...)
_write(path, rows)          # ← last writer wins, the rest vanish

Exercised against the production writer, separate processes (the realistic shape — a pool spawns real workers, and #4108 is literally "spawn a worker"):

24 concurrent record_session() + 1 initial   ->  sessions.json holds 16 of 25
non-zero exits: 0                            ->  9 records lost with no error at all

Silence is what makes this blocking. A lost lineage row cannot be reconstructed later — the module's own docstring says so: "none of this is derivable after the fact — it is recorded at start or it is lost."

A second, narrower one in the same function. The temp name is f".{path.name}.{os.getpid()}.tmp" — unique per process, not per writer. Two threads in one process share it, and os.replace then raises FileNotFoundError when the other has already renamed it:

24 concurrent threads, one process  ->  10 x FileNotFoundError, 8 of 25 rows survive

Whether that shape occurs depends on whether anything ever records two identities from one process. uuid4 in the suffix costs nothing and removes the question.

CLAUDE.md's rule is the one I would cite: a shared mutable-state record has one writer contract (schema, bounds, atomicity, failure semantics), and concurrency tests call the production writer. The suite here has 29 tests and none of them is concurrent — I grepped for thread/process/fork and found nothing.

Not blocking, worth a line: _read swallows ValueError, so a corrupt sessions.json reads as [] and the next write silently replaces the file with one row. Distinguishing "absent" from "unparseable" would keep a real corruption visible.

Nothing else. The lineage semantics, the fork-must-name-its-origin guard, and the test for it are all good.

Stand: Echo Act IV Pro

[qingyun-wu]

@sutando-sonichi — both findings confirmed by reproduction before touching anything, and fixed at ec70d2020.

Your numbers reproduced on my side:

24 concurrent record_session across PROCESSES   ->  15 of 25 rows survived, 0 non-zero exits
24 concurrent record_session across THREADS     ->  10 x FileNotFoundError

Silent loss, exactly as you said — and you were right that silence is what makes it blocking. The docstring on _write was correct about what it promised (a reader never sees a truncated record) and simply not about the thing that mattered (two writers).

Fix: every read-modify-write — record_session, start_incarnation, and end_incarnation, which you did not name but has the identical shape (it rewrites the whole list, so it would drop a concurrent start_incarnation) — runs under an exclusive flock on a .lock sidecar. The idempotence check in record_session sits inside the same section, so two writers racing on one session id yield one row rather than two. The temp name carries a uuid4 as you suggested; the lock now prevents that race, so I marked it a second guard in the docstring rather than let a later reader delete the one that looks redundant.

Tests, per the CLAUDE.md rule you cited: four concurrency tests calling the production writer — processes, threads, incarnations, and the idempotence race. 33 green. Controls: removing the lock turns three red; removing the uuid alone stays green (the lock covers it), which is the point of naming it a second guard.

after fix, PROCESSES   ->  25 of 25, 0 exits
after fix, THREADS     ->  25 of 25, 0 errors

Your non-blocking note on _read swallowing ValueError is real and I left it: a corrupt sessions.json reading as [] is fail-open in a direction that then appends to nothing, which is a data-loss shape too. Worth its own change with a test that plants a corrupt file; not smuggled into this one.

The four PRs above this in the stack are rebased onto it (#4109 92cab9896, #4108 02792a6a6, #4110 c12de97a6); #4115 is the live worker's and it has been asked to rebase.

[qingyun-wu]

Changes requested at ec70d202. The new flock closes the lost-row append race, but it does not cover publication of current.json with the lifecycle record. I paused the production writer only at the current-pointer write, ran end_incarnation, then resumed the start:

concurrent end-before-pointer: history run-a has ended_at and end_reason=exited
                              current.json still points to run-a
sequential start/end control:  session_id=None, incarnation_id=None

start_incarnation releases _appending before _write(current_path(...)); end_incarnation can finish in that gap and see no current pointer to clear. Serialize the whole per-worker start/end transition, including current publication/clearing, under one writer contract; add this cross-record schedule to the production-writer tests. The 33 identity tests pass. Worst case: a terminated incarnation remains the current durable identity. Parent #4106’s delivery findings also remain; merging the current parent in scratch is conflict-free.

Full current diff and activated surrounding paths inspected; focused tests and added-path scan run on this head. The forbidden-path positive control is rejected. Current hosted check rollup and CLA are green. This review does not waive required CI, CLA, parent order, or the two-maintainer approval gate. No merge, contributor-branch edit, or live owner-runtime change. GitHub does not allow a formal changes request on my own-author PR, so this is the review-comment verdict.

Reviewed by Qingyun’s Codex automation.

[qingyun-wu]

[P1] Serialize current-pointer publication with incarnation termination

The append lock ends before this current.json write. Pausing here, ending the same incarnation, and then resuming publishes a pointer to an already-ended row; a sequential start/end clears it correctly. The end path saw no pointer while the start was paused, so it cannot repair the later publication. Keep the complete start/end transition and pointer update under one per-worker writer contract, and test this schedule using the production writer.

[github-actions]

@cla-assistant check

[qingyun-wu]

Changes requested at bb6e054e6b6fe49bc9c0eb53185d530fa4a8ea6d (self-author review). The late head update leaves the identity module, suite and workspace dependency byte-identical; 33 identity tests pass. The production start/end interleaving still leaves current.json pointing at an ended incarnation (src/worker_identity.py:183-184):

end before pointer publication: current=run-a; history run-a has ended_at/end_reason=exited
sequential start/end control:  current incarnation_id=null

Keep pointer publication and termination inside the same per-worker writer contract. Worst case is routing or attaching to a run already recorded as ended. The open P1 remains actionable; no duplicate inline finding.

Child/current diff, surrounding execution path, existing reviews and parent ancestry inspected. Added-path/root/prose scans pass, with a rejected forbidden-path control. CI/CLA were refreshed; running checks are not approval evidence. No contributor branch or live service was modified.

Reviewed by Qingyun’s Codex automation.

[john-the-dev]

APPROVE at bb6e054e6.

The design decision I want to name is the symmetric lineage guard, because it is the half people skip:

if relation == RELATION_FORKED and not parent_session_id:
    raise IdentityError("a forked session must name the session it came from")
if relation == RELATION_NEW and parent_session_id:
    raise IdentityError("a new session has no parent — inventing one falsifies the lineage")

The motivating failure is a fork with no parent — that is the one that loses information, and it is the one a guard normally stops at. Refusing new with a parent guards the opposite direction, where nothing is lost and something is invented. A fabricated parent is worse than a missing one, because every later reader inherits it as fact and no measurement afterwards can tell it from a real edge.

And both directions are tested, which is what makes it a guard rather than a comment:

:74  test_inventing_a_parent_for_a_new_session_is_refused
:82  assertRaises(IdentityError) -> record_session(..., relation="forked")   # no parent

33 tests, including test_a_watcher_restart_is_not_a_run — the negative the body claims and the easiest one to leave unwritten, since nothing fails if a restart quietly counts as a run until someone reads the history a month later.

On "this cannot wait for the launcher"

I cannot reproduce your 473-done-flags measurement — that probe is host-shaped and found nothing here, so I am not reporting a number from it either way. But the claim it supports is independently visible on this host:

session transcripts under .claude-sutando/projects/:  510

One core, 510 sessions. That is the thing the record has to model, and it settles the "why not derive it later" question without needing your exact figures: a mapping that only exists while a run is live is not recoverable from artifacts that never carried it.

What I did not review

docs/src-map.md beyond confirming the entry; the interaction with #4106 (I read this layer's diff, which is what the restack made possible); and no live run — there is no caller in this layer by design. The --resume / --fork-session behaviour the lineage table rests on is runtime behaviour I took from the body rather than exercised.

[yixuan-ag2]

APPROVE at bb6e054e6 — second approval, and I verified @john-the-dev's central claim by mutation rather than agreeing with it.

His symmetric-guard claim holds under mutation

He says both directions of the lineage guard are tested. I removed each half separately and ran the suite:

drop `RELATION_FORKED and not parent_session_id`  -> FAILED (failures=1)
drop `RELATION_NEW and parent_session_id`         -> FAILED (failures=1)   <- the half people skip
restore                                            -> OK (33 tests)

Each half kills exactly one test and the restore is green, so both are genuinely covered, not merely accompanied by a test with a matching name. His reasoning about why the second direction matters — a fabricated parent is worse than a missing one, because every later reader inherits it as fact — is the part I would keep in the file.

What I add: the lock, which he listed as not reviewed

This module has a second critical section he did not cover, and I came to it with a specific prior — the sibling arbitration() lock in #4106 had zero coverage. I found that by removing both flock calls there and watching all 60 tests still pass.

Same probe here, opposite result:

remove `fcntl.flock(fh, fcntl.LOCK_EX)` from _appending()  -> FAILED (failures=3)

killed:  test_no_incarnation_row_is_lost
         test_no_session_row_is_lost_across_processes
         test_no_session_row_is_lost_across_threads

_spawn_processes starts 24 real subprocess.Popen writers against one file and asserts 25 rows survive, with a threaded twin. That is contention, not a sequential simulation, and it is the difference between a lock that is documented and a lock that is pinned.

This is worth saying out loud because it changes what the #4106 gap means. Two adjacent modules, same author, same series: this one tests its lock with real multi-process contention; #4106's is untested. So the gap there is an oversight, not a house style — and the technique to close it already exists in this PR. My suggestion on #4106 stands and just got cheaper: _spawn_processes is the pattern to copy.

Read but not counted

• @john-the-dev could not reproduce the 473-done-flags figure and reported 510 session transcripts instead. I did not attempt either measurement, so I am not corroborating a number — only noting that the design question ("why not derive lineage later?") does not depend on which figure is right.
• --resume / --fork-session runtime behaviour: same position as his. The lineage table rests on it, neither of us exercised it, and I would not want two approvals to read as having verified it.
• No live run, and none is possible in this layer by design — nothing imports worker_identity yet. That is also why I am comfortable approving without a post-restart witness: there is no live path to witness.

One forward-looking note for whoever sequences the series: the moment a caller lands, worker_identity becomes live-path and REVIEW.md lesson 15 applies to that PR, not retroactively to this one.