[sonichi]

Summary

Adds typed channel-key constructors so the convention "discord-voice uses VC ID, phone uses Twilio SID" is code-enforced rather than CLAUDE.md-only.

Before this PR, discord-voice-server.ts passed CHANNEL_ID! raw to resultBelongsTo, and conversation-server.ts passed callSession.callSid raw. Both keys looked like opaque strings to the helper — if a future consumer adopted an ID format that happened to overlap with one of those spaces, two consumers could fight for the same file. Per-consumer prefixes (dvoice- / phone-) make that class of collision impossible.

Per Chi's ask

Discussion in #susan today. Susan asked "is the rule enforced by CLAUDE.md only?" — I replied that filename shape + safety are code-enforced (regex + glob semantics in each bridge), but the key choice (which consumer uses which ID format) was doc-only convention. Offered to ship a ~10-line PR adding typed constructors. Chi: "Sg" → here it is.

Changes

• src/result-channel-key.ts / src/result_channel_key.py — add discordVoiceKey(vcId) / discord_voice_key(vc_id) (returns dvoice-${sanitize(id)}) and phoneCallKey(callSid) / phone_call_key(call_sid) (returns phone-${sanitize(id)}). Internally use the existing sanitizeKey so behavior on unsafe input is unchanged.
• skills/discord-voice/scripts/discord-voice-server.ts — route channelKey through discordVoiceKey() instead of raw CHANNEL_ID!.
• skills/phone-conversation/scripts/conversation-server.ts — route the resultBelongsTo check through phoneCallKey(callSession.callSid).
• tests/result-channel-key.test.{ts,py} — 6 new tests each: prefix, sanitization, fallback on empty, round-trip through resultFilename+resultBelongsTo, and explicit collision-resistance check.
• CLAUDE.md — doc the typed constructors as the required entry point for the per-channel pull namespace.

Why this matters

Writer + consumer must go through the same typed function so the keys agree. The typed function is the single source of truth — that's what makes the rule self-enforcing instead of doc-only.

// Before
const channelKey = CHANNEL_ID!;  // raw VC id

// After
const channelKey = discordVoiceKey(CHANNEL_ID!);  // 'dvoice-<safe(vc-id)>'

# Before / after symmetric in conversation-server.ts
resultBelongsTo(name, callSession.callSid)              // raw SID
resultBelongsTo(name, phoneCallKey(callSession.callSid))  // 'phone-<safe(sid)>'

Test plan

• 24 TS tests + 24 Python tests, all passing locally (npx tsx --test tests/result-channel-key.test.ts and python3 tests/result-channel-key.test.py).
• Existing tests for raw sanitizeKey / resultFilename / resultBelongsTo API still pass — the new helpers are additive sugar on top.
• New test typed keys cannot collide across consumers (even if IDs match) explicitly verifies discordVoiceKey('CA1234abcd') !== phoneCallKey('CA1234abcd').

Compat note

No behavior change for existing users — the legacy resultFilename(rawKey, taskId) API is untouched. The two production consumers (discord-voice + phone) get the new prefix on next process start; in-flight files use whatever the current process wrote. No migration needed; both consumers read-and-delete their scoped files within seconds.

🤖 Generated with Claude Code

[qingyun-wu]

LGTM — code-enforcing the channel-key convention (was CLAUDE.md-only) is the right call; the dvoice-/phone- prefixes close a real, if narrow, collision class, and the tests cover sanitize + path-traversal (../etc → phone----etc). CI green.

Verified the writer↔consumer symmetry is complete: grepped the tree for programmatic result_filename/resultFilename callers → none. The only writer of results/<key>.task-{id}.txt is the core agent following CLAUDE.md, which this PR updates ("always go through the typed key constructor"). So no orphaned raw-key writer is left behind — the invariant holds end-to-end, not just in the two consumers.

One non-blocking note: a result file written with the old unprefixed key during the deploy window won't match the new prefixed scan, so it'd be stranded. Negligible in practice (read-and-delete within the 3s poll, and voice sessions are short-lived) — but maybe worth a one-line ack in the PR body for the record.

[liususan091219]

[Self-redacted — see follow-up comment for concise version.] — Lucy

[liususan091219]

[Self-redacted — moved discussion to private #dev channel.] — Lucy

[liususan091219]

Verified a9d015f closes the writer/consumer gap from the POC:

• _enqueue_context_prep_task now imports discord_voice_key and computes scoped_key = discord_voice_key(str(channel_id)) → dvoice-<id>
• Synthetic-task f-string interpolates {scoped_key} (line 293), no longer raw {channel_id}
• Fallback f"dvoice-{channel_id}" mirrors the constructor's contract when the import fails — writer keeps producing the shape the consumer scans for, no silent regression on import error

End-to-end the typed-key contract holds: writer → consumer use the same prefix via the same helper (or matching literal). Magic-word context-handoff path stays live after merge.

— Lucy (Mac Studio bot)

[liususan091219]

Re-review after the a9d015f follow-up commit (fix(join-trigger): route synthetic-task body through discord_voice_key()). Looked at the +19/-7 diff in skills/discord-voice/scripts/join_trigger.py:271-307 against the typed constructor in src/result_channel_key.py:96-98 (discord_voice_key(vc_id) -> f"dvoice-{sanitize_key(vc_id)}").

The fix correctly closes the writer-side of the architectural intent: the body now instructs the core to use a scoped result filename built via the same typed helper that the consumer (discord-voice-server.ts) scans for. Import-via-try/except is the right defensive shape for a skill script that may be invoked outside the repo, and the fallback f"dvoice-{channel_id}" matches the constructor's literal output for numeric Discord channel IDs (which is what channel_id always is here).

BLOCKING

None.

NON-BLOCKING

1. Fallback skips sanitize_key() (join_trigger.py:283). Today this is safe — Discord channel IDs are always digit strings, so sanitize_key() is a no-op. But the constructor at result_channel_key.py:97 calls sanitize_key(vc_id) and the fallback doesn't, so if channel_id ever picks up a non-id-shaped value (e.g. a name passed in for testing, or a future code path), the writer fallback would produce a key the consumer (which routes through the typed helper) wouldn't claim. One-line fix: inline the same sanitization in the fallback, or import sanitize_key separately so it's always available.

2. sys.path.insert per call (join_trigger.py:276). The insert runs every time the magic word fires; sys.path accumulates duplicate entries over the process lifetime. Cosmetic — Python dedupes on lookup, no functional impact — but the cleaner shape is to move the import to module load (with the same try/except), or guard the insert with if str(_REPO_ROOT / "src") not in sys.path:. Optional.

Approving the contract. Items above are nits, the architectural fix is correct.

— Maddy

[liususan091219]

LGTM — clean enforcement of the channel-key convention via typed constructors. Solid PR.

This is the right move on the "is the rule enforced by CLAUDE.md only?" question from #susan today. Before this PR, the per-consumer key choice ("discord-voice uses VC ID, phone uses Twilio SID") was a doc-only convention that the helper code couldn't validate — resultBelongsTo saw opaque strings, and if a future consumer adopted an ID format that overlapped with one of the existing key spaces, two consumers could fight for the same file. The typed constructors with per-consumer prefixes (dvoice- / phone-) make that class of collision provably impossible.

1. Single source of truth in src/result-channel-key.ts:82-97 + src/result_channel_key.py:87-102. Writer and consumer go through the same function — discordVoiceKey(vcId) returns the same string on both sides. The only way to write a dvoice--prefixed file and the only way to scan for one is via this function. That's what makes the convention self-enforcing instead of documentation-load-bearing. The TS / Python symmetry comment ("Keep symmetric with src/result_channel_key.py") is the right cross-language guardrail.

2. sanitizeKey reuse is the right abstraction layer. The typed constructors don't reimplement sanitization — they wrap the existing primitive. Means input-safety stays consistent across the legacy raw-key API and the new typed API, and any future hardening to sanitizeKey propagates everywhere.

3. Test coverage is sonichi-tier. Six tests per language: prefix shape, sanitization on unsafe input, fallback on empty/None, round-trip through result_filename+result_belongs_to, and explicit collision-resistance test (discord_voice_key('CA1234abcd') !== phone_call_key('CA1234abcd')). That last one is the test that future-you will thank past-you for — when the collision was "hypothetical" pre-PR, it's now "regression-tested" post-PR. Worth noting also that the test file structure mirrors the source file structure (assertion order, naming convention) — easy to keep in sync.

4. CLAUDE.md update lands the prose where it needs to be. Adding "Always go through the typed key constructor" with the rationale "the per-consumer prefix is code-enforced (single helper, single source of truth) so cross-consumer namespace collisions are impossible regardless of what ID format a future consumer adopts" is exactly the right framing — explains the WHY in addition to the HOW. Future readers won't be tempted to bypass the constructor "just this once."

5. Compat handling in join_trigger.py:_enqueue_context_prep_task is graceful. The fallback scoped_key = f"dvoice-{channel_id}" if the import fails preserves the writer's contract even when the helper isn't reachable (e.g. a stripped-down deployment). Symmetric with the constructor's output for the common case of pure-digit Discord channel IDs.

Non-blocking observations:

• One small symmetry gap in the join_trigger.py fallback (scripts/join_trigger.py:283). The except-fallback f"dvoice-{channel_id}" doesn't go through sanitize_key — so if a future Discord channel ID ever contains an unsafe character, the fallback would diverge from discord_voice_key()'s output. In practice Discord IDs are pure digits so this is hypothetical, but worth a one-line guard (re.sub(r'[^A-Za-z0-9-]', '-', str(channel_id)) inline, or just import sanitize_key separately) so the fallback's contract is identical to the constructor's. Not blocking; flag for follow-up.
• In-flight files use whichever process wrote them, per the PR body's compat note. That's correct for the discord-voice and phone consumers because they read-and-delete within seconds. Worth re-noting that this works because the bridge restart cadence + the consumer's poll interval are well within the file lifetime — if a future bridge has a longer read-delete cycle, the migration story may need to be more careful.
• Test name nit (tests/result-channel-key.test.py:225): test_typed_keys_sanitize_input asserts phone_call_key("../etc") == "phone----etc" — that's the right behavior (path-traversal char becomes -), but the assertion preserves the literal four-dash output. Would be slightly more readable as phone-{sanitize_key('../etc')} so the expected RHS shows the mechanism rather than the literal. Style preference; not a defect.

Cross-PR linkage: This builds directly on the per-channel pull namespace machinery that #1003 (or wherever the original resultBelongsTo landed) introduced. The convention was always implicit; this PR makes it explicit + code-enforced. Pairs naturally with the broader "tighten silent-failure paths" arc in #1093 / #1094 — all three PRs replace soft-discipline with mechanical enforcement.

Tests note: 24 TS + 24 Python tests passing per the body. Particularly the collision-resistance test is the keystone — it's the test that justifies the entire PR's existence. If that test ever starts failing in CI, you've broken the invariant the typed constructors are providing.

Ready to merge.

— Lucy (Mac Studio bot)

[bassilkhilo-ag2]

Cold review (@bassilkhilo-ag2):

Good defensive typing. Encoding the key format in the type system (dvoice-${channelId}, phone-${callSid}) is better than CLAUDE.md prose — it's enforced at call sites, not just documented.

The Python mirror (result_channel_key.py) maintaining parity with the TS module is the right pattern for cross-language consistency.

CLAUDE.md edit: the PR updates CLAUDE.md to document the key constructors. Since this is the shared CLAUDE.md (loaded on every session), keep the addition concise — it looks like it is. No concerns there.

Minor: src/result-channel-key.ts is a small new module — verify it's exported from the barrel src/index.ts (if one exists) so downstream code doesn't have to import by relative path.

[bassilkhilo-ag2]

LGTM. Typed channel-key constructors are the right fix.

• Prefixing dvoice- and phone- is the minimal change that makes cross-consumer namespace collisions impossible regardless of future ID formats
• Both TS and Python implementations are symmetric — good
• The fallback in join_trigger.py (f"dvoice-{channel_id}") mirrors the constructor's output and ensures writer/consumer still agree if the import fails
• CLAUDE.md update correctly documents the new convention ("always go through the typed constructor")
• Tests cover both TS and Python constructors

[bassilkhilo-ag2]

Code review (Sutando bot):

PR #1090 — feat(result-channel-key): typed per-consumer key constructors

The architectural goal is right: a single source of truth for the dvoice-/phone- prefix so writer and consumer can't drift independently. The typed constructors + sanitizeKey wrapping provide that.

Correctness: discordVoiceKey(CHANNEL_ID!) on the discord-voice-server.ts side and discord_voice_key(str(channel_id)) on the join_trigger.py side both go through sanitizeKey / sanitize_key, so the filename shape is guaranteed to match. Good.

One issue in join_trigger.py:

except Exception:
    scoped_key = f"dvoice-{channel_id}"

This should be except ImportError: (mirrors PR #1094's fix to read-quota.py). The current except Exception: would swallow unexpected runtime errors from result_channel_key.py — if sanitize_key throws (e.g., import-time name clash), you'd get a raw channel_id key instead of the sanitized one, which could silently break the consumer's scan without error. Narrow this to except ImportError:.

Also, the fallback f"dvoice-{channel_id}" bypasses sanitize_key — if channel_id contains characters that sanitize_key would strip (it shouldn't for Discord snowflakes, but for correctness): consider f"dvoice-{str(channel_id).replace(chr(47), '_')}" as a minimal safe fallback, or just accept the minor risk for snowflake IDs.

Otherwise LGTM — good abstraction, symmetric Python/TS implementations, 85 tests covering the constructors. The except ImportError nit is worth fixing before merge.

— Chi (Sutando node 2)

[VasiliyRad]

LGTM. The typed key constructor pattern is sound — thin wrappers over sanitizeKey that prepend a per-consumer prefix (dvoice-/phone-), making namespace collisions structurally impossible regardless of future ID formats.

Writer/consumer symmetry is complete: discord-voice-server.ts and join_trigger.py both route through discordVoiceKey/discord_voice_key; conversation-server.ts routes through phoneCallKey. The Python fallback in join_trigger.py correctly mirrors the constructor output if the import fails.

Backwards compat is preserved — existing bridges still glob on task-* and the <prefix>.task-{id}.txt shape slides past them, same as before. The only behavioral change is the key value itself gaining the prefix, which both sides now agree on by construction.

Test coverage is thorough: prefix correctness, sanitization of unsafe input, null/empty fallback to unknown, round-trip through resultFilename+resultBelongsTo, and explicit cross-consumer collision check. TS and Python test suites are symmetric.

No issues found.

[bassilkhilo-ag2]

LGTM — clean, well-motivated upgrade. Typed constructors are the right abstraction here: single source of truth for the prefix, symmetric TS/Python, tests cover the key cases (prefix, sanitize, null-fallback, round-trip, collision-resistance).

One minor observation: in join_trigger.py the except-fallback (line ~289) does f"dvoice-{channel_id}" without calling sanitize_key on channel_id. For Discord snowflakes (pure digits) this is harmless, but the fallback diverges from what the constructor would produce if a future channel ID contains slashes or dots. Low risk today, but worth noting if this file is ever extended to other platforms.

Otherwise no issues. VasiliyRad already approved; happy to see this land.

[qingyun-wu]

LGTM — clean, well-scoped, and the tests are thorough (collision case + round-trip through resultFilename/resultBelongsTo, in both TS and Python). Code-enforcing the prefix convention through a single constructor is the right call — it turns a CLAUDE.md-only convention into a compile/runtime invariant.

I verified the thing most likely to break here — producer and consumer must agree on the new prefix, or scoped results silently never match:

• discord-voice: the consumer (discord-voice-server.ts) and the automated writer (join_trigger.py's instruction string) are both updated → both emit/expect dvoice-. ✓
• phone: only the consumer (conversation-server.ts) changed in code — but that's correct, not a gap. There's no automated phone-side enqueuer (the scripts dir is just conversation-server.ts + the two guards; no join_trigger.py analog). The phone scoped-result producer is the core agent writing per CLAUDE.md, and you've updated CLAUDE.md to the phone-<…> form. So the symmetry holds. ✓

One non-blocking nit — the except fallback in join_trigger.py:

scoped_key = f"dvoice-{channel_id}"

skips sanitize_key, so the comment "Fallback mirrors the constructor's contract" slightly overstates it: on the import-failure path with a non-digit channel_id, this diverges from the consumer's sanitized key (dvoice-<sanitized>). Harmless in practice — Discord VC IDs are pure-digit snowflakes, so sanitize_key is a no-op on them — but either run the fallback through an inline sanitize (re.sub(r'[^A-Za-z0-9_-]', '-', str(channel_id))) or soften the comment to note it assumes an already-safe id.

Not blocking — ship it.

[jgluck96]

LGTM. Code-enforcing the key convention is the right call — CLAUDE.md-only was one missed update away from a silent collision. The import-fallback in join_trigger.py (except Exception: scoped_key = f'dvoice-{channel_id}') is correct: hardcodes the same prefix the constructor would produce, so the consumer scan still works even on a broken import. Tests on both TS and Python sides cover the expected shapes.