Patch session metadata atomically by stable ID

[schickling-assistant]

Summary

• separate immutable session identity from mutable, non-unique display names
• add exact-ID-only atomic metadata patching for display names and tags
• preserve unknown/recovery metadata fields and emit full-fidelity metadata_change records
• validate display names consistently across the API and CLI
• generation-guard GC flapping and pty exec so they cannot overwrite a concurrent patch or same-ID replacement
• make orphan and abandoned GC reaps prove ownership before signaling and report every safe skip

Contract

pty metadata patch --id <stable-id> reads one merge-style JSON object from stdin:

{
  displayName?: string | null
  tags?: Record<string, string | null>
}

IDs are immutable and remain the authoritative lookup key. Display names are presentation metadata: they may be duplicated and changed without restarting the session. Name-based lookup succeeds only when the name resolves to one live session; ambiguous names fail and list the candidate IDs.

Display names must be non-empty, already trimmed, at most 160 Unicode scalar values, and contain no control characters or U+2028/U+2029. Slash and backslash are allowed.

Coherence boundary

The exact metadata patch operation, GC flapping transition, and pty exec command update acquire the event lock before the metadata lock and publish from the current generation. Orphan and abandoned reaps use two generation-checked leases: one before signaling and one after daemon shutdown/flush before cleanup. Busy, stale, failed-signal, and shutdown-timeout outcomes are reported without claiming a reap succeeded.

The metadata file remains authoritative. Lifecycle events and metadata are not generally a cross-file transaction: for example, session_exit may publish before its exit metadata while that metadata lock is busy. A process crash or I/O failure between metadata and event files can still omit a notification because this change does not introduce a journal.

pty exec children receive an opaque generation owner token. Sessions started by an older build fail closed and need one restart before using pty exec; no compatibility fallback can safely prove ownership.

Validation

Exact combined-tree aarch64-linux proof at 0deb3f0:

• TypeScript build and typecheck passed
• 8 focused metadata/event, atomic-write, spawn, GC, generation-race, and exec test files passed
• 117 of 117 focused tests passed

Earlier exact macOS proof against upstream d5fabc3:

• 15 of 16 test files passed
• 260 of 261 tests passed
• the sole failure was the attach-stream free-form completion test in tests/completions.test.ts; it reproduced unchanged on the exact d5fabc3 baseline and is inherited

The branch was merged with upstream main in 2cc286e, followed by lifecycle hardening in 706e4f5 and review corrections in 0deb3f0.

Part of compoundingtech/st2#128.

[myobie]

Independent review at exact 706e4f5 found two blockers; please keep this draft.

1. Busy GC is not currently skip/report safe. The orphan and abandoned passes send SIGTERM before acquiring the cleanup lease. If a live metadata lock then blocks cleanupAll, the daemon has already been terminated, its metadata remains, and GC reports nothing. Acquire the event + metadata cleanup lease before signaling, or explicitly report the partially acted-on busy session. Add a regression that asserts process state and result reporting, not only retained metadata.

2. GC flapping and cmdExec still use unlocked stale-snapshot readMetadata/writeMetadata followed by a separate event append. They can discard a concurrent exact-ID patch or write into a replacement generation. Route both through the locked full-record mutation, with expected-generation protection where an owning daemon is involved, and cover concurrent-writer plus same-ID replacement races.

The narrow patchMetadataById boundary is accurately documented: lock contention cannot split an effective patch, while crash/I/O between metadata and event files can omit the notification because there is no journal. The broader lifecycle-serialization claims are not yet true—session_exit can publish while exit metadata is still blocked, as the new test itself demonstrates. Narrow those claims to the exact patch operation unless the remaining lifecycle paths become coherent.

After correction, please rerun the focused metadata/event, atomic-write, spawn, GC, and race regressions. The previously reported completion failure is isolated and is not a blocker.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 706e4f5e4c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[schickling-assistant]

Addressed the independent review blockers in 0deb3f0:

• GC orphan/abandoned reaps now prove ownership before signaling, wait for daemon shutdown/flush, revalidate the generation before cleanup, and explicitly report every skip. Busy-lock regressions prove the daemon remains alive and unreported as reaped.
• GC flapping now merges into the current locked record and emits under the event lease; concurrent presentation changes survive, while replacement generations are untouched.
• pty exec now uses a per-generation child owner token, mutates the current locked record, and emits coherently. Concurrent-writer and same-ID replacement regressions prove it cannot overwrite or execute against the wrong generation.
• The PR description now narrows the coherence claim to these corrected operations and explicitly states that lifecycle event/metadata publication is not generally transactional.

Validation on the exact head: TypeScript build + typecheck passed; 8 focused files and 117/117 tests passed on aarch64-linux. The PR remains draft.