[schickling-assistant]

Problem

Replacing identity.toml by hand does not provide a durable prepared identity, guarded retries, or a restart barrier proving that the new NodeID is live. Switching before peers trust the replacement can strand a consumer.

Goal

Implement Nathan's #338 direction: prepare → publish over the old identity using the atomic expiry batch in #340 → activate. There is no dual-identity serving, no request-ID protocol, and no changes to consumer orchestration outside Fabric.

Decisions

• identity.toml remains the only active-key authority. Preparation publishes a synced 0600 pending file; activation retains a private retired copy and replaces the authoritative key by one rename under a nonblocking cross-process identity lock.
• Guards are public NodeIDs. An already-active retry does not rewrite the key or recapture state; it only reconciles daemon readiness. Native supervisors retain ownership; unsupervised homes directly spawn a replacement, including recovery when no daemon remains.
• Before the rename, the live daemon fences exposure mutations, snapshots only ephemeral declarations and stops completely. Activation holds the daemon startup lease across the rename, then releases it before restarting; a concurrent up cannot load the retired identity in the stop/swap gap. A failed stop leaves the home prepared rather than swapping beneath an old serving endpoint. Replacement startup consumes the one-start handoff independently of the CLI's survival; persisted exposure configuration remains authoritative.
• The live daemon reports runtime-peers mode in its IdentitySnapshot reply; this overrides any stale marker. A durable marker records successfully initialized serving state for crash recovery, not attempted startup options. The existence of an old runtime policy file never implicitly enables grants after a crash.
• Full daemon teardown retires tunnel sessions, mux connections, and queued dials before a replacement becomes ready. Persisted dial declarations are also removed rather than silently re-establishing old local-identity consumers.
• Sync author identity is the NodeID byte-for-byte. Its existing companion transition stops the old-author engine and reopens the same sync history/files under the new author.
• The local home cannot read remote expiry records. Retired secrets therefore use the conservative upper bound: activation + 600 seconds. Running-daemon cleanup and subsequent identity commands remove elapsed retired files; an offline home is cleaned when next started.

Verification

Review repair head: a8fc2f5c6ec5f68518fc557357fee9445d530cde. Independent reviewer FabricRekey338.IdentityReviewDelta returned GO at this exact head before push (source review only). The new #340 fixes were merged normally, then Nathan’s owner-rebased #341 and 0.9.3 release metadata were integrated by a second ordinary merge; no force push. #340 is already on main at fbf74b700ed10080e2ca1ec50cbf19b205af59a7.

New review regressions:

• Foreground liveness/lifetime and stale-socket crash recovery; failed pre-swap write restores current daemon; live OFF mode defeats stale ON marker: identity_foreground_activation_failed_swap_and_stale_socket_recovery_keep_runtime_off.
• Recovery stays identity-locked on stop/write failures: pre_rename_recovery_retains_identity_lock_on_stop_and_write_failures.
• Bounded coded waits: identity_control_wait_is_bounded_and_coded, identity_native_command_timeout_is_bounded_and_coded.
• Abort handoff removal/unfence: identity_snapshot_abort_removes_handoff_and_unfences_exposures.
• Cleanup/corrupt pending recovery: cleanup_errors_and_corrupt_pending_do_not_block_status_or_prepare.
• Exclusive unique scratch and guard recheck: private_scratch_writes_do_not_follow_the_old_shared_symlink, activation_rechecks_pending_guard_after_preservation.
• Fresh daemon/readiness and completed teardown: identity_reconcile_waits_for_fresh_daemon_before_shutdown, identity_activation_waits_for_an_already_stopping_foreground_node, identity_foreground_stop_waits_for_lifetime_lease_after_socket_refusal.
• Missing native executable retains standalone ownership: identity_missing_native_tool_preserves_uninstalled_ownership.

Local commands all run one at a time inside gate-slot --class heavy, with CARGO_TARGET_DIR=/home/schickling/.cache/fabric-mesh-cargo/fabric and nix develop -c:

• At clean 8e532d0 (same repaired code before the owner metadata merge): cargo test --locked -p fabric --lib identity::tests -- --test-threads=1 14 passed; --lib service::tests::identity_ 2 passed; --bin fabric identity_ 4 passed; --test json_output identity_ 3 passed.
• At clean final a8fc2f5: cargo test --locked -p fabric --bin fabric identity_ -- --test-threads=1 4 passed; cargo test --locked -p fabric --test json_output identity_ -- --test-threads=1 3 passed.
• Hosted CI at this head: test in progress; Nix in progress, observed once after push. No watches or frequent polling. No green-CI claim for the new head yet.
• The task-owned Cargo target directory was removed after the checks (cargo clean: 5.4 GiB); its parent directory is empty.

Deferred #343 work was not added to this PR. The two “small if feasible” items were both implemented here: live-daemon runtime mode in the IdentitySnapshot reply plus OFF-stays-OFF coverage, and a short Status readiness poll before stopping a newly starting daemon.

Complexity

One identity module owns key state, guard/lock, retirement and restart recovery. Existing daemon teardown, service ownership, peer persistence and sync-author transition remain authoritative; no second endpoint or parallel identity policy is introduced.

Concerns

Re-keying intentionally interrupts old transport sessions. The caller must publish the prepared NodeID before activation; Fabric cannot prove that every remote peer has installed its batch.

Open questions: none. Nathan confirmed activation + 600 seconds as the retired-secret upper bound and retiring persisted dial declarations in his review.

Friction & bottlenecks

No measured runtime bottleneck claimed. Local checks are targeted and admission-gated. Review pins exact committed heads and scoped deltas; no uncommitted work is included in an attestation.

Follow-ups

Consumer-side publish/activation orchestration remains outside this repository and PR. No merge or auto-merge is requested; Nathan owns bottom-up review and merge.

References

Refs #338. Depends on #340. Direction: #338 (comment)

Posted on behalf of @schickling

field	value
agent_identity	agent/dotfiles/fabric-mesh
session	01a17e44
agent_persona	generalist
agent_supervisor	unavailable
agent_tool	OMP
agent_tool_version	18.12.5
agent_runtime	OMP 18.12.5
agent_model	anthropic/claude-opus-5-5
worktree	smalltalk/main

[myobie]

Review of #341 at 96c1b08 (stacked on #340), source review of the key state machine and the daemon restart path; I ran no builds.

Key state machine: no blockers. Every key file is created 0600 with create_new, so there is no umask race or symlink follow. The rename is the only authoritative step, the pending file stops existing at the rename, and I found no point where two keys claim to be active or identity.toml is partial. Guards compare public IDs derived from the key files on disk; already_active rewrites nothing. The lock is flock (released by the kernel on a crash), and the retired-file sweep matches the exact name pattern. No secret reaches output, errors, Debug or the golden files. The crash-point table gives prepared or active at every step. The answers to your two questions: deleting the retired secret at activation + 600 s is the intended upper bound (no remote expiry input; do not guess earlier), and retiring dial declarations and requiring fresh declarations is correct.

Required before merge:

1. up --foreground never takes the daemon lease, so activate swaps the key beneath a live old endpoint. The lease is acquired only in run_daemon_with_options (daemon.rs:3454); the foreground branch of up (main.rs:~1093) calls start_with_daemon_options directly. try_daemon_lease then returns Some in activate (main.rs:664), skipping IdentitySnapshot and Shutdown. reconcile_identity_daemon (main.rs:3124) has the same blind spot. The rename happens under the serving old identity, the readiness poll times out after 30 s with restart_failed, and every retry repeats it, with the old NodeID accepting connections throughout. Use Status as the liveness probe in both places (anything that answers is a running daemon: snapshot and shut down), not the lease alone. Add a test with a foreground node.

2. Pre-rename failure after the old daemon stopped leaves the node down. preserve stops the daemon (identity.rs:192), then activate_locked can fail (ENOSPC or EIO writing the retired or intent file, the 15 s DaemonStopTimeout). The error is a plain IO error or daemon_stop_timeout, the state is prepared, and the supervisors do not restart a clean exit. On any error after the stop and before the rename, run reconcile_identity_daemon against the current identity, best effort, and say in the error whether the daemon was restarted.

3. Bound the control waits. send_control ends in read_to_end with no timeout, and IdentitySnapshot/Shutdown in preserve (main.rs:666-667) and the first Status/Shutdown in reconcile (main.rs:~3121-3126) hold the identity lock. A wedged daemon hangs activate and every other identity command with identity_busy. Wrap each in a timeout that maps to a coded error. Same for the systemctl/launchctl calls.

4. Clean up after an aborted pre-swap. The exposure fence set by IdentitySnapshot is never cleared (daemon.rs:4545); if the CLI dies after the snapshot, every expose/unexpose returns identity_busy until restart. Retrying activate recovers, but say so in the error and in docs/errors.md. And remove identity.exposures when activation aborts before the rename: inherited_exposures checks only the next ID, so a later activation with no daemon resurrects a stale ephemeral list, and one invalid entry makes every start fail with restart_failed.

5. status and prepare must not die on cleanup or a bad pending file. The retired-file sweep uses remove_file(...)? (identity.rs:153), so a directory, EPERM or racing delete aborts status and prepare; log per entry and continue, ignoring NotFound. A corrupt identity.next.toml makes snapshot() fail forever (:60-68) with no CLI recovery: prepare must replace an unreadable pending file with a new one.

6. Small and cheap: the two scratch writers share identity.tmp (identity.rs:215-221, path.with_extension("tmp")) with truncate and no O_EXCL; use unique names with create_new. Re-check both guards right before the rename (preserve can take 15 s). Correct the README/docs: the identity lock is held through the restart (so status is identity_busy meanwhile), previous echoes the caller's value for already_active, and "existing restart path" is a new direct spawn.

Should fix here if small, otherwise #343: take the runtime-peers mode from the live daemon (return it in the IdentitySnapshot reply) rather than the marker file; the failed-launch test pre-removes the marker and passes trivially, so add the OFF-stays-OFF case. reconcile after the lease release can see a fresh daemon that holds the lease but is not serving yet and shut it down; poll Status for the expected ID briefly before shutting anything down.

The remaining items are in #343. Push these and I review only the delta; I merge #340 first.

[schickling-assistant]

1. Status liveness in activation/reconciliation and foreground lifetime lease: fb40b6a + b3bf4d9; identity_foreground_activation_failed_swap_and_stale_socket_recovery_keep_runtime_off.
2. Pre-rename failure restores current daemon best effort, reports outcome, and retains identity lock: fb40b6a + ebc9ac3; pre_rename_recovery_retains_identity_lock_on_stop_and_write_failures and the foreground test (fixture isolated in 8e532d0).
3. Bounded coded control/native-command waits: fb40b6a + b3bf4d9; identity_control_wait_is_bounded_and_coded, identity_native_command_timeout_is_bounded_and_coded.
4. Abort removes handoff and clears reachable exposure fence; abandoned-fence recovery documented: fb40b6a; identity_snapshot_abort_removes_handoff_and_unfences_exposures.
5. Cleanup errors no longer block status/prepare; corrupt pending key is replaced: fb40b6a; cleanup_errors_and_corrupt_pending_do_not_block_status_or_prepare.
6. Unique exclusive scratch files, final guard recheck, README/error/JSON corrections: fb40b6a; private_scratch_writes_do_not_follow_the_old_shared_symlink, activation_rechecks_pending_guard_after_preservation.
   Small items fixed here: live snapshot runtime mode/OFF-stays-OFF (foreground test); fresh-daemon Status poll (identity_reconcile_waits_for_fresh_daemon_before_shutdown). Existing Follow-ups from runtime expiry (#340) and identity activation (#341) reviews #343 items remain deferred; nothing new moved there.
   Merged your rebased cb4e01d normally in a8fc2f5; no force push. Independent exact-head GO; local 14 identity + 2 native + 4 binary + 3 JSON tests passed, final merged-head binary/JSON recheck 4+3 passed. New test CI and Nix CI are running; no watches.

Posted on behalf of @schickling

field	value
agent_identity	agent/dotfiles/fabric-mesh
session	01a17e44
agent_persona	generalist
agent_supervisor	unavailable
agent_tool	OMP
agent_tool_version	18.12.5
agent_runtime	OMP 18.12.5
agent_model	anthropic/claude-opus-5-5
worktree	smalltalk/main

[myobie]

Round-2 delta at a8fc2f5 reviewed (source review of every item in code, then of the new tests; no builds). No blockers. I merge when deterministic is green.

Verified fixed: the Status liveness probe in activate and reconcile (and the lifetime lease for up --foreground, same acquire and CLOEXEC as fabric daemon, so service units are unchanged); recovery with the identity lock held for every error between the stop and the rename; every wait on the path bounded and coded; the abort clears identity.exposures and the fence; the live snapshot's runtime mode with OFF-stays-OFF; per-entry sweep errors; unique exclusive scratch files and the guard recheck; the docs.

Four small things remain in the new code. I would like them in one follow-up PR before the next release, not in this one:

1. Recovery does not work for the daemon_stop_timeout case. Shutdown is acked and the 15 s lease wait expires while the daemon is still tearing down. recover_identity_activation then calls reconcile_identity_daemon(current): Status is refused, the lease is held, and after the 2 s poll it sends Shutdown, which returns DaemonNotRunning (main.rs:3199), so reconcile bails and nothing restarts the node when teardown finishes. On DaemonNotRunning from that Shutdown, fall through to identity_stop_lease and then the restart.
2. prepare leaves a secret behind on failure. The key scratch file .identity.next.<rand>.tmp (identity.rs:90-92, :251) is not removed when the sync or rename fails, and the unique names are never swept, so a crash or failure leaves an unreferenced private key on disk. Remove it on error and sweep stale .identity.next.*.tmp under the lock.
3. prepare replaces a pending key on any non-NotFound read error (identity.rs:62-68). EMFILE, EIO or EACCES on a valid pending key drops it silently. Replace only on parse or UTF-8 failure and propagate other I/O errors.
4. SIGINT, SIGTERM or SIGHUP between the Shutdown and the rename skip recovery (the activation path installs no handler, so an SSH drop leaves the node down with state prepared). Ignore or handle these for the window.

Also tracked in #343, not required: the warn! calls for sweep and corrupt-pending are invisible in the CLI (no subscriber); the fresh-daemon poll in reconcile is 2 s; recovery drops ephemeral exposures (document it); recover_identity_activation should use read_id, not load_or_create_identity; the several tests that cover only the helper or only the Status timeout.

[myobie]

Merged (main 4f63a18) at a8fc2f5 with all checks green. Next: the one small follow-up PR with the four items in my review (stop-timeout recovery, prepare scratch-key cleanup, replace-only-on-parse-failure, signals during the critical window). I cut 0.9.4 with #340, #341 and that follow-up once it is merged and the Linux gate has passed, so smalltalk should pin to the 0.9.4 tag, not main.