Summary
CI runs deno check on both edge functions and never runs deno test. Every test in
supabase/functions/hal-mcp/index.test.ts — 63 of them — is dead weight in CI: written,
committed, reviewed, and never executed.
A green CI on a PR touching hal-mcp therefore proves type-checking only. It says nothing
about behaviour.
Location
.github/workflows/ci.yml:21,25
- run: deno check index.ts # hal-mcp
- run: deno check index.ts # edifice-map-generator
No deno test step exists anywhere in the workflow.
How it was found
PR #76 (fix for #74) added 146 lines of tests asserting that push_mission_context issues one
targeted UPDATE per note_id — the core of the fix. Its CI came back green on three checks
(Deno type-check, Migration smoke-test, Python tests) and the PR was merged on that basis.
Those 146 lines had never run. Executed by hand before deploying:
cd supabase/functions/hal-mcp
SUPABASE_URL="https://zgkvbjqlvebttbnkklpo.supabase.co" \
SUPABASE_SECRET_KEYS='{"default":"test-admin"}' \
deno test --allow-all index.test.ts
# ok | 63 passed | 0 failed
They pass — this time. The point is that nothing would have caught it if they hadn't.
Why the env vars matter
index.ts:6 reads SUPABASE_URL at module load, so the test file cannot even be imported
without it. And the value must be the real project URL: four OAuth-metadata tests assert the
exact resource_metadata URL, so a placeholder like http://localhost:54321 fails those four
while the other 59 pass. Whoever writes the CI step will hit this — it is a fixture requirement,
not a defect.
SUPABASE_SECRET_KEYS needs a syntactically valid JSON value; the test file's own default is
{"default":"test-admin"}.
Fix direction
Add a deno test step to ci.yml next to the existing type-check, with both variables set from
repo-level (non-secret) values — the URL is public and the secret-keys value is a test fixture,
so neither needs GitHub Secrets.
Worth checking at the same time whether edifice-map-generator has tests that are equally
unrun.
Acceptance
- CI fails when a
hal-mcp unit test fails (verify negatively: break one assertion on a scratch
branch and confirm the job goes red).
- The 63 existing tests pass in CI.
docs/operations.md §1 mentions the test job alongside the schema-snapshot obligation.
Summary
CI runs
deno checkon both edge functions and never runsdeno test. Every test insupabase/functions/hal-mcp/index.test.ts— 63 of them — is dead weight in CI: written,committed, reviewed, and never executed.
A green CI on a PR touching
hal-mcptherefore proves type-checking only. It says nothingabout behaviour.
Location
.github/workflows/ci.yml:21,25No
deno teststep exists anywhere in the workflow.How it was found
PR #76 (fix for #74) added 146 lines of tests asserting that
push_mission_contextissues onetargeted
UPDATEpernote_id— the core of the fix. Its CI came back green on three checks(Deno type-check, Migration smoke-test, Python tests) and the PR was merged on that basis.
Those 146 lines had never run. Executed by hand before deploying:
They pass — this time. The point is that nothing would have caught it if they hadn't.
Why the env vars matter
index.ts:6readsSUPABASE_URLat module load, so the test file cannot even be importedwithout it. And the value must be the real project URL: four OAuth-metadata tests assert the
exact
resource_metadataURL, so a placeholder likehttp://localhost:54321fails those fourwhile the other 59 pass. Whoever writes the CI step will hit this — it is a fixture requirement,
not a defect.
SUPABASE_SECRET_KEYSneeds a syntactically valid JSON value; the test file's own default is{"default":"test-admin"}.Fix direction
Add a
deno teststep toci.ymlnext to the existing type-check, with both variables set fromrepo-level (non-secret) values — the URL is public and the secret-keys value is a test fixture,
so neither needs GitHub Secrets.
Worth checking at the same time whether
edifice-map-generatorhas tests that are equallyunrun.
Acceptance
hal-mcpunit test fails (verify negatively: break one assertion on a scratchbranch and confirm the job goes red).
docs/operations.md§1 mentions the test job alongside the schema-snapshot obligation.