fix(session-core): stream id collisions, wire-status narrowing, and two vanished conventions

[Harry19081]

Summary

Four follow-ups from the test-suite audit in #879, each traced to a root cause rather than patched where it surfaced.

• A transcript-corruption bug: stream placeholder ids collided within a millisecond and silently merged two streams into one event
• Four more wire statuses force-cast into Session.status, same defect as the one fixed in test: repair dead test wiring, normalize placement, cover high-risk modules #879
• The frontend-ui-audit skill restored — and the .gitignore rule that made it disappear, fixed
• Test placement is now enforced, because documenting it held for less than a day

Problem

1. Stream ids collided. createStreamMessageId / createStreamThinkingId used Date.now() as their only nonce. The id keys an eventStoreProxy upsert, and upsert merges on id — so two placeholders minted for one session in the same millisecond were not two events. The second stream's text overwrote the first stream's row, collapsing two logically distinct streams into one transcript entry. Both call sites mint a placeholder only when the lane id is null, which is exactly what reset() leaves behind, so a fast turn boundary is the trigger.

2. Four unvalidated wire statuses. #879 fixed one instance in sessionSyncReconcile; four remained. Session.status drives sidebar grouping, Kanban lanes and every terminal-status predicate, and a value outside the union reached it.

3. frontend-ui-audit had no SKILL.md at either the user-global or workspace path, while CLAUDE.md and AGENTS.md route all UI-component work to it. Not cosmetic: 55 reports under docs/frontend-ui-audit-* between 2026-07-12 and 2026-08-22 carry a "skill was unavailable, manual fallback" note. Every UI audit for six weeks was improvised.

4. The placement convention regressed in under a day. #879 normalized 53 mixed directories to 0 and wrote the rule into CONTRIBUTING.md. Two directories re-mixed almost immediately, both from a commit that merged after the cleanup.

Solution

Stream ids — a counter, deliberately not a UUID

A module-level monotonic counter, zero-padded, appended after the timestamp.

compareChatEvents (snapshotMaterialization.orderMembership.ts:84) and the raw-transcript dialog (transcript.ts:37) both break createdAt ties with id.localeCompare. Same-millisecond placeholders share a createdAt, so the id is the chat ordering key. A UUID would randomize transcript order; a padded counter keeps lexicographic order equal to creation order. Zero-padding matters — unpadded, -10 sorts before -2.

The counter never resets, so uniqueness no longer depends on the wall clock advancing: a stalled clock or a backwards NTP step still yields distinct ids. This mirrors what Rust already does for its authoritative ids in agent-core's streaming module, whose own comment describes the same upsert-overwrite hazard.

Two constraints found and now pinned by assertions: the stream-msg-ts- / stream-think-ts- prefixes are load-bearing across the FFI boundary (session-persistence/src/crud.rs:26 matches on them to keep live-only rows out of the DB), and the id must stay colon-free or parseActivityId reinterprets it.

Confirmed by driving the real handler through delta → reset() → delta with the clock pinned: pre-fix it reproduces the exact signature the intermittent drops the live stream state test emits.

The four casts — graded by exposure

Site	What the value was
sessionSyncStateHelpers.ts:181	Completely ungated. runStatus is typed string; the only early return above fires on a different condition.
useNativeSessionStatusMonitor.ts:133	The widest door. Straight off the session-status-changed Tauri payload, written unconditionally for every session.
sessionSyncStateHelpers.ts:269	Runtime-safe only because a Set guard upstream happens to hold nothing outside the union.
cliTurnLifecycleCoordinator.ts:134	Same — plus a second, upstream cast at line 80 that was the real laundering point.

cliTurnLifecycleCoordinator is narrowed at its entry point so the terminal guard, the turn-lifecycle writes and the row write all see a validated value. Behavior is unchanged: an unknown status narrows to idle, neither running nor terminal, so it is ignored exactly as before — now with a warning.

installing maps to running at all four sites, consistent with RUNNING_SESSION_STATUSES and IN_PROGRESS_STATUSES.

On evidence quality: reverting the cast alone on the two guarded sites does not fail — the guard still blocks it. Rather than claim a bite that could not be shown, those tests assert union membership and go red when the guard is widened while a cast is present, which is how the defect would actually reappear. The three-way matrix (cast+widened guard = red, fix+widened guard = green) is in the commit body.

The skill — fixing the cause, not the symptom

The root cause is in .gitignore, not the file. .orgii/skills/* is ignored, and each vendored skill is re-admitted by an explicit pair of negation rules. frontend-ui-audit was never added to that allowlist, so it only ever existed as a local file on one machine and vanished with it, while its five siblings survived. This adds the missing pair, so it is tracked by the same mechanism as the others rather than a force-add the next person would have to know about.

The SKILL.md is reconstructed from the conventions the 45 report directories actually follow: the five dimensions, the verdict vocabulary, the mandatory Reason column, and the decision rules the reports encode — the D2 bridge-layer exemption, concentration over raw count, fixing design-system components first because the gap multiplies, the 3+ threshold for abstract, and Sweep Discipline counting a multi-file fix as one candidate. It says so at the top and invites correction.

Placement — enforced

scripts/quality/check-test-placement.mjs, wired to pnpm run check:test-placement and a CI step beside lint. It groups tests by the directory that owns them and names both the offending directory and which files to move. On a tie it names the colocated file — a tie means the newcomer should join the incumbent, and both regressions were exactly that shape.

Potential risks

• fix(session-core) commits change runtime behavior. The status narrowing is behavior-preserving by inspection of every consumer. The id format change is additive — no consumer parses the suffix — but it does alter tie-break ordering for same-millisecond events, which is the point.
• The reconstructed SKILL.md may differ from the original, which nobody has. It matches what the reports actually do; correct it where it disagrees with your memory.
• Three as SessionStatus casts are deliberately untouched — agentLiveStatusAtom.ts:40 (already guarded by a Set membership test), SessionService.ts:208 and launchPayload.ts:324 (different shapes, different provenance).
• The placement check adds a CI step; it runs in well under a second.

Validation

Check	Result
npx vitest run	1189 files / 9650 tests passed, 0 failed, 0 skipped
npx tsc --noEmit	exit 0
ESLint	clean on all touched files
pnpm run check:test-placement	consistent across 418 directories
Mutation probes	every fix proven by turning its mutant red, then reverted

Baseline before this branch: 1166 files / 9560 tests.

Rust is untouched, so cargo test --workspace was not re-run; #879's gate covers it on this branch's base.