feat: report sourceImageTag in the info message (surface worker-code vs WASM version) - #122
Conversation
…version) The translate Worker's `gitHash` is the WORKER CODE's commit, but its WASM Opus codec is sourced independently at deploy time from a Docker image tag (SOURCE_IMAGE_TAG). Those two can drift — e.g. worker code from a branch with a `latest` WASM that predates it — and nothing in the info message surfaced the codec's origin, so such a mismatch was invisible (a real incident: DTX worker code shipped against a pre-DTX WASM, silently producing no audio). Surface `env.SOURCE_IMAGE_TAG` as `sourceImageTag` in the info message (worker `buildServerInfo` + node `serverInfo`), alongside `gitHash`. The peer (JVB) already logs the info message, so a code/WASM mismatch is now visible at a glance. No behavior change; SOURCE_IMAGE_TAG is already a deployed var. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code ReviewOverviewSmall, focused observability improvement: surfaces Code QualityPattern consistency ✅ — The conditional guard Separation of concerns ✅ — The Node path ( Comment quality ✅ — Comments in all three files explain the why well (independent versioning, drift risk, real incident), not just the what. Potential IssuesCLAUDE.md not updated
Performance / SecurityNo concerns. This is a one-time read at connection establishment, not a hot path. Test CoverageNo new tests were added. Given the simplicity (a single conditional field append in two places), that's acceptable. If you want belt-and-suspenders coverage, a brief test asserting that the SummaryThe change is correct, minimal, and solves a real observability gap. The two items worth addressing before merge:
Neither is a blocker for the logic itself, but (1) is a stated project requirement. 🤖 Generated with Claude Code |
Review follow-ups (#122): - CLAUDE.md: add `sourceImageTag` to the sample `info` JSON and explain it — the translate Worker's WASM codec image tag, reported so a code/WASM mismatch is visible against `gitHash` (per the "update CLAUDE.md in the same commit" convention for info-message format changes). - README: add a `SOURCE_IMAGE_TAG` row to the translation env-var reference. - serverInfo.ts: reword the comment — it surfaces for any deploy that sets SOURCE_IMAGE_TAG (in practice the translate Worker), not "translate-worker only". Docs/comment only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code ReviewOverviewThis is a clean, small, well-motivated change (+14/-1 lines). It surfaces What's Good
Issues / SuggestionsMissing test coverage (minor)
it('includes sourceImageTag when SOURCE_IMAGE_TAG is set', () => {
const saved = process.env.SOURCE_IMAGE_TAG;
try {
process.env.SOURCE_IMAGE_TAG = 'v1.2.3';
const info = buildServerInfo({});
expect(info.sourceImageTag).toBe('v1.2.3');
} finally {
if (saved === undefined) delete process.env.SOURCE_IMAGE_TAG;
else process.env.SOURCE_IMAGE_TAG = saved;
}
});
it('omits sourceImageTag when SOURCE_IMAGE_TAG is unset', () => {
const saved = process.env.SOURCE_IMAGE_TAG;
delete process.env.SOURCE_IMAGE_TAG;
try {
const info = buildServerInfo({});
expect('sourceImageTag' in info).toBe(false);
} finally {
if (saved !== undefined) process.env.SOURCE_IMAGE_TAG = saved;
}
});(Alternatively, add Doc example field ordering (nit) The CLAUDE.md example places Security / CorrectnessNo concerns. OverallApproved with the suggestion to add the two test cases. The change is correct, well-scoped, and closes a real operational gap. |
Review follow-up (#122): serverInfo.test.ts tracks every other optional field (sessionId, instanceId, location) but had no case for sourceImageTag. Add SOURCE_IMAGE_TAG to the env save/restore list and two cases — present when set, absent when unset. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code ReviewSummary: This PR adds a OverviewThe change is minimal (26 additions), touches three runtime files and one test, and is entirely additive — no behavior change unless Code Quality
Tests — two cases added ( Minor observations
Security / PerformanceNo concerns. DocumentationBoth Verdict: LGTM. Clean, well-motivated, minimal change that follows all project conventions. The CLAUDE.md example nuance (point 1) is the only thing worth a second look, but it's non-blocking. |
Review follow-up (#122): the sample `info` JSON shows `sourceImageTag` alongside `"runtime": "cloudflare-container"`, but the container never emits that field (code + codec are one image there). Note that the block is a composite of every possible field and that `sourceImageTag`/`worker` appear only on the translate-Worker path, so the example no longer reads as if the container emits them. Docs only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Code Review Overview This PR surfaces SOURCE_IMAGE_TAG as sourceImageTag in the info message for both src/serverInfo.ts (Node) and worker/translationRuntime.ts (Cloudflare Worker). The motivation is solid: the translate Worker has two independently-versioned inputs (worker code via gitHash, WASM codec via Docker image tag), and a silent mismatch already caused a real incident. This is a minimal, focused change. What's Good
Minor Issues
CLAUDE.md says: 'sourceImageTag and the worker block are emitted only on the translate-Worker path, not on the container shown by the cloudflare-container runtime here.' But src/serverInfo.ts (the Node/container path) also gains the if (process.env.SOURCE_IMAGE_TAG) check in this PR. In practice the container will not have SOURCE_IMAGE_TAG set, so the statement is operationally true -- but technically misleading. Worth either rewording to 'in practice only emitted on the translate-Worker path (where SOURCE_IMAGE_TAG is set at deploy); the container omits it because code and codec are co-versioned in the same image', or just dropping the parenthetical about which path emits it. The same slight imprecision appears in README.md: 'Set by the translate-Worker deploy' implies it cannot be set elsewhere, when the code will surface it anywhere SOURCE_IMAGE_TAG is in the environment.
The existing note reads: 'It intentionally omits the Node message's providersAvailable, config.* ... and instanceId fields.' sourceImageTag is now a new field added to the Worker info (not omitted), but the note does not mention it. A reader consulting this section will not know it's there. A one-liner like 'It now also includes sourceImageTag when SOURCE_IMAGE_TAG is set' would close the gap. Verdict No correctness issues, no behavioural changes at runtime, tests are appropriate. The two documentation nits above are worth a quick fix to keep CLAUDE.md accurate, but they are low-stakes. |
Review follow-up (#122): the previous doc pass over-claimed that sourceImageTag is emitted "only on the translate-Worker path", but the serverInfo.ts (Node) path gains the same conditional in this PR — it's emitted by any runtime that has SOURCE_IMAGE_TAG in its environment (in practice only the translate-Worker deploy sets it; the container omits it because code and codec are co-versioned in one image). Reword CLAUDE.md and the README row accordingly, and note in the Worker /translate info-block section that sourceImageTag is now included when the var is set (it's not among the omitted fields). Docs only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code ReviewOverall: Clean, well-scoped observability improvement. No blocking issues. OverviewThis PR surfaces Code Quality
Test Coverage
Minor Observations
Security / PerformanceNo concerns — this is a pure observability addition with no behavioral effect and no sensitive data exposure (a Docker image tag is already public information in the deployed environment). Summary: Ship it. The change is minimal, correct, well-tested on the changed path, and solves a real operational problem. The CLAUDE.md note is a nit, not a blocker. |
What
Surface the WASM codec's origin (
SOURCE_IMAGE_TAG) assourceImageTagin theinfomessage, alongside the existinggitHash.Why
The translate Worker has two independently-versioned inputs:
gitHash.SOURCE_IMAGE_TAG), chosen separately from the code ref.These can drift, and nothing in
infosurfaced the codec's origin — so a mismatch was invisible. This bit us for real: DTX worker code was deployed withIMAGE_TAG=latest(a pre-DTX WASM lacking_opus_frame_encoder_set_dtx), the encoder threw on init, and the session silently produced zero translated audio — with onlygitHash:"dev"/SOURCE_IMAGE_TAG:"latest"to go on.With this change the peer (JVB), which already logs the
infomessage, sees both identities side-by-side (gitHash=<worker sha>, sourceImageTag=<codec tag>), so a code/WASM mismatch is obvious at a glance.Changes
worker/translationRuntime.tsbuildServerInfoandsrc/serverInfo.ts: addsourceImageTagfromenv.SOURCE_IMAGE_TAG(worker) /process.env.SOURCE_IMAGE_TAG(node), conditional (omitted when unset — e.g. the container, where code + codec are one image so it doesn't apply).worker/env.d.ts: typeSOURCE_IMAGE_TAG.No behavior change —
SOURCE_IMAGE_TAGis already a deployed var; this only reports it.Related
Complements a small
deploy.shchange (infra) that stamps the worker-code commit into__GIT_HASH__sogitHashstops being"dev". Each is useful alone; together they make a code/WASM mismatch fully diagnostic. (A follow-up "robust" tier could bake the WASM's own build commit into the image aswasmGitHash.)Testing
typecheck+typecheck:worker+check:worker-safeclean; full unit suite (620) passes.+10lines, 3 files.🤖 Generated with Claude Code