fix(loader): reject bare own-segment var refs at load with namespaced suggestion (zk5)#95
Merged
brandon-fryslie merged 1 commit intoJun 10, 2026
Conversation
… suggestion (zk5) Sheriff finding #1, locked decision NAMESPACED-ONLY. [LAW:one-source-of-truth] The cross-ref validator admitted a segment's own locals in bare form while the runtime scope proxy resolves only literal store keys (segment locals are stored namespaced as segName.varName) — so a bare own-segment ref validated at load, then threw MissingFieldError at render. - cross-ref: ONE templateScope mirroring the runtime store's key set (globals bare, segment locals namespaced) checks every template ref — segment fields, segment vars, global vars, layout-node when. The segment name survives only as a diagnostic hint: a bare ref to an own local is rejected with a message naming the namespaced form to write. - cycles: drop the segCtx bare→namespaced aliasing (a second copy of the rejected resolution rule) and its false matches-runtime comment. - render.ts / dsl-types.ts: correct the comments that claimed bare-name access works or was a planned follow-up. - allVarNames (bare + namespaced) survives for depends_on only — a literal name list, not a template ref; its bare-name admission is a separate pre-existing contract (follow-up ticket). No working config breaks: a bare own-segment ref already failed at render; this moves the guaranteed failure to a load-time diagnostic.
There was a problem hiding this comment.
Z.ai Coding Agent Review
This PR tightens segment-local variable scoping so that bare-name refs (.local) are rejected at load time and only the namespaced form (.segName.local) is valid — making a variable reference's meaning a pure function of the reference string, independent of which segment is rendering. This is a genuine [LAW:one-source-of-truth] improvement.
No must-change issues found. Specifics:
templateScopeis built once and used uniformly for all template ref validation — globals bare, segment locals namespaced — faithfully mirroring the runtime store's key set. Correct.segCtxis purely a diagnostic hint that only affects error messages (suggesting the namespaced form for own-segment bare refs). It never gates resolution logic. Correct.allVarNamesis preserved fordepends_onvalidation (bare names admitted there), clearly documented as a separate pre-existing contract. Correct.- Cycle detection (
cycles.ts) is simplified correctly — removingsegCtxsince refs are now always literal store keys. Bare refs that slip through would simply be unresolved edges, but cross-rejection blocks them before cycle detection runs. - Tests cover the key cases: bare own-segment ref → hint, sibling var template → hint, global context → no hint, namespaced form → passes, cross-segment bare ref → rejected without hint, cross-segment namespaced → passes.
One pre-existing observation (not introduced by this diff): allVarNames in cross-ref.ts includes bare segment-local names (line 37: allVarNames.add(v)) for depends_on scope. This means depends_on: ["local"] inside segment s would match segment s's own var local — this is the "deliberately untouched" contract noted in the comment. Worth being aware of if a future PR further unifies the two scopes.
✅ Approved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket
brandon-config-validation-zk5— Sheriff finding #1 (law audit 2026-06-09), locked decision NAMESPACED-ONLY.[LAW:one-source-of-truth]Problem
The cross-ref validator admitted a segment's own locals in bare form (
segScope.add(vName)), with a comment claiming this 'matches runtime scope-proxy rules'. It doesn't: the runtime scope proxy resolves only keys literally present in the store, and segment locals are stored namespaced assegName.varName. A bare own-segment ref therefore passed validation and threwMissingFieldErrorat render — the validator certified configs that fail, and two comments asserted opposite facts about the same contract.Fix
templateScopeset, a faithful mirror of the runtime store's key set (globals bare, segment locals namespaced only), checks every template ref: segment template/bg/fg/when, segment-var templates and cache keys, global-var templates, and layout-nodewhen. A ref's meaning is now a pure function of the ref string, never of which segment is rendering. The segment name survives purely as a diagnostic hint: a bare ref to an own local fails withTemplate references unknown variable ".local" (segment-local vars are namespaced — write ".s.local").segCtxbare→namespaced aliasing (a second copy of the rejected resolution rule) and its false matches-runtime comment.depends_ononly — a literal name list, not a template ref; its own bare-name admission is filed asbrandon-config-validation-095.No working config breaks
A bare own-segment ref already failed at render unconditionally; this moves a guaranteed render-time failure to a load-time diagnostic that names the correct form. Neither the bundled default config nor any known user config declares segment-local vars.
Verification
pnpm typecheck,pnpm lint: cleanpnpm test: 66 suites, 1117 tests pass