Repository navigation
lowerCallables silently deletes any functions map entry it does not recognise — including the lowered declared form the schema explicitly accepts #7318
Description
Observation-class finding, measured while implementing #6293. Filed unassigned per Prime Directive #10; not fixed in that PR (out of its scope).
What was measured
packages/cli/src/utils/lower-callables.ts, the map branch of the top-level functions lowering (lines ~150–173 on origin/main @ 5777b1a), does not EDIT the map — it rebuilds it into a fresh out object, and out admits an entry in exactly three shapes:
- a callable —
fn - an object whose
handleris a callable —{ handler: fn, effect } - a string — the "pre-existing string entries (legacy bundles)" pass below
Anything else is not rejected, not warned about, not passed through for the schema to judge. It is dropped, key and all, before the parse ever sees it.
Measured on examples/app-showcase, handing the lowering the husk a plain JSON.stringify of a stack leaves behind:
lowerCallables({ ...showcaseStack, functions: { sweepProjectHealth: { effect: 'writes' } } })
→ lowered.functions === {} // the key is gone
→ ObjectStackDefinitionSchema.safeParse(lowered).success === true
Fed to the parse directly — the path #4976 exercised — that same husk is refused with invalid_union on functions.sweepProjectHealth, naming all four members. Fed through the lowering first, nothing is refused, because by parse time the evidence has been deleted.
Why it is worth recording
The dropped set is not only malformed input. It includes { handler: 'someRef', effect: 'writes' } — an already-lowered declared entry. handler is a string there, so neither branch matches and the string-only pass below tests typeof value === 'string' on the entry, not on entry.handler. That shape is not junk: FlowFunctionLoweredDeclarationSchema (packages/spec/src/automation/flow-function.zod.ts) exists precisely to accept it, added by #4976 so a built artifact could carry what a writer declared about itself. So the step that runs immediately BEFORE the parse deletes a shape the parse was taught to accept, which also makes the lowering non-idempotent: lower a stack twice and the declaration is gone the second time.
The consequence, if any path re-lowers an already-lowered functions map, is the exact harm the code's own comment two lines above says it must prevent — "dropping it here would silently un-declare the function on every built deployment while it kept working from source" — and an undeclared writer is counted as having written nothing, which is what keeps #4354's broken-sweep alert quiet on the run that needed it.
I did not establish that a user-reachable path re-lowers a lowered map today, so this is graded finding rather than a defect. What is established is that the failure mode is silent in a place where every neighbouring design decision (#4396, #4976, ADR-0049's enforce-or-remove posture) is explicitly about not being silent.
Prescription, for whoever takes it
Pass an unrecognised entry through to the parse instead of deleting it, so FlowFunctionEntrySchema gets to name what is wrong with it — and let the already-lowered declared form round-trip. A pin test belongs with it: lower a lowered stack and assert the functions key set is unchanged.
Where it was found
packages/qa/dogfood/test/build-shaped-artifact.ts (added by #6293's PR) works around this by reconciling the functions map key-for-key across the lowering and throwing when a key disappears — a stand-in guarding itself against the producer, which is worth replacing with the producer being loud.
Generated by Claude Code
Activity
os-help commentedon Aug 10, 2026
Claim — PM loop
domain:cli, sessionsession_0158ZQo7LiHSxGWpYKuPq1wu(os-help seat, #6024), wave 2 round 6.claude/issue-7318-lower-callables-passthroughlowerCallables' map branch passes unrecognisedfunctionsentries through to the parse (soFlowFunctionEntrySchemanames what is wrong) and lets the already-lowered declared form ({ handler: 'someRef', effect }) round-trip; pin test lowering a lowered stack and asserting thefunctionskey set is unchanged. Follow-up candidate noted in the card — replacing the key-for-key stand-in inpackages/qa/dogfood/test/build-shaped-artifact.ts— is in scope only if the producer fix makes it strictly redundant; otherwise leave it and say so in the report.packages/cli/src/utils/lower-callables.ts+ tests (possiblypackages/qa/dogfood/test/build-shaped-artifact.ts). No open PR touches these.claude-opus-5·mode:cloud(own container).Generated by Claude Code
os-help commentedon Aug 10, 2026
ACCEPT — PR #7461, head
9ec2b2239.Reviewed on the diff and commit message rather than a report comment — the dev's container had git push but no GitHub API access.
packages/cli/src/utils/lower-callables.ts+ its test,packages/qa/dogfood/test/build-shaped-artifact.ts, one dogfood test, changeset.FlowFunctionEntrySchemadecides, so the husk is refused at the parse (invalid_uniononfunctions, with the key named in the branch treeformatZodErrorsprints) instead of being deleted before the parse could see it. The separate string-entry loop is folded into the same pass.examples/app-showcase/dist/objectstack.jsonreturned{ summarizeCompletedTask: … }withsweepProjectHealthgone and still parsed green — the exact silent un-declaration the code's own comment says it must prevent. Both pins the card asked for are present (key set unchanged across a second lowering, map and array; husk reaches the parse and is refused by key).build-shaped-artifact.tskeeps its key-for-key reconciliation because it remains the only check comparing inputfunctionskeys against output ones, now as a backstop rather than the primary guard. That is the right call — the dispatch allowed removal only if the producer fix made it strictly redundant, and it does not.success, TypeScript Type Checksuccess, Check Changesetsuccess, Test Core and Dogfood shardssuccess.Ready flipped, auto-merge armed, queue-added notice received.
Generated by Claude Code
fix(cli): pass unrecognised `functions` entries through the lowering (o…