feat(config): JSON5 DSL loader with cross-ref + cycle validation (3rq.1) by brandon-fryslie · Pull Request #1 · promptctl/cc-candybar

brandon-fryslie · 2026-05-13T18:29:24Z

Summary

Implements loadDslConfig(filePath): DslConfig — the single validated entry point for the new Segment DSL config format
Top-level shape validation for globals, variables, segments, layout with per-kind required-field checks and closed-enum narrowing
Cross-reference pass: every template ref, depends_on target, and layout entry must resolve to a declared variable/segment
Cycle detection (DFS) over the template-kind variable graph — catches self-cycles, 2-node, and N-node cycles; reports the cycle path
Compiler-style error aggregation: all issues collected into ConfigError.issues[] before throwing, each with path, message, and line
Types in src/config/dsl-types.ts enforce all invariants structurally (discriminated union for 8 source kinds, 5-arm CacheDecl union for exactly-one cache policy)
73 tests covering malformed JSON5, missing required fields, undefined cross-refs, cycles, and valid config corpora

Lives alongside the existing loader.ts — chunk 7 (segment-dsl-migration-bzh) will wire this through the daemon and delete the legacy loader.

Test plan

pnpm test -- test/dsl-loader.test.ts → 73 tests pass
pnpm test → 644 tests pass (no regressions)
pnpm typecheck → clean

Single entry point loadDslConfig(filePath) → DslConfig that either returns a fully-validated config (every legal state representable, every illegal state unrepresentable) or throws ConfigError aggregating every issue compiler-style. Shape validation: globals + 8 source kinds (literal/input/env/file/shell/ template/time/git) with per-kind required fields; cache policies validate "exactly one of ttl/watch_file/depends_on/key/never" present; segment fields narrowed to closed enums; layout entries must resolve to declared segments. Cross-ref pass: extracts dotted var refs from every template (template/bg/fg/ when + per-variable templates and cache.key), validates each against the declared variable namespace following the scope-proxy's leaf-vs-namespace dispatch rules in src/template-engine/scope.ts. Cycle pass: DFS over the template→template-variable graph; reports the cycle path. Self-cycles, two-node, and N-node cycles all caught. Lives at src/config/dsl-loader.ts alongside the existing loader.ts — chunk 7 (brandon-segment-dsl-migration-bzh) wires this through the daemon and deletes the legacy loader.

Copilot

Pull request overview

Adds a new JSON5 DSL config loader (loadDslConfig / parseDslConfig) that produces a strongly-typed DslConfig after structural validation, cross-reference resolution, and template-variable cycle detection. It lives alongside the legacy loader.ts and will be wired into the daemon in a follow-up chunk. Errors are aggregated compiler-style into a ConfigError.issues[] list with paths, messages, and best-effort source line numbers.

Changes:

New src/config/dsl-types.ts defines the DslConfig shape with discriminated unions for 8 source kinds and a 5-arm CacheDecl union enforcing exactly-one cache policy.
New src/config/dsl-loader.ts implements multi-stage validation (JSON5 parse → top-level shape → per-kind shape → cross-refs → cycle DFS) with helper utilities for ref extraction and source-line lookup.
Adds json5@^2.2.3 as a runtime dependency and 73 tests covering syntax errors, missing required fields, undefined cross-refs, cycles, and a full valid-config corpus.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/config/dsl-types.ts	Defines `DslConfig`, variable kind unions, cache policy union, and supporting enum constants.
src/config/dsl-loader.ts	Implements parse/validate pipeline, cross-ref check, cycle detection, and error aggregation.
test/dsl-loader.test.ts	73-case test suite covering JSON5 errors, shape validation, cross-refs, cycles, error aggregation, and helpers.
package.json	Adds `json5` dependency; reorders optional native binary entries.
pnpm-lock.yaml	Records `json5@2.2.3` resolution.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

Comments suppressed due to low confidence (1)

src/config/dsl-loader.ts:963

The unused-variable suppression void segName; is a code smell signaling that the loop intends to track segment context but doesn't. Combined with the bug noted above (segment-local vars keyed by bare name only), this confirms the segment context is being dropped where it should be propagated. Removing this suppression and using segName to construct fully-qualified node names would address both issues at once.

    void segName;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

brandon-fryslie · 2026-05-14T04:36:02Z

+  for (const [segName, seg] of Object.entries(cfg.segments)) {
+    if (!seg.vars) continue;
+    for (const [vName, vDecl] of Object.entries(seg.vars)) {
+      if (vDecl.kind === "template") {
+        templateVarNames.add(vName);
+        templateVarNames.add(`${segName}.${vName}`);
+      }
+    }
+  }
+
+  const graph = new Map<string, Set<string>>();
+  for (const name of templateVarNames) graph.set(name, new Set());
+
+  const addEdges = (from: string, template: string): void => {
+    for (const ref of extractTemplateRefs(template)) {
+      if (templateVarNames.has(ref)) {
+        graph.get(from)!.add(ref);
+        continue;
+      }
+      // Resolve "first identifier" — `.session.id` may indicate dependence on
+      // `session` if that's the template var (matches scope.ts proxy walk).
+      const head = ref.split(".")[0]!;
+      if (head !== ref && templateVarNames.has(head)) {
+        graph.get(from)!.add(head);
+      }
+    }
+  };
+
+  for (const [name, v] of Object.entries(cfg.variables)) {
+    if (v.kind === "template") addEdges(name, v.template);
+  }
+  for (const [segName, seg] of Object.entries(cfg.segments)) {
+    if (!seg.vars) continue;
+    for (const [vName, vDecl] of Object.entries(seg.vars)) {
+      if (vDecl.kind === "template") addEdges(vName, vDecl.template);
+    }
+    void segName;
+  }


Resolution: already-fixed on main.

This PR (#1) is the 3rq.1 commit (cb18319); follow-up PR #2 (merged as f1b1ef2) bundled this same 3rq.1 commit and extended it with 3rq.3 work. As part of that follow-up the segment-var graph was rewritten to use only the canonical ${segName}.${vName} form as a graph node — bare vName is no longer added to templateVarNames, and addEdges is invoked with the canonical name (see current main src/config/dsl-loader.ts lines 962–972 and 1018–1026). The bare/namespaced conflation you identified at lines 963/964 is gone, and segCtx resolves bare refs like .local against the namespaced form explicitly.

Since PR #1 is fully subsumed by merged PR #2, this PR should be closed as superseded rather than merged. Resolving this thread.

+          line: findKeyLine(ctx.source, path.split(".")),
+        });
+        return null;
+      }


brandon-fryslie · 2026-05-19T05:43:17Z

Already merged in main. The 3rq.1 DSL loader was absorbed into #2 (3rq.3) as a sub-commit — see f1b1ef2 in main, which carries both "feat(config): JSON5 DSL loader with cross-ref + cycle validation (3rq.1)" and "feat(config): extend cycle detection to depends_on + cache.key edges (3rq.3)" as combined commit messages. src/config/dsl-loader.ts, src/config/dsl-types.ts, and test/dsl-loader.test.ts already exist in main with the 3rq.1 content. Closing as obsolete.

Proves epic k5a done-gate #1: a set-state `theme` click recolors the WHOLE bar live. The existing widget test passes a STATIC basePalette into renderDsl, so it verifies the click writes SessionState but never the recolor — which lives in the daemon's per-render basePalette resolution (effectiveThemeName -> resolverForThemeName), OUTSIDE renderDsl. This test replicates that resolution exactly as src/daemon/server.ts does, so a click changes the bytes a non-picker segment renders. Asserts: whole-bar bg-color footprint changes (dark<->light disjoint), the non-picker segment itself recolors, and active-marking tracks the rendered theme (config-default bold initially, picked option bold after the click). [LAW:verifiable-goals][LAW:single-enforcer] drives the real spine — registerDslConfig + renderDsl + parseHandlerUrl + VERBS, no parallel rig. Follow-up k5a.6 owns the paginated-menu widget (collapse + width-paginated navigation) that replaces the flat overflowing picker.

Proves epic k5a done-gate #1: a set-state `theme` click recolors the WHOLE bar live. The existing widget test passes a STATIC basePalette into renderDsl, so it verifies the click writes SessionState but never the recolor — which lives in the daemon's per-render basePalette resolution (effectiveThemeName -> resolverForThemeName), OUTSIDE renderDsl. This test replicates that resolution exactly as src/daemon/server.ts does, so a click changes the bytes a non-picker segment renders. Asserts: whole-bar bg-color footprint changes (dark<->light disjoint), the non-picker segment itself recolors, and active-marking tracks the rendered theme (config-default bold initially, picked option bold after the click). [LAW:verifiable-goals][LAW:single-enforcer] drives the real spine — registerDslConfig + renderDsl + parseHandlerUrl + VERBS, no parallel rig. Follow-up k5a.6 owns the paginated-menu widget (collapse + width-paginated navigation) that replaces the flat overflowing picker. Co-authored-by: elton-prawn <264370887+elton-prawn@users.noreply.github.com>

* feat(layout): collapse LayoutNode to container | segment (2de.11) [LAW:one-type-per-behavior] LayoutNode is now exactly `container | segment`. The `inline` and `stepper` node kinds are deleted: interaction, state-driven display, and multi-region clickability all live in a segment's TEMPLATE (via widgets today; the `actions:` model in .12), so they were variability the node type should never have carried. A `segment` is THE unit of rendering — one ref into the named segments map, rendered to one strip item (the 2de.10 structural join). `cells` and `layout` rows survive as authoring SUGAR, lowered at the loader to `container(horizontal, [segment…])` so nothing downstream sees more than two kinds [LAW:one-source-of-truth]. Deleted (their surface is gone, not relocated yet): - InlineNode/StepperNode/InlineCell/ClickWrite types + nodeStateUse projection - node-registry inline/stepper arms, the composite "HUE PIN", setStateUrl/glyphs - validateInlineNode/InlineCell/ClickWrite/StepperNode loader validators - deriveNodeValidators/nodeContributions/nodeDeriveSpecs; deriveValidators is now widget-only (the actions surface merges in at .12) - the node-set-write session.id trigger + node backing-var cross-ref loop Hue rotation is retained but purely DECORATIVE — each segment advances the cursor by one, a container by none; it carries no structural meaning now that cohesion is one-segment-one-item. Tests: dsl-inline-cell + dsl-stepper-node deleted (they pinned the removed surfaces); segment-render-unit #1/#2 re-expressed through the segment surface (a link-bearing template; an empty template) [LAW:behavior-not-structure]; node builders in the suite construct the canonical container|segment tree. dsl-spine snapshot byte-identical; full suite green (1057); live daemon renders the real config (widgets, hue, OSC-8) unchanged. * refactor(layout): drop stale loader comment; delegate deriveValidators [LAW:comments-explain-why-only] Remove a stale WHAT-comment above layoutRowsToNode — it still described the lowering as "a vertical container of cells leaves" and sat directly above the correct description, two divergent copies back-to-back. [LAW:one-source-of-truth] deriveValidators delegates to deriveWidgetValidators rather than duplicating its body. The widget surface IS the whole writable-key surface today, so one body means no drift; when the actions table lands (.12) this single site becomes the widget+action merge. Addresses PR #70 review (both nits). lint + typecheck clean; validator/loader suites green. --------- Co-authored-by: elton-prawn <264370887+elton-prawn@users.noreply.github.com>

… suggestion (zk5) (#95) 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. Co-authored-by: elton-prawn <264370887+elton-prawn@users.noreply.github.com>

Copilot AI review requested due to automatic review settings May 13, 2026 18:29

Copilot started reviewing on behalf of brandon-fryslie May 13, 2026 18:30 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

brandon-fryslie mentioned this pull request May 13, 2026

feat(config): extend cycle detection to depends_on + cache.key edges (3rq.3) #2

Merged

3 tasks

brandon-fryslie closed this May 19, 2026

elton-prawn deleted the bf/3rq.1-jsonschema-loader branch May 19, 2026 05:43

This was referenced May 28, 2026

Config cascade + CLI hardening + daemon socket decoupling + visible diagnostics #33

Merged

feat(themes,daemon): live per-render theme from SessionState (k5a.4) #48

Merged

test(themes): end-to-end theme-picker recolor verification (k5a.5) #49

Merged

brandon-fryslie mentioned this pull request Jun 4, 2026

feat(layout): collapse LayoutNode to container | segment (2de.11) #70

Merged

brandon-fryslie mentioned this pull request Jun 10, 2026

fix(loader): reject bare own-segment var refs at load with namespaced suggestion (zk5) #95

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(config): JSON5 DSL loader with cross-ref + cycle validation (3rq.1)#1

feat(config): JSON5 DSL loader with cross-ref + cycle validation (3rq.1)#1
brandon-fryslie wants to merge 1 commit into
mainfrom
bf/3rq.1-jsonschema-loader

brandon-fryslie commented May 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

brandon-fryslie May 14, 2026

Uh oh!

brandon-fryslie commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

brandon-fryslie commented May 13, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

brandon-fryslie May 14, 2026

Choose a reason for hiding this comment

Uh oh!

brandon-fryslie commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants