fix(persistence): preserve subagent cleanup and retention

[RohanK6]

Problem / Motivation

The live subagent client learns its session identity, provider, and (for Claude
Code) project CWD before the best-effort state.json update records them. If that
state write is skipped or fails and the run then exits abnormally, a tombstone
built only from state cannot identify or correctly route cleanup of the provider
transcript files.

Cleanup also needs two safety properties: current readable state—not a stale
tombstone snapshot—owns the mutable keep retention decision, and unreadable
state must preserve both the session files and the tombstone folder that owns
their identity for a later retry.

Why it matters

A failed non-retained run can leak .json / .jsonl transcript files containing
task context, tool calls, and partial output. Misrouting a Claude Code session
through the ACP cleanup default leaks the same files. Conversely, deleting when
retention intent is unknown can destroy resume material, while deleting only the
tombstone folder makes retained session files permanently invisible and
unreclaimable.

What changed (motivation → approach → change)

SubagentInfo now captures the live client's session_id, provider, and Claude
Code CWD immediately after session acquisition, before the fail-closed resume
guard, context construction, and fallible state update. Abnormal-exit tombstones
receive each non-empty live cleanup value independently. Shared-runtime handles
capture identity inside _create_shared_session; PID persistence is best-effort,
so a disk error cannot orphan the live shared handle by triggering dedicated
fallback. Early exits with no live value retain the existing persisted-state
behavior; an empty in-memory value never overwrites usable data. The current
base's manager extraction keeps continuation arbitration in
subagent_manager/continuation.py and session acquisition in
subagent_manager/run.py; subagent.py remains the unchanged state-owning facade,
while persistence bridges live cleanup identity to its existing tombstone writer.

The generic tombstone writer also snapshots non-empty session ID, provider, and
CWD from readable state so gateway-restart and delivered tombstones retain
cleanup identity if state corrupts later. Remembered live values are merged after
that state fallback and therefore override it. Cancel recovery can acquire multiple
sessions under one run ID; persistence atomically records complete per-session
cleanup generations in an owner-only record below the file-gated trust/ root,
outside the agent-writable run folder and independent of the tombstone's
orphan-exclusion role. Every read and write first applies fail-loud owner-only
restriction to all trust-store directories, including inheritable Windows DACLs,
and re-locks an existing record because tightening its parent does not retrofit
an older file ACL. Only a missing record reads as empty; I/O, parse, and schema
failures propagate so a later append cannot rewrite unreadable history as a fresh
single-generation record. Prune contains those failures per tombstone and continues
later entries without altering the corrupt record; shared-session setup logs them
best-effort and retains the live handle. Each generation records retention intent and the continuation
owner key before the later best-effort combined state update. Protected ownership
outranks empty or conflicting agent-writable keys, while the referenced owner's
current readable state remains authoritative; generation keep is the missing-field
fallback. Session acquisition publishes
the generation synchronously in memory before submitting durable work, so executor
saturation or cancellation cannot hide the live SID from terminal tombstones. The
in-memory fallback is append-only; the worker deduplicates only while serializing
the protected record off-loop and never replaces the live list, so an older writer
cannot discard a concurrent recovery SID. On the dedicated arm, durable generation
persistence follows the cancellation-drained provenance write. Both dedicated and
shared identity workers are shielded and fully drained before cancellation is
re-raised, so restart cannot precede protected authority. Cancellation cannot skip
required model fields while the already-published memory record still
feeds terminal tombstones. Current main's #7557 also moves the run PID, session
record, and shared-runtime PID writes through the same drained off-loop state
helper. The composition keeps cleanup CWD sourced from the identity captured at
acquisition, preserves the shared handle when storage fails, and binds generation
arguments inside its executor callable so cancellation probes reach the actual
drained state.json writer rather than the already-safe memory-first publication.
Slow storage therefore cannot stall heartbeat or hide a just-acquired session from
a contending tombstone, and a transient SID1 generation-write failure cannot be
lost when SID2 later succeeds.
Shared-handle ownership and provider references are attached immediately after
handle creation, before any cancellable persistence await, so force-reap always
destroys the shared handle instead of resetting a nonexistent dedicated session.
Shared-path persistence errors are logged without abandoning the live handle or
triggering dedicated fallback. Event-loop tombstone snapshots are memory-only:
they acquire the identity lock non-blocking and use writer-published generations,
never reading the protected durable record. Executor-owned prune independently
merges that record after restart; successful prune or explicit folder deletion
evicts both the durable and in-memory fallback. Tombstones snapshot the list and
leave the latest in top-level compatibility fields for diagnostics and restart
hints, but neither spelling can authorize provider deletion. Tombstone-write and
restart-scan fallbacks populate a separate SID-less retention-hint set used only by
the SessionManager exemption. Prune's deletion set comes only from the protected
record and synchronous live gateway publication. Retention fallback selects a
protected generation matching the current readable-state SID, or the latest
protected generation when that SID is absent or mismatched; agent-writable
state/tombstone SID and owner fields cannot select a victim identity or suppress
trusted owner/keep metadata. A
forged tombstone list/top-level SID or legacy cleanup-identities.json inside the
agent-writable run folder therefore cannot add another run's SID to that set.

During stale pruning, readable dictionary state is required before immediate
cleanup. Tombstone identity, provider, and CWD may guide folder eligibility and
match a trusted generation, but cannot expand provider-deletion authority. Only a
literal current-state keep is True is retained; strings such as "false" are
non-retention rather than truthy policy. keep=True preserves the identity folder
for restart registry rebuild; release to false allows prune to retry a failed
provider-file deletion. Readable keep=True
always defers disk prune; release or the conversation TTL writes false and owns
deletion. This arbitration is part of cleanup correctness, not an independent
retention feature: once durable identity makes provider files reachable, a stale
keep=False prune racing promotion could destroy newly reachable resume material.
False-to-true promotion and prune share per-agent state transactions inside
the single gateway process. Continuation tombstones lock both child and original
owner in stable order, then re-read under lock, so unrelated agents never contend.
Promotion writes true before that locked read, or prune keeps arbitration through
provider cleanup and folder removal so a later promotion returns retryable rather
than racing deletion. On the event loop, promotion probes arbitration and the
per-agent off-loop-writer lock non-blocking. Contention returns retryable
conversation_busy, so a later retry writes keep=True only after every older
writer completes. Off-loop promotion lets update_state acquire that non-reentrant
lock normally, avoiding self-deadlock while preserving serialization. Transient
persistence errors likewise return retryable without dispatch; retry restores exact
pre-attempt SessionManager/TTL ownership. The coordinator returns the result
directly through the unchanged forwarding facade, so concurrent callers carry
independent outcomes without a thread-local side channel. A process crash leaves no
half-committed claim format: the next prune re-reads current owner state under
arbitration. If state and a rewritten tombstone both lack a top-level SID, prune
derives retention and owner from the latest valid durable cleanup generation.
Provider cleanup occurs before lock release; folder and protected-record removal
follow only when every trusted generation reports success. Unsupported providers
and transient deletion failures preserve both retry surfaces for up to 90 days,
preventing a permanently missing route from accumulating private run folders forever.
A legacy SID present only in agent-folder state/tombstone receives the same bounded
lookup window: it cannot authorize provider deletion, but allows a later trusted
migration to reclaim the transcript. At the hard ceiling, only run-local metadata is
reaped; untrusted identity is never used for provider-file deletion. Restart registry
rebuild also requires literal keep is True, exact state SID/conversation-owner
matching against a protected/live generation, and sources provider/CWD from that
trusted record, preventing forged state from seeding TTL-release deletion. An
explicitly injected noncanonical state reader remains an application-owned trusted
integration seam; the canonical disk reader never takes that fallback. Continuation folders
follow the original conversation's readable keep
value directly: false or missing is non-retention, while unreadable owner state
receives bounded grace instead of inheriting the continuation's stale local true.
Registry rebuild, prune, and the conversation TTL sweep share
subagent_id_from_conversation_key; malformed keys are dropped per entry so they
cannot abort later cleanup. Every completed plain run records keep=False; readable legacy or failed-write
records with no keep key are treated as non-retained and prune at the normal
cutoff.
Unreadable state with tombstone SID also keeps the SessionManager deletion
exemption until prune owns cleanup. An acquisition-time keep=False generation
remains unknown because a later promotion may have landed only in the now-unreadable
state; only keep=True may collapse uncertainty, since it can only preserve data.
Missing, malformed, deeply nested,
Unicode-invalid, or non-object state receives an extra 24-hour grace anchored on
the normalized tombstone death time, preserving identity and session files beyond
the six-hour continuation TTL. Tombstone death is normalized without float
conversion; hostile values use validated file mtime (or the current sweep time when
clock rollback leaves no bounded timestamp) for cutoff and grace, and timestamps
exactly at the cutoff are eligible across platform clock resolutions.

Tests

The affected persistence suite proves the complete chain:

1. Session creation captures the live client SID and provider before persistence.
2. An abnormal timeout tombstone receives live SID/provider/CWD even when state
   never contained them; a fail-closed resume_failed before context construction
   also tombstones the fresh session identity; shared-session cleanup-generation and
   PID write failures retain the live shared handle instead of spawning an orphan
   fallback; and cancel recovery from SID1 to SID2 records and reclaims both sessions
   even when SID1's protected-record write fails transiently, SID2 succeeds, shutdown
   clears the tombstone, and simulated restart drops process memory. A cancellation-at-
   to_thread regression proves the live shared SID and ownership are visible before
   the queued durable writer starts, then drives force-reap through shared-provider
   shutdown and proves the handle is destroyed without dedicated reset; a rendezvous
   test proves a concurrent publication cannot be overwritten by an older writer. A direct regression
   makes the protected durable-record reader raise if tombstone creation calls it,
   proving event-loop snapshots remain memory-only while prune still recovers durable
   generations. A security regression writes the same forged victim SID to the
   tombstone's top-level and generation-list fields plus the old agent-folder
   cleanup-identities.json path, then proves prune deletes only the run-owned
   transcript while preserving the unrelated victim transcript. Another
   regression forces owner-only directory restriction to fail and proves both read
   and write abort before the protected record changes. A malformed protected record
   likewise raises and remains byte-for-byte untouched by a later append; a sorted
   corrupt-first prune proves the valid later entry still reclaims. The persistence
   fixture also asserts trust records stay under that test's own temp root. Shared
   I/O/parse/schema failure cases retain the created handle. Shared and dedicated
   cancellation tests hold the identity worker in flight and prove the run
   cannot finish/restart until the protected write lands.
3. Readable partial state with keep=False but no SID cleans through tombstone
   metadata; generic gateway-restart/delivered tombstones snapshot readable
   SID/provider/CWD as compatibility and restart hints if state corrupts later,
   without granting provider-deletion authority. Tombstone writing and the
   executor-owned restart scan publish only a SID-less SessionManager hint; a
   regression forges state/tombstone victim SIDs and proves the hint becomes live
   while the provider-deletion set remains empty. The same scan matrix proves a
   retained state row is rejected when SID or conversation owner mismatches the
   protected generation, and that keep="false" is not retained. Another regression makes
   read_tombstone raise if the event-loop fallback calls it.
4. Readable missing-state keep prunes at the normal cutoff; explicit keep=True
   always preserves the folder for restart recovery until release or conversation
   TTL writes false. A barrier-controlled race proves promotion never blocks the
   event loop while prune owns arbitration: the first attempt returns retryable
   conversation_busy, then a retry after prune completes cleanup returns
   conversation_gone. An unsupported Claude Code cleanup regression proves prune
   preserves both the run folder and protected identity record, then reaps both once
   a cleanup implementation reports success. A legacy state/tombstone-only SID
   regression similarly preserves the lookup without invoking cleanup, then proves
   later protected publication makes reclamation safe. Parameterized hard-ceiling
   regressions prove both legacy-no-authority and unsupported-provider retry folders
   are reaped after 90 days without ever using untrusted SID data for provider cleanup.
   A held upstream off-loop writer lock
   is also retryable immediately; after release, promotion writes keep=True and the
   stale writer cannot roll it back. A synchronous off-loop promotion regression also proves
   update_state owns its lock without recursive acquisition or deadlock. Transient
   persistence errors and skipped state writes are
   retryable and cannot dispatch; retry also preserves any SessionManager/TTL
   ownership that existed before the attempt. Continuations use the original
   owner's readable keep directly; if the later combined identity/retention state
   write fails, the cleanup generation restores the missing owner key and intent,
   and a subsequent readable owner release still wins. Unreadable owner intent receives
   bounded 24-hour grace.
5. Missing, malformed, deeply nested, Unicode-invalid, or non-object state is
   unreadable; the SessionManager fallback preserves tombstone-identified material
   until prune's bounded 24-hour grace expires. A regression records acquisition-time
   keep=False, promotes state to true, corrupts state, and proves the stale false
   generation remains unknown for grace rather than immediately deleting resume data.
   Another forges a conflicting tombstone SID and proves latest protected keep=True
   still wins without provider cleanup or folder removal. Readable legacy continuations use
   their resolved state SID when the original owner is unreadable and the old
   tombstone has no SID. Malformed continuation owner IDs are bounded as unknown
   retention per entry; a sorted-first malformed record cannot abort pruning of a
   valid stale record later in the same sweep. A deeply nested sorted-first
   tombstone is likewise isolated so the valid next entry still prunes. String,
   NaN, infinite, future, and oversized tombstone
   death values use bounded file-mtime fallback rather than aborting or evading
   prune; if both death time and mtime are future-dated after wall-clock rollback,
   the current sweep time preserves unknown-retention grace instead of authorizing
   immediate cleanup; exact cutoff equality remains eligible on coarse-resolution
   filesystems.
6. Readable state promoted to keep=True after tombstone creation retains the
   session.

Validation evidence (the long persistence suites were not rerun during the latest recovery):

• Earlier composed candidate: test/test_subagent_persistence.py test/test_subagent_continuable.py test/test_session_cleanup.py test/test_session_sharing.py test/test_continuable_followups.py test/test_jsondecodeerror_redundancy_ratchet.py: 211 passed
• Earlier composed candidate plus upstream test/test_subagent_state_write_serialization.py, untouched test/test_subagent_coverage.py, test/test_subagent_context_group_plumbing.py, test/test_app_spawn_capability.py, the two upstream base-regression suites, and the exact ACP stale-turn test: 621 passed
• Latest rebased review-fix head: affected continuation, cleanup, sharing, serialization, security, fix(subagent): take a run's three state.json writes off the event loop #7557 integration, repaired IRQ regression, facade-compatibility shards, unreadable protected-record retention, forged-tombstone/sidecar isolation, SID-less hint isolation, fail-loud trust ACLs, stale-false promotion grace, provider-cleanup retry preservation, all repaired trusted-publication fixtures including invalid-time Windows cases, legacy SID lookup preservation, bounded unreclaimable cleanup retries, protected restart-registry authorization, injected trusted-reader compatibility, literal-boolean retention, cancellation-drained identity durability, protected-read failure preservation/containment, test-root isolation, shared parse/schema resilience, tombstone-independent retention fallback, protected-owner/SID forgery containment, and future-clock unreadable grace: 159 passed
• Black baseline, isort --check-only src/kiro_crew test, flake8 src/kiro_crew test, agent-SDK boundary, docs lint, and git diff --check: passed
• mypy src/kiro_crew/: passed (1,264 source files)
• Earlier local GPT 5.6 and Opus 4.8 consequence-chain reviews: PASS; current-head server reviews rerun after publish

Manual verification

N/A — this backend persistence invariant is covered through live capture,
abnormal-exit tombstone creation, provider-specific metadata, and the real stale
prune path.

Screenshots / video

N/A — backend-only persistence change with no rendered UI delta.

Pattern harvest

Rule candidate: semgrep
Pattern: synchronous threading.Lock acquisition in an async/event-loop call path
Rationale: event-loop paths must use non-blocking acquisition or offload blocking work.

Related Issues

no linked issue: this implements item 7c of #6292 but does not close the
umbrella issue.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (subagent system spec synchronized with cleanup and retention behavior)
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

N/A — the repository template does not currently provide CLA attestation wording.

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 78fb3a9b74f0cfb24180d039ec2a02859877da3f via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 78fb3a9

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 78fb3a9b74f0cfb24180d039ec2a02859877da3f via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound fix for a real transcript leak, but the deletion-authority machinery lands with fragile trust seams and a retention inversion humans should weigh.

Watch

• prune_stale_tombstones grew from ~20 to ~180 inline lines of interacting flags (retention_unknown, fallback_sid, claim_agent_id, hinted keys) deciding whether to delete private data. This is the highest-consequence path in the PR and the hardest to re-verify after the next edit; only _should_defer_tombstone_cleanup was extracted.
• Trust by function identity: _scan_keep_states_impl skips the trusted-generation requirement when read_state is not self._persistence.read_state. Any embedder or test that injects a reader silently disables the forged-state protection this PR exists to add — the security posture depends on whether a module attribute was monkeypatched.
• Retention inversion on the unsupported-provider arm: _cleanup_session_files_sync still has no route for Claude Code, now returns False, and prune preserves the run folder (containing result.txt/task data) for 90 days instead of reaping at 7 — extending private-data retention 13x while waiting for a cleanup implementation that does not exist ("a future provider cleanup implementation … can retry").
• Continuations that previously proceeded after a swallowed promotion failure now return retryable conversation_busy on lock contention or transient persistence errors — a new user-visible failure mode on a working path; the safety argument is plausible but the caller-experience change is undocumented in the spec's user-facing terms.

Suggestions

• Replace the read_state is … identity probe with an explicit trusted-reader flag (constructor/attribute), so opting out of the canonical reader is a deliberate declaration rather than an inference.
• Extract prune's retention/ownership resolution into one pure function returning a decision record, matching the _should_defer_tombstone_cleanup pattern, so the deletion matrix stays independently testable.

[DESIGN-REVIEWED] 78fb3a9

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 78fb3a9b74f0cfb24180d039ec2a02859877da3f via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All verification is done. The core fix is real and well-anchored (the trust/ root is genuinely on the sensitive-path floor at security.py; the base's prune really did trust the agent-writable ts.get("session_id") for deletion). The counted findings: the injected-reader branch in _scan_keep_states has zero non-test consumers, and the same identity now lives in seven places.

First-Principles-Verdict: CONCERNS

The identity-capture fix is sound and boundary-derived, but it tows a retention-arbitration subsystem, and one production branch exists only so test monkeypatches stay green.

What this change ships

Intent: stop failed subagent runs from leaking or wrongly deleting provider transcript files — a FIX.

1. Abnormal-exit tombstones now identify transcripts from acquisition-time capture — justified (the fix)
2. Provider-deletion authority moved to an owner-only trust/ record; forged run-folder SIDs can't delete another run's files — justified (base trusted agent-writable ts.get("session_id"))
3. Unreadable state now defers deletion with a 24-hour grace — justified (declared safety property)
4. Failed/unsupported provider cleanup now retries, keeping folders up to a new 90-day ceiling — rides along
5. Continuations can now fail with a retryable conversation_busy promotion error — rides along, new user-visible failure mode
6. Only literal keep is True retains; truthy strings no longer do — changed default, rides along
7. Invalid or at-cutoff tombstone timestamps now prune via mtime fallback — rides along
8. Malformed conversation-registry keys dropped instead of releasing garbage — rides along
9. Shared-session PID write failure now swallowed, reversing the base's documented propagate decision — changed default
10. Restart keep-scan trusts a monkeypatched state reader via a production identity check — zero non-test consumers
    (More than 10; kept the most visible.)

Watch

• _scan_keep_states_impl's if read_state is self._persistence.read_state branch: grepped subagent\.read_state — 0 injectors in src/, 10 in test/. The "embedders" claim is unsupported, and the branch turns agent-writable state into trusted identity exactly when patched — inverting the PR's own authority model.
• The same run identity now lives in 7 places (state.json, tombstone top-level, tombstone cleanup_identities, _LIVE_CLEANUP_IDENTITIES, _LIVE_CLEANUP_HINTS, the trust/ sidecar, info._session_*); every future writer must keep them coherent — that is the permanent cost of this shape.
• The promotion-vs-prune race predates this PR; fixing it here adds a second per-agent lock registry (_RETENTION_LOCKS) beside _STATE_LOCKS plus the user-facing retryable error — declared as "part of the cleanup fix", but it is a distinct pre-existing defect riding on the identity fix.

Subtractions

• Delete the injected-reader fallback in subagent_manager/continuation.py _scan_keep_states_impl (the identity check plus the synthesized trusted_identity dict) — 0 non-test injectors counted; update the ~10 test patches of kiro_crew.subagent.read_state instead.
• Fold _RETENTION_LOCKS/_RETENTION_LOCKS_GUARD (subagent_persistence.py) into the existing _STATE_LOCKS registry: promotion already probes _STATE_LOCKS, and prune holding it through cleanup yields the same arbitration with one registry; the only extra blockers are a live owner's off-loop writers, which already block on their own fsync.

[FIRST-PRINCIPLES-REVIEWED] 78fb3a9

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 78fb3a9b74f0cfb24180d039ec2a02859877da3f via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 78fb3a9

[RohanK6]

fixed — The original session_id snapshot could delete retained resume material when state.json became unreadable. Final SHA 4733d9f6b77fd1d3c41d7f92e695a170b971f5aa keeps retention authority in current readable state and fails safe to retention when state is unreadable; it does not persist a stale tombstone keep snapshot. The table-driven regression proves partial non-retained state cleans via the tombstone identity, corrupt state retains, and a post-tombstone keep=True promotion retains.

[RohanK6]

fixed — The tombstone is now self-sufficient only for immutable session identity, not retention policy. Final SHA 4733d9f6b77fd1d3c41d7f92e695a170b971f5aa reads keep from live state so later promotion wins, and defaults unreadable state to retention. A three-case truth-table regression covers partial-readable cleanup, corrupt-state retention, and promoted-state retention.

[RohanK6]

fixed — Copying session_id from persisted state could not close the window where that same state write failed. Final SHA e73461310e6456cf8a5caec5aadba5beabcac32b captures the live client identity into SubagentInfo before the fallible state update and passes it to abnormal-exit tombstones only when non-empty, preserving persisted fallback behavior for early exits. Tests prove live capture, timeout tombstone propagation without persisted identity, partial-state cleanup, corrupt-state retention, and post-tombstone keep=True retention.

[RohanK6]

• fixed — dead assertion: Final SHA c41527c64c0c888c1470aca0e2dcf2a8bfa9b35e branches directly on isinstance(state, dict) and removes the redundant production narrowing assertion while preserving mypy-clean types.
• fixed — title scope: The PR title is now fix(persistence): preserve subagent cleanup and retention, covering both cleanup identity and the declared bounded-retention behavior.
• rebutted — remove timestamp ordering: Readable-owner-always-wins caused the prior current-head GPT blocker: stale owner keep=False immediately deleted a newer retained continuation. The 30-day cap bounds over-retention but cannot prevent that immediate data loss. The timestamp comparison and regression are therefore required for correctness.
• rebutted — retention rules lack a concrete defect: Base prune removed keep=True identity folders needed for restart rebuild while leaving provider files unreclaimable; promotion/release writes are explicitly best-effort. The bounded grace and cap close those concrete leak/destruction paths without creating unbounded retention.

[RohanK6]

• fixed — retention arbitration clock: Final SHA 83cbb58a76e424b533e1c1e683a9fdc2e658b715 stamps keep_updated_at only when the explicit keep value changes. Unrelated PID, turn, reconciliation, and idempotent keep writes leave that clock untouched; owner/continuation arbitration uses it, with updated_at only as a legacy fallback. Regression coverage proves a later unrelated owner PID write cannot erase a newer continuation promotion, while a later release still wins.
• fixed — invalid timestamp failure direction: Missing, malformed, infinite, future, and oversized keep clocks now fall back to finite, non-future tombstone died under the same absolute 30-day cap. Recent resume material survives clock corruption; a 31-day tombstone still reclaims it. The six affected suites pass 191/191, and repository-backed GPT 5.6 and Opus 4.8 both passed the exact SHA.

[RohanK6]

• fixed — tombstone format ownership: Final SHA 22ab5ff5d68724118426b3c097a508c7c687aa24 adds read_tombstone in subagent_persistence.py, using _agent_dir validation and (OSError, ValueError) handling for missing/corrupt/non-object data. subagent.py no longer imports or parses JSON directly; _keep_recorded_on_disk consumes the shared helper. Direct regression coverage pins all four shapes.
• rebutted — four-site model requires a wider redesign: The coupled behavior is intentional and now has one authoritative spec plus focused tests for each boundary: readable-state authority, unreadable tombstone-SID exemption, bounded prune grace/cap, and keep-decision ordering. Replacing that model would widen this defect fix into persistence architecture work without a concrete failing path.
• rebutted — make the 30-day cap configurable: The cap is an absolute leak-safety backstop for failed demotion writes. Making it operator-configurable could restore unbounded retention and weaken the invariant this PR adds; per-cause tombstone TTLs remain configurable independently. The six affected suites pass 192/192, and repository-backed GPT 5.6 and Opus 4.8 passed the exact SHA.

[RohanK6]

fixed — Final SHA 6fca2ec874ce122b761d1cf3d56e76cca934e9e4 separates retention arbitration from best-effort provider cleanup. Any unexpected state/owner/clock/verdict exception now logs and continues, preserving the identity folder and provider files. Cleanup runs only after arbitration explicitly permits deletion; cleanup failure still cannot block rmtree. An injected-decision-failure regression proves zero cleanup and folder survival.

[RohanK6]

fixed — Final SHA 6fca2ec874ce122b761d1cf3d56e76cca934e9e4 refreshes keep_updated_at on every explicit keep write, including repeated keep=True promotion during active continuation use. Unrelated PID/turn/reconciliation writes still leave the retention clock untouched, so the 30-day cap measures idle age rather than age since first promotion. The six affected suites pass 193/193, and repository-backed GPT 5.6 and Opus 4.8 passed the exact SHA.

[RohanK6]

fixed — Final SHA aa197ec3a488aaa84863611c885370e52f4796a7 routes prune_stale_tombstones through the same read_tombstone helper used by the SessionManager fallback. Missing, corrupt, hostile-ID, and non-object tombstones return None and are skipped; the same-job inline json.loads and JSONDecodeError handler are gone. The six affected suites pass 193/193, and repository-backed GPT 5.6 and Opus 4.8 passed the exact SHA.

[RohanK6]

rebutted — configurable 30-day cap: _RETAINED_STATE_MAX_AGE_SECS is an absolute leak-safety backstop for failed demotion writes, unlike the ordinary delivered-result TTL. Making it operator-configurable could restore unbounded retained provider material and weaken the invariant this PR adds. Active continuation use refreshes the dedicated keep clock, so the fixed cap now measures idle age rather than deleting actively used material.

[RohanK6]

• fixed — legacy clock transition: Final SHA 4421f5f034d8d89ec7d9ae6c333eceddb62e54e1 no longer uses generic updated_at for owner/continuation ordering. A readable owner with no keep remains authoritative non-retention; an explicit legacy keep without a valid dedicated clock becomes bounded-unknown and receives the 24-hour grace. Unrelated PID/reconciliation writes therefore cannot promote a stale legacy owner over a post-upgrade continuation.
• fixed — bounded arbitration errors: Decision exceptions preserve only while tombstone died is within the absolute 30-day cap. After cap expiry they warn, recover cleanup identity from the tombstone, perform best-effort provider cleanup, and remove the folder. Regression coverage proves both preservation and expiry reclaim. The six affected suites pass 194/194, and repository-backed GPT 5.6 and Opus 4.8 passed the exact SHA.

[RohanK6]

fixed — Final SHA c80c31056f238c409a0cafd104dfccb10626747c removes owner/continuation keep-clock comparison entirely. Readable original-owner state is the single continuation retention authority (true retains under the idle cap; false or missing reclaims); unreadable owner state receives bounded grace. The two synthetic ordering tests and persisted arbitration surface are removed, while keep_updated_at remains only the retained-run active-idle clock.

[RohanK6]

fixed — Final SHA c80c31056f238c409a0cafd104dfccb10626747c adopts the single-authority design: continuation pruning directly uses readable original-owner keep, with bounded grace only when the owner is unreadable. The nested wall-clock reconciliation ladder and migration semantics are gone. Tombstone died is also normalized once for every cutoff/grace/cap path, with hostile values using validated file mtime or zero. The specified six suites pass 197/197, and repository-backed GPT 5.6 and Opus 4.8 passed the exact SHA.

[RohanK6]

fixed — Readable keep=True now defers disk prune unconditionally; release/TTL remains the only authority that writes false. The racy 30-day keep clock/cap and keep_updated_at are removed. The remaining false→true boundary is serialized: prune creates a durable owner-side .prune-claimed under the same short lock used by promotion, and a losing continuation returns conversation_gone before dispatch. A barrier-controlled regression proves prune cannot act on stale keep=False after promotion. The established affected suites pass 198/198; adding untouched test_subagent_coverage.py passes 445/445.

[RohanK6]

• fixed — remove keep clock/cap: readable owner keep=True is unconditional; release/TTL owns demotion. The clock helpers, cap branches, and obsolete tests are removed.
• fixed — root-cause readers: read_state and update_state catch OSError, ValueError (including Unicode/JSON failures), and RecursionError, reject non-object data, and have direct Unicode/depth/non-object coverage. read_tombstone has the same depth hardening. The broad arbitration-insurance branch/test and SessionManager broad wrapper are removed.
• fixed — sweep isolation: malformed owner IDs and deeply nested sorted-first tombstones cannot abort later valid entries.
• rebutted — messaging parser unification: that handler deliberately performs sensitive-path gating and off-loop reads via asyncio.to_thread; replacing it with the synchronous persistence helper would drop those constraints. The persistence/prune/SessionManager call sites do share read_tombstone.

[RohanK6]

fixed — Promotion now acquires _RETENTION_ARBITRATION_LOCK non-blocking, so prune-side state/claim/fsync latency cannot freeze the event loop. Contention returns retryable conversation_busy without dispatch; a barrier regression proves immediate refusal while prune owns the lock and conversation_gone only after the durable claim commits. Real update_state(False) and transient OSError are also retryable. Exact-SHA GPT/Opus pass; 490 regression tests and all gates pass.

[RohanK6]

• fixed — transient write errors: skipped/unreadable writes and OSError now return retryable conversation_busy, not terminal conversation_gone, and never dispatch.
• fixed — ownership preservation: retry snapshots and restores exact pre-attempt SessionManager and TTL-registry state, so contention cannot unmark an already-retained conversation.
• fixed — single-process invariant: code/spec now name single-gateway arbitration; promotion is non-blocking.
• fixed — parser drift: prune, disk fallback, registry rebuild, and TTL sweep share subagent_id_from_conversation_key; malformed registry keys are safely dropped with regression coverage.

[RohanK6]

• retained with evidence — durable claim: the separate O_EXCL+fsync claim is the cleanup commit record. It survives gateway crash and unrelated state.json rewrites; replacing it with an in-memory set would reopen cleanup-owned resume material after restart before prune retries. Exact-SHA GPT and Opus verified the crash-consistency consequence chain.
• retained with evidence — state_writer seam: after current main extracted continuation logic into a rebound component, injecting the facade update_state preserves established monkeypatch contracts while the real production call remains the single default writer. Untouched coverage and component-ratchet suites pass in the 490-test set.

[RohanK6]

• fixed — event-loop blocking: both cleanup-identity sidecar writes now run through await asyncio.to_thread(...).
• fixed — leaked test threads: the promotion barrier test signals and joins both threads in finally, then asserts neither remains alive.
• fixed — shared persistence fallback: shared-handle identity is captured first; sidecar OSError is logged/contained and cannot trigger dedicated fallback or abandon the handle.
• fixed — import policy: persistence dependencies are module-scope imports captured in method defaults, which survive component global rebinding without function-local imports or facade changes.

Exact-SHA GPT/Opus pass; 491 tests and all gates pass.

[RohanK6]

• fixed — precedence drift: _merge_cleanup_identity_records is now the single normalization, SID-deduplication, and richer-field precedence owner used for sidecar writes, tombstone assembly, and prune extraction.
• retained with evidence — sidecar plus tombstone snapshot: the sidecar survives deliberate tombstone clearing and restart; the tombstone snapshot independently preserves an in-memory generation when a sidecar write fails but tombstone persistence succeeds. Tests cover transient SID1 failure, SID2 success, tombstone clear, memory reset, restart, and cleanup of both generations.
• fixed — concurrency invariant: promotion is non-blocking on the event loop; prune remains executor-side and the spec/module comment names the single-gateway contract.

[RohanK6]

• fixed — cleanup-lock stall: event-loop identity snapshots acquire non-blocking and use the writer-published in-memory generation on contention. The writer now publishes before any durable read; a barrier regression proves a tombstone sees the SID while the worker is blocked pre-read.
• fixed — import policy: all new persistence imports are module-scope and captured in method defaults across component global rebinding; no local cast remains.
• fixed — shared error path: offloaded sidecar OSError is logged/contained and cannot trigger dedicated fallback.
• fixed — thread hygiene: barrier tests release and join every worker in finally.

Exact-SHA GPT/Opus pass; 495 tests and all gates pass.

[RohanK6]

• fixed — facade contract: _promote_conversation remains truthfully -> None; the coordinator stores the tri-state result in its own slot and the sole synchronous caller reads it immediately. No cast or return-type lie remains.
• fixed — precedence ownership: _merge_cleanup_identity_records is the single SID normalization/dedup/enrichment function used by sidecar writes, snapshots, and prune extraction.
• fixed — in-memory lifecycle: successful prune and explicit folder deletion evict _LIVE_CLEANUP_IDENTITIES; regression coverage pins both deletion and lock-contention snapshot behavior.
• retained with evidence — sidecar plus tombstone: the sidecar survives deliberate tombstone clearing/restart, while a tombstone snapshot independently preserves a writer-published generation during sidecar failure. Both failure directions are covered.

[RohanK6]

• fixed — concurrent outcome isolation: the coordinator now stores promotion outcomes in threading.local, preserving the truthful None facade while preventing concurrent callers from clobbering CLAIMED/RETRYABLE. A deterministic two-thread rendezvous regression forces both outcomes to be stored before either caller reads back and proves distinct conversation_gone/conversation_busy results.
• retained with evidence — identity schema ownership: subagent.py remains unchanged to satisfy the enforced agent-SDK boundary ratchet; the run component uses typed locals and persistence owns the durable schema. No untyped reader depends on the dynamic diagnostics fields.
• fixed — precedence and lifecycle: one merge helper owns SID normalization/dedup/enrichment; nonblocking snapshots, prune, and explicit folder deletion all have lifecycle regressions, and successful deletion evicts the in-memory fallback.

Exact-SHA GPT/Opus pass; 497 tests and all gates pass.

[RohanK6]

CI rerun requested: Backend Tests (3.10, 1) failed only in test/test_acp_client.py::TestAcpClientStaleTurnOracleGate::test_real_oracle_movement_defers_via_fake_proc (Stale turn detected timing assertion). The exact test passes locally (1 passed), all other substantive CI jobs passed, and Coverage/PR Readiness failed only downstream. Fork authors cannot use gh run rerun --failed (Must have admin rights to Repository). Please rerun failed jobs on run 33374911521.

[RohanK6]

This is ready for review – all AI analyzers pass

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• This PR is OVERLAPPING with PR #7195. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #6713: KEEP. Different user goals with no behavioral collision; only ordinary rebase friction in run.py and subagent.md. Files: src/kiro_crew/subagent_manager/run.py, docs/system-specs/modules/subagent.md.
• This PR is OVERLAPPING with PR #8003. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #6713: KEEP. Neither PR covers the other's goal, but both rewrite the same off-loop state-writer/retention-hold machinery in run.py. Agree a landing order and have the second PR rebase, since 8003's submission-time hold interacts with 6713's non-blocking _STATE_LOCKS probe in promote_retention. Files: src/kiro_crew/subagent_manager/run.py, src/kiro_crew/subagent_manager/continuation.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

[RohanK6]

Thanks @bolichen97 – will keep this open per your suggestion. Confirming that this is ready for review

[bolichen97]

The security property this hinges on is right and is the part I checked hardest: provider-deletion authority comes only from the protected record under the file-gated trust/ root plus synchronous live gateway publication, while the agent-writable tombstone.json/state.json SID and cleanup_identities fields are demoted to display and retention hints — so a forged victim SID (or a planted legacy cleanup-identities.json in the run folder) cannot widen prune's deletion set, and there is a regression pinning exactly that. _protect_cleanup_identities_path correctly re-locks the existing file rather than only its parents, since tightening a parent does not retrofit a Windows file DACL, and it is fail-loud on both the read and the write path. _read_cleanup_identities_file treating only FileNotFoundError as empty is the right call — a parse or schema error propagating is what stops a later append from rewriting unreadable history as a fresh single generation. Concurrency checks out: publish_live_cleanup_identity deliberately skips _CLEANUP_IDENTITY_LOCK and relies on list.append atomicity so the event loop can publish before a queued to_thread is cancelled, _live_cleanup_identities uses a non-blocking acquire, prune runs under maintenance_executor() so its blocking _acquire_retention_locks is off-loop, and the two lock orders (retention -> cleanup-identity in prune, retention -> state in promotion) form no cycle. _remember_identity_off_loop shields and fully drains before re-raising, so restart cannot precede the authority record. Two things I looked at and accepted as deliberate rather than defects: _cleanup_session_files_sync now returning False for non-kiro providers keeps those run folders for 90 days instead of dropping them at 7 (metadata only, bounded, and the alternative is a permanently unreachable transcript), and the shared-path PID write becoming best-effort is the right trade since the old propagating behavior orphaned a live shared handle — except Exception still lets CancelledError through. One non-blocking nit for a follow-up: the subagent.py facade still declares _promote_conversation -> None while it now forwards RetentionPromotionResult, papered over with # type: ignore[func-returns-value]; the declared type is now inaccurate even though the call site guards on an explicit RETRYABLE.