Merge branch 'next' into kimi-runtime-support
This commit is contained in:
Viktorplus
5
.changeset/913-plan-phase-toplevel-spawn-guard.md
Normal file
5
.changeset/913-plan-phase-toplevel-spawn-guard.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 913
|
||||
---
|
||||
**Top-level Claude Code `/gsd-plan-phase` now always spawns the researcher/planner/plan-checker agents instead of collapsing them inline** — a `<runtime_compatibility>` block after `</available_agent_types>` makes the Agent-availability requirement explicit and documents that the workflow fails-closed (stops with a clear log message) in genuinely Agent-less contexts; seven "ORCHESTRATOR RULE — CODEX RUNTIME" labels are renamed to "ALL RUNTIMES" so the guard applies universally; `execute-phase.md` scopes its existing "Other runtimes" inline-fallback prose to non-Claude contexts, preserving the #853 backgrounded-agent behaviour. (#913)
|
||||
2
.gitignore
vendored
2
.gitignore
vendored
@@ -132,6 +132,8 @@ build/
|
||||
/gsd-core/bin/lib/phase-id.cjs
|
||||
/gsd-core/bin/lib/config-loader.cjs
|
||||
/gsd-core/bin/lib/model-resolver.cjs
|
||||
/gsd-core/bin/lib/loop-resolver.cjs
|
||||
/gsd-core/bin/lib/federated-config.cjs
|
||||
/gsd-core/bin/lib/phase-locator.cjs
|
||||
/gsd-core/bin/lib/roadmap-parser.cjs
|
||||
/gsd-core/bin/lib/drift.cjs
|
||||
|
||||
@@ -149,10 +149,13 @@ A bundle delivering one optional GSD feature, toggled as a unit at install or af
|
||||
Generated description of what the five-step loop (Discuss → Plan → Execute → Verify → Ship) exposes as extension points: per-step loop points, agent roles, and core artifacts. Sourced from structured `<!-- gsd:loop-host ... -->` HTML-comment markers embedded near the top of each of the five step workflow files (`discuss-phase.md`, `plan-phase.md`, `execute-phase.md`, `verify-work.md`, `ship.md`). Generated by `scripts/gen-loop-host-contract.cjs` → `gsd-core/bin/lib/loop-host-contract.cjs` (ADR-894 §3 phase 3a-impl-2). Covers exactly the 12 canonical points (discuss:pre/post, plan:pre/post, execute:pre/wave:pre/wave:post/post, verify:pre/post, ship:pre/post). The generator enforces a drift guard: every declared non-orchestrator agent role must correspond to an actual agent reference in the workflow file. Consumed by `gen-capability-registry.cjs` (replaces the former inline `LOOP_HOST_CONTRACT` constant). Run `node scripts/gen-loop-host-contract.cjs --write` after editing a workflow step marker.
|
||||
|
||||
### Capability Registry
|
||||
Generated central manifest projecting all co-located Capability declarations into one validated artifact for runtime resolution and for the install, surface, config, and loop-extension adapters. Mirrors the research-profiles / package-identity generation pattern (co-located source → generated central file). Generated by `scripts/gen-capability-registry.cjs` → `gsd-core/bin/lib/capability-registry.cjs` (ADR-894 §5 phase 3a-impl). Role-partitioned indexes: `bySkill`, `byAgent`, `byLoopPoint` (hook ordering materialized), `configKeys`, `runtimes`, `requiresClosure(id)`. Validated against the Loop Host Contract (12 points; generated by `gen-loop-host-contract.cjs` from workflow markers, phase 3a-impl-2). Run `node scripts/gen-capability-registry.cjs --write` after editing any `capabilities/<id>/capability.json`.
|
||||
Generated central manifest projecting all co-located Capability declarations into one validated artifact for runtime resolution and for the install, surface, config, and loop-extension adapters. Mirrors the research-profiles / package-identity generation pattern (co-located source → generated central file). Generated by `scripts/gen-capability-registry.cjs` → `gsd-core/bin/lib/capability-registry.cjs` (ADR-894 §5 phase 3a-impl). Role-partitioned indexes: `bySkill`, `byAgent`, `byLoopPoint` (hook ordering materialized), `configKeys` (ownership map: key→capId), `configSchema` (full per-key schema: key→{ owner, type, default, description }), `runtimes`, `requiresClosure(id)`. ADR-857 phase 3b adds `configSchema` with validated type/default/description per key, sourced from each capability's `.config` slice. Validated against the Loop Host Contract (12 points; generated by `gen-loop-host-contract.cjs` from workflow markers, phase 3a-impl-2). Run `node scripts/gen-capability-registry.cjs --write` after editing any `capabilities/<id>/capability.json`.
|
||||
|
||||
### Loop Extension Point [Planned]
|
||||
A named, stable site on a host loop step (per-step `pre`/`post` plus per-wave in Execute; ~12 total) where Capabilities register hooks. Three hook kinds: `step` (runs as its own sequenced unit), `contribution` (injects into the core step's prompt/context), and `gate` (checks and optionally blocks via a declared `blocking` flag). Each hook declares the artifacts it produces and consumes; hook order is derived by topological sort of that produces/consumes graph (capability-id tiebreak), which also defines data flow — file-artifact based, surviving `/clear` and fresh executor contexts. Hooks are surfaced by runtime resolution with concrete projection: the workflow calls a query (extending the `init.*` resolution seam) that resolves the active hooks and returns fully-rendered, ordered markdown for the executor. Failure is default-resilient — a non-gate hook that errors is skipped with a warning; a hook may opt into `onError: halt`. Part of the Capability system.
|
||||
### Federated Config
|
||||
ADR-857 phase 3b seam that merges capability-declared config slices into the `loadConfig` return value. Implemented in `src/federated-config.cts` → `gsd-core/bin/lib/federated-config.cjs`. Exports `mergeFederatedConfig({ configSchema, isCentralKey, userConfig }) → { values, validKeys, warnings }`. Rules: central-schema keys are skipped with a `pending-migration` warning; malformed slices are skipped with a warning (never throws); valid federated keys (absent from the central schema) resolve to the user-supplied value (if type-matches) or the slice default. Object writes are guarded against prototype pollution with inline literal `__proto__`/`constructor`/`prototype` key checks. Wired into `loadConfig` as a true no-op today: every Capability config key is still in the central config-schema, so `isCentralKey()` returns true for all of them and `values` is always empty. The channel becomes live when a key is atomically removed from the central schema at cutover (the ADR-857 migration step). `loadConfig` exposes `_setFederatedRegistryForTests`/`_resetFederatedRegistryForTests` seams for injecting a synthetic registry in tests.
|
||||
|
||||
### Loop Extension Point
|
||||
A named, stable site on a host loop step (per-step `pre`/`post` plus per-wave in Execute; 12 total) where Capabilities register hooks. Three hook kinds: `step` (runs as its own sequenced unit), `contribution` (injects into the core step's prompt/context), and `gate` (checks and optionally blocks via a declared `blocking` flag). Each hook declares the artifacts it produces and consumes; hook order is derived by topological sort of that produces/consumes graph (capability-id tiebreak), which also defines data flow — file-artifact based, surviving `/clear` and fresh executor contexts. Hooks are surfaced by runtime resolution with concrete projection: the workflow calls a query that resolves the active hooks and returns fully-rendered, ordered markdown for the executor. Failure is default-resilient — a non-gate hook that errors is skipped with a warning; a hook may opt into `onError: halt`. Part of the Capability system. ADR-857 phase 3c ships the registry-consuming query layer: `gsd-core/bin/lib/loop-resolver.cjs` exposes `resolveLoopHooks({ point, registry, config })` (pure, no I/O), `renderLoopHooks(resolved)` (pure markdown renderer), and `cmdLoopRenderHooks(cwd, point, raw, opts)` (I/O entry point); activated via `gsd-tools loop render-hooks <point>` which emits `{ point, activeHooks[], rendered }`. Activation is driven by `when` (dotted config key resolved against `loadConfig`), with inline literal `__proto__`/`constructor`/`prototype` prototype-pollution guard. Wiring a workflow to call this query is the ADR-857 phase-6 cutover (out of scope here).
|
||||
|
||||
### Runtime Capability [Planned]
|
||||
A `role: runtime` variant of a Capability (a Capability carries `role: feature | runtime`) that projects GSD's produced artifacts (skills/agents/hooks/commands) onto one host CLI's conventions — config-surface format, artifact-layout kinds, command template, hooks manifest, sandbox tier. It is a declarative descriptor over a fixed first-party primitive vocabulary (not a code adapter); install composes active Feature Capabilities × the chosen Runtime Capability at the InstallPlan seam (ADR-0058). First-party runtimes are authored through the same descriptor a third party would write (dogfooding the interface); tier-1 (Claude Code, Codex, Antigravity) is fully tested, the other existing runtimes ship lower-tier, none dropped. Third-party runtime loading is deferred to a purely additive external loader + trust gate.
|
||||
|
||||
@@ -343,7 +343,8 @@ Node.js CLI utility (`gsd-tools.cjs`) with domain modules split across `gsd-core
|
||||
|
||||
| Module | Responsibility |
|
||||
| ---------------------- | --------------------------------------------------------------------------------------------------- |
|
||||
| `config-loader.cjs` | Project config loading — defaults merge, legacy-key migration, workstream overlay, unknown-key/profile-override validation (extracted from `core.cjs`, ADR-857) |
|
||||
| `config-loader.cjs` | Project config loading — defaults merge, legacy-key migration, workstream overlay, unknown-key/profile-override validation, and federated config overlay (ADR-857 phase 3b) (extracted from `core.cjs`, ADR-857) |
|
||||
| `federated-config.cjs` | Defensive merge of capability-declared config slices (ADR-857 phase 3b); exports `mergeFederatedConfig`; no-op until capability keys are removed from the central config-schema at cutover |
|
||||
| `core-utils.cjs` | Shared low-level utility primitives — POSIX path normalization, sub-repo/subdirectory scanning, phase file stats, slug/one-liner/plan-id helpers, time-ago (extracted from `core.cjs`, ADR-857) |
|
||||
| `core.cjs` | Shared utilities; compatibility re-exports for planning, I/O (`io.cjs`), and phase-id helpers |
|
||||
| `io.cjs` | CLI I/O primitives — output/error emission, JSON-error mode, large-payload temp-file spillover |
|
||||
@@ -372,6 +373,7 @@ Node.js CLI utility (`gsd-tools.cjs`) with domain modules split across `gsd-core
|
||||
| `profile-output.cjs` | Profile rendering, USER-PROFILE.md and dev-preferences.md generation |
|
||||
| `loop-host-contract.cjs` | Generated Loop Host Contract — 12 loop points, per-step agent roles, and core artifacts; emitted by `scripts/gen-loop-host-contract.cjs` from workflow markers (ADR-894 §3); consumed by `gen-capability-registry.cjs` |
|
||||
| `capability-registry.cjs` | Generated central Capability Registry — role-partitioned index of all co-located capability declarations; emitted by `scripts/gen-capability-registry.cjs` (ADR-894 §5) |
|
||||
| `loop-resolver.cjs` | Loop Extension Point resolver — ADR-857 phase 3c registry-consuming query; filters `byLoopPoint` by config activation, renders active hooks as markdown, emits `{ point, activeHooks, rendered }` envelope; `gsd-tools loop render-hooks <point>` |
|
||||
|
||||
|
||||
---
|
||||
|
||||
@@ -293,6 +293,7 @@
|
||||
"docs.cjs",
|
||||
"drift.cjs",
|
||||
"fallow-runner.cjs",
|
||||
"federated-config.cjs",
|
||||
"frontmatter.cjs",
|
||||
"gap-checker.cjs",
|
||||
"graphify.cjs",
|
||||
@@ -308,6 +309,7 @@
|
||||
"learnings.cjs",
|
||||
"legacy-cleanup.cjs",
|
||||
"loop-host-contract.cjs",
|
||||
"loop-resolver.cjs",
|
||||
"milestone.cjs",
|
||||
"model-catalog.cjs",
|
||||
"model-profiles.cjs",
|
||||
|
||||
@@ -370,7 +370,7 @@ The `gsd-planner` agent is decomposed into a core agent plus reference modules t
|
||||
|
||||
---
|
||||
|
||||
## CLI Modules (99 shipped)
|
||||
## CLI Modules (101 shipped)
|
||||
|
||||
Full listing: `gsd-core/bin/lib/*.cjs`.
|
||||
|
||||
@@ -404,6 +404,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`.
|
||||
| `docs.cjs` | Docs-update workflow init, Markdown scanning, monorepo detection |
|
||||
| `drift.cjs` | Post-execute codebase structural drift detector (#2003): classifies file changes into new-dir/barrel/migration/route categories and round-trips `last_mapped_commit` frontmatter |
|
||||
| `fallow-runner.cjs` | Fallow audit adapter for `/gsd-code-review`: binary resolution (`PATH` then `node_modules/.bin`), actionable missing-binary errors, and structural findings normalization |
|
||||
| `federated-config.cjs` | Defensive merge of capability-declared config slices into the loadConfig return value — ADR-857 phase 3b; exports `mergeFederatedConfig({ configSchema, isCentralKey, userConfig })` → `{ values, validKeys, warnings }`; no-op until a key is atomically removed from the central config-schema (the cutover step) |
|
||||
| `frontmatter.cjs` | YAML frontmatter CRUD operations |
|
||||
| `gap-checker.cjs` | Post-planning gap analysis (#2493): unified REQUIREMENTS.md + CONTEXT.md decisions vs PLAN.md coverage report (`gsd-tools gap-analysis`) |
|
||||
| `graphify.cjs` | Knowledge-graph build/query/status/diff for `/gsd-graphify` |
|
||||
@@ -419,6 +420,7 @@ Full listing: `gsd-core/bin/lib/*.cjs`.
|
||||
| `learnings.cjs` | Cross-phase learnings extraction for `/gsd-extract-learnings` |
|
||||
| `legacy-cleanup.cjs` | Detect and remove leftover get-shit-done-cc artifacts; exports `planLegacyCleanup` (pure scan) and `applyLegacyCleanup` (thin IO applier) that root out stale files from the old package across every GSD-managed runtime config directory (#607) |
|
||||
| `loop-host-contract.cjs` | Generated Loop Host Contract — 12 loop points, per-step agent roles, and core artifacts for the five-step pipeline (discuss/plan/execute/verify/ship); emitted by `scripts/gen-loop-host-contract.cjs --write` (ADR-894 §3); consumed by `gen-capability-registry.cjs` |
|
||||
| `loop-resolver.cjs` | Loop Extension Point resolver — ADR-857 phase 3c registry-consuming query; given a canonical loop point, filters `byLoopPoint` by config activation (`when` key traversal with prototype-pollution guard), returns `{ point, activeHooks, rendered }` envelope; `resolveLoopHooks` and `renderLoopHooks` are pure (no I/O); command surface: `gsd-tools loop render-hooks <point>` |
|
||||
| `milestone.cjs` | Milestone archival, requirements marking |
|
||||
| `model-catalog.cjs` | CJS adapter over the shared model catalog JSON; exports canonical runtime tier defaults, agent profile maps, alias maps, and routing metadata for all CLI consumers |
|
||||
| `model-profiles.cjs` | Backward-compatible profile helpers derived from `model-catalog.cjs`; no longer owns its own model table |
|
||||
|
||||
@@ -74,6 +74,8 @@ export default tseslint.config(
|
||||
'gsd-core/bin/lib/config-schema.cjs',
|
||||
'gsd-core/bin/lib/model-profiles.cjs',
|
||||
'gsd-core/bin/lib/model-resolver.cjs',
|
||||
'gsd-core/bin/lib/loop-resolver.cjs',
|
||||
'gsd-core/bin/lib/federated-config.cjs',
|
||||
'gsd-core/bin/lib/installer-migrations/002-codex-legacy-hooks-json.cjs',
|
||||
'gsd-core/bin/lib/installer-migrations/003-rename-get-shit-done-to-gsd-core.cjs',
|
||||
'gsd-core/bin/lib/observability/logger.cjs',
|
||||
|
||||
@@ -163,6 +163,12 @@
|
||||
* learnings prune --older-than <dur> Remove entries older than duration (e.g. 90d)
|
||||
* learnings delete <id> Delete a learning by ID
|
||||
*
|
||||
* Loop Extension Point Queries (ADR-857 phase 3c):
|
||||
* loop render-hooks <point> Resolve + render active Capability hooks at a loop point
|
||||
* Returns JSON envelope { point, activeHooks, rendered }
|
||||
* Valid points: discuss:pre/post, plan:pre/post,
|
||||
* execute:pre/wave:pre/wave:post/post, verify:pre/post, ship:pre/post
|
||||
*
|
||||
* GSD-2 Migration:
|
||||
* from-gsd2 [--path <dir>] [--force] [--dry-run]
|
||||
* Import a GSD-2 (.gsd/) project back to GSD v1 (.planning/) format
|
||||
@@ -201,6 +207,7 @@ const { routeVerifyCommand } = require('./lib/verify-command-router.cjs');
|
||||
const { routeVerificationCommand } = require('./lib/verification-command-router.cjs');
|
||||
const verification = require('./lib/verification.cjs');
|
||||
const { routeInitCommand } = require('./lib/init-command-router.cjs');
|
||||
const loopResolver = require('./lib/loop-resolver.cjs');
|
||||
const { routePhaseCommand } = require('./lib/phase-command-router.cjs');
|
||||
const { routePhasesCommand } = require('./lib/phases-command-router.cjs');
|
||||
const { routeValidateCommand } = require('./lib/validate-command-router.cjs');
|
||||
@@ -379,7 +386,7 @@ async function main() {
|
||||
'current-timestamp, detect-custom-files, docs-init, effort, extract-messages, find-phase, ' +
|
||||
'from-gsd2, frontmatter, gap-analysis, generate-claude-md, generate-claude-profile, ' +
|
||||
'generate-dev-preferences, generate-slug, graphify, history-digest, init, intel, ' +
|
||||
'classify-confidence, learnings, list-todos, milestone, package-legitimacy, phase, phase-plan-index, phases, profile-questionnaire, ' +
|
||||
'classify-confidence, learnings, list-todos, loop, milestone, package-legitimacy, phase, phase-plan-index, phases, profile-questionnaire, ' +
|
||||
'profile-sample, progress, prompt-budget, requirements, research-plan, research-store, resolve-granularity, resolve-model, roadmap, scaffold, state, ' +
|
||||
'task, template, validate, verify, verify-path-exists, verify-summary, workstream, worktree\n\n' +
|
||||
'Global flags:\n' +
|
||||
@@ -1118,6 +1125,20 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand
|
||||
break;
|
||||
}
|
||||
|
||||
case 'loop': {
|
||||
// loop render-hooks <point>
|
||||
const loopSubcommand = args[1];
|
||||
if (loopSubcommand === 'render-hooks') {
|
||||
loopResolver.cmdLoopRenderHooks(cwd, args[2], raw, {});
|
||||
} else {
|
||||
error(
|
||||
`Unknown loop subcommand: ${loopSubcommand}. Available: render-hooks`,
|
||||
core.ERROR_REASON ? core.ERROR_REASON.SDK_UNKNOWN_COMMAND : undefined,
|
||||
);
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
case 'phase-plan-index': {
|
||||
phase.cmdPhasePlanIndex(cwd, args[1], raw);
|
||||
break;
|
||||
|
||||
@@ -207,6 +207,27 @@ const configKeys = {
|
||||
"workflow.ui_safety_gate": "ui"
|
||||
};
|
||||
|
||||
const configSchema = {
|
||||
"workflow.ui_phase": {
|
||||
"owner": "ui",
|
||||
"type": "boolean",
|
||||
"default": true,
|
||||
"description": "Enable the UI design-contract gate during planning."
|
||||
},
|
||||
"workflow.ui_review": {
|
||||
"owner": "ui",
|
||||
"type": "boolean",
|
||||
"default": true,
|
||||
"description": "Enable the retrospective UI audit."
|
||||
},
|
||||
"workflow.ui_safety_gate": {
|
||||
"owner": "ui",
|
||||
"type": "boolean",
|
||||
"default": true,
|
||||
"description": "Block execution on unmet UI-SPEC contracts."
|
||||
}
|
||||
};
|
||||
|
||||
const runtimes = {};
|
||||
|
||||
const _requiresGraph = {
|
||||
@@ -236,6 +257,7 @@ module.exports = {
|
||||
byAgent,
|
||||
byLoopPoint,
|
||||
configKeys,
|
||||
configSchema,
|
||||
runtimes,
|
||||
requiresClosure,
|
||||
};
|
||||
|
||||
@@ -21,8 +21,11 @@ Orchestrator coordinates, not executes. Each subagent loads the full execute-pla
|
||||
instead of spawning parallel agents. Only attempt parallel spawning if the user
|
||||
explicitly requests it — and in that case, rely on the spot-check fallback in step 3
|
||||
to detect completion.
|
||||
- **Other runtimes:** If `Agent`/`agent` tool is unavailable, use sequential inline execution as the
|
||||
fallback. Check for tool availability at runtime rather than assuming based on runtime name.
|
||||
- **Other runtimes:** If `Agent`/`agent` tool is genuinely unavailable (e.g. a backgrounded
|
||||
Claude Code agent per #853, or a non-Claude runtime), use sequential inline execution as
|
||||
the fallback for executor parallelization only. If `Agent` IS available (top-level Claude
|
||||
Code), you MUST spawn gsd-executor agents — inline execution is not authorized. Check for
|
||||
actual tool availability, not runtime name.
|
||||
|
||||
**Fallback rule:** If a spawned agent completes its work (commits visible, SUMMARY.md exists) but
|
||||
the orchestrator never receives the completion signal, treat it as successful based on spot-checks
|
||||
|
||||
@@ -27,6 +27,31 @@ Valid GSD subagent types (use exact names — do not fall back to 'general-purpo
|
||||
- gsd-plan-checker — Reviews plan quality before execution
|
||||
</available_agent_types>
|
||||
|
||||
<runtime_compatibility>
|
||||
**Subagent spawning — top-level Claude Code:**
|
||||
The Agent tool IS available in a top-level Claude Code session. Always spawn
|
||||
gsd-phase-researcher, gsd-planner, and gsd-plan-checker as separate Agent() calls.
|
||||
Never absorb these roles inline. Role separation is required regardless of `--chain`
|
||||
or `--auto` — those options suppress interactive prompts only; they NEVER authorize
|
||||
collapsing plan roles into the orchestrator context.
|
||||
|
||||
**Backgrounded Claude Code (via manager/autonomous):**
|
||||
The calling workflow (manager.md / autonomous.md) already runs plan-phase inline via
|
||||
Skill() on Claude Code so that the plan-checker subagent can still spawn. plan-phase
|
||||
itself does not need to detect this case.
|
||||
|
||||
**#1009 caveat (discuss-phase early-exit):**
|
||||
The "display the command and exit" instruction near `## 4` applies only to the
|
||||
discuss-phase early-exit path. It does NOT authorize inline role performance for any
|
||||
plan-phase agents.
|
||||
|
||||
**Other runtimes:**
|
||||
If the Agent tool is genuinely absent (e.g. a backgrounded Claude Code agent per
|
||||
#853, or a non-Claude runtime that does not expose Agent/agent), log the gap and
|
||||
stop — do NOT perform researcher/planner/checker roles inline. Independent agent
|
||||
contexts are required for the plan-checker gate to be meaningful.
|
||||
</runtime_compatibility>
|
||||
|
||||
<process>
|
||||
|
||||
## 0. Git Branch Invariant
|
||||
@@ -537,7 +562,7 @@ Agent(
|
||||
)
|
||||
```
|
||||
|
||||
> **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available.
|
||||
> **ORCHESTRATOR RULE — ALL RUNTIMES**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available.
|
||||
|
||||
### Handle Researcher Return
|
||||
|
||||
@@ -856,7 +881,7 @@ Agent(
|
||||
)
|
||||
```
|
||||
|
||||
> **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available.
|
||||
> **ORCHESTRATOR RULE — ALL RUNTIMES**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available.
|
||||
|
||||
**Handle return:**
|
||||
- **`## PATTERN MAPPING COMPLETE`:** Update `PATTERNS_PATH` to the created file path, continue to step 8.
|
||||
@@ -1019,7 +1044,7 @@ Agent(
|
||||
)
|
||||
```
|
||||
|
||||
> **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available.
|
||||
> **ORCHESTRATOR RULE — ALL RUNTIMES**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available.
|
||||
|
||||
**If `CHUNKED_MODE` is `true`:** Skip the Agent() call above — proceed to step 8.5 instead.
|
||||
|
||||
@@ -1075,7 +1100,7 @@ Agent(
|
||||
)
|
||||
```
|
||||
|
||||
> **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available.
|
||||
> **ORCHESTRATOR RULE — ALL RUNTIMES**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available.
|
||||
|
||||
Handle return:
|
||||
- **`## OUTLINE COMPLETE`:** Read `PLAN-OUTLINE.md`, extract plan list. Continue to 8.5.2.
|
||||
@@ -1119,7 +1144,7 @@ For each plan entry extracted from `PLAN-OUTLINE.md`:
|
||||
)
|
||||
```
|
||||
|
||||
> **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available.
|
||||
> **ORCHESTRATOR RULE — ALL RUNTIMES**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available.
|
||||
|
||||
4. **Verify disk:** Check `${PHASE_DIR}/{plan_id}-PLAN.md` exists. If missing: offer 1) Retry, 2) Stop.
|
||||
|
||||
@@ -1277,7 +1302,7 @@ Agent(
|
||||
)
|
||||
```
|
||||
|
||||
> **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available.
|
||||
> **ORCHESTRATOR RULE — ALL RUNTIMES**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available.
|
||||
|
||||
## 11. Handle Checker Return
|
||||
|
||||
@@ -1392,7 +1417,7 @@ Agent(
|
||||
)
|
||||
```
|
||||
|
||||
> **ORCHESTRATOR RULE — CODEX RUNTIME**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available.
|
||||
> **ORCHESTRATOR RULE — ALL RUNTIMES**: After calling Agent() above, stop working on this task immediately. Do not read more files, edit code, or run tests related to this task while the subagent is active. Wait for the subagent to return its result. This prevents duplicate work, conflicting edits, and wasted context. Only resume when the subagent result is available.
|
||||
|
||||
After planner returns -> spawn checker again (step 10), increment iteration_count.
|
||||
|
||||
|
||||
@@ -105,6 +105,100 @@ function loadCentralConfigKeys() {
|
||||
}
|
||||
}
|
||||
|
||||
// ─── Config-slice validation ──────────────────────────────────────────────────
|
||||
|
||||
const VALID_CONFIG_SLICE_TYPES = new Set(['boolean', 'string', 'number', 'enum']);
|
||||
|
||||
/**
|
||||
* Validate a single config-slice entry (one key's { type, default, description }).
|
||||
* Returns an array of error strings. Empty = valid.
|
||||
*
|
||||
* @param {string} capId Capability id (for error messages)
|
||||
* @param {string} key Config key (for error messages)
|
||||
* @param {object} slice The slice object from cap.config[key]
|
||||
* @returns {string[]}
|
||||
*/
|
||||
function validateConfigSliceEntry(capId, key, slice) {
|
||||
const errors = [];
|
||||
|
||||
if (typeof slice !== 'object' || slice === null || Array.isArray(slice)) {
|
||||
errors.push('capability "' + capId + '" config["' + key + '"]: slice must be a non-null object');
|
||||
return errors;
|
||||
}
|
||||
|
||||
// type must be one of the allowed set
|
||||
if (!VALID_CONFIG_SLICE_TYPES.has(slice.type)) {
|
||||
errors.push(
|
||||
'capability "' + capId + '" config["' + key + '"]: type must be one of ' +
|
||||
[...VALID_CONFIG_SLICE_TYPES].join(', ') + ' (got: ' + JSON.stringify(slice.type) + ')',
|
||||
);
|
||||
}
|
||||
|
||||
// default must be present
|
||||
if (!Object.prototype.hasOwnProperty.call(slice, 'default')) {
|
||||
errors.push(
|
||||
'capability "' + capId + '" config["' + key + '"]: default is required',
|
||||
);
|
||||
} else {
|
||||
// type-consistency check
|
||||
const def = slice.default;
|
||||
if (slice.type === 'boolean') {
|
||||
if (typeof def !== 'boolean') {
|
||||
errors.push(
|
||||
'capability "' + capId + '" config["' + key + '"]: default must be a boolean for type:"boolean" (got: ' + typeof def + ')',
|
||||
);
|
||||
}
|
||||
} else if (slice.type === 'string') {
|
||||
if (typeof def !== 'string') {
|
||||
errors.push(
|
||||
'capability "' + capId + '" config["' + key + '"]: default must be a string for type:"string" (got: ' + typeof def + ')',
|
||||
);
|
||||
}
|
||||
} else if (slice.type === 'number') {
|
||||
if (typeof def !== 'number') {
|
||||
errors.push(
|
||||
'capability "' + capId + '" config["' + key + '"]: default must be a number for type:"number" (got: ' + typeof def + ')',
|
||||
);
|
||||
} else if (!Number.isFinite(def)) {
|
||||
// FIX 6a: Reject NaN and non-finite number defaults
|
||||
errors.push(
|
||||
'capability "' + capId + '" config["' + key + '"]: default for type:"number" must be a finite number (got: ' + String(def) + ')',
|
||||
);
|
||||
}
|
||||
} else if (slice.type === 'enum') {
|
||||
// FIX 5a: enum REQUIRES a non-empty values array (all strings), and default must be in it
|
||||
if (!Array.isArray(slice.values) || slice.values.length === 0) {
|
||||
errors.push(
|
||||
'capability "' + capId + '" config["' + key + '"]: type:"enum" requires a non-empty "values" array of strings',
|
||||
);
|
||||
} else if (!slice.values.every((v) => typeof v === 'string')) {
|
||||
errors.push(
|
||||
'capability "' + capId + '" config["' + key + '"]: type:"enum" values array must contain only strings',
|
||||
);
|
||||
}
|
||||
if (typeof def !== 'string') {
|
||||
errors.push(
|
||||
'capability "' + capId + '" config["' + key + '"]: default must be a string for type:"enum" (got: ' + typeof def + ')',
|
||||
);
|
||||
} else if (Array.isArray(slice.values) && slice.values.length > 0 && !slice.values.includes(def)) {
|
||||
errors.push(
|
||||
'capability "' + capId + '" config["' + key + '"]: default "' + def +
|
||||
'" is not one of the declared enum values [' + slice.values.join(', ') + ']',
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// description must be a non-empty string
|
||||
if (typeof slice.description !== 'string' || slice.description.length === 0) {
|
||||
errors.push(
|
||||
'capability "' + capId + '" config["' + key + '"]: description must be a non-empty string (got: ' + JSON.stringify(slice.description) + ')',
|
||||
);
|
||||
}
|
||||
|
||||
return errors;
|
||||
}
|
||||
|
||||
// ─── Per-capability validation ────────────────────────────────────────────────
|
||||
|
||||
const KEBAB_RE = /^[a-z][a-z0-9-]*$/;
|
||||
@@ -951,6 +1045,7 @@ function buildRegistry(capMap) {
|
||||
const byAgent = Object.create(null);
|
||||
const byLoopPoint = Object.create(null);
|
||||
const configKeys = Object.create(null);
|
||||
const configSchema = Object.create(null);
|
||||
const runtimes = Object.create(null);
|
||||
|
||||
// Initialize byLoopPoint for all valid points
|
||||
@@ -989,6 +1084,29 @@ function buildRegistry(capMap) {
|
||||
// S2b: inline literal guard at each write site (CodeQL barrier)
|
||||
if (key === '__proto__' || key === 'constructor' || key === 'prototype') continue;
|
||||
configKeys[key] = capId;
|
||||
|
||||
// Build configSchema entry — validate the slice first (throw on violation)
|
||||
const slice = (cap.config || {})[key];
|
||||
const sliceErrors = validateConfigSliceEntry(capId, key, slice);
|
||||
if (sliceErrors.length > 0) {
|
||||
throw new Error(
|
||||
'configSchema validation failed during registry build:\n' +
|
||||
sliceErrors.map((e) => ' ' + e).join('\n'),
|
||||
);
|
||||
}
|
||||
// S2b: inline literal guard for configSchema write site
|
||||
if (key !== '__proto__' && key !== 'constructor' && key !== 'prototype') {
|
||||
configSchema[key] = {
|
||||
owner: capId,
|
||||
type: slice.type,
|
||||
default: slice.default,
|
||||
description: slice.description,
|
||||
};
|
||||
// Preserve values array for enum types if present
|
||||
if (slice.type === 'enum' && Array.isArray(slice.values)) {
|
||||
configSchema[key].values = slice.values;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for (const step of (cap.steps || [])) {
|
||||
@@ -1050,6 +1168,7 @@ function buildRegistry(capMap) {
|
||||
byAgent,
|
||||
byLoopPoint,
|
||||
configKeys,
|
||||
configSchema,
|
||||
runtimes,
|
||||
};
|
||||
}
|
||||
@@ -1085,6 +1204,8 @@ function serializeRegistry(registry, capMap) {
|
||||
lines.push('');
|
||||
lines.push('const configKeys = ' + JSON.stringify(registry.configKeys, null, 2) + ';');
|
||||
lines.push('');
|
||||
lines.push('const configSchema = ' + JSON.stringify(registry.configSchema, null, 2) + ';');
|
||||
lines.push('');
|
||||
lines.push('const runtimes = ' + JSON.stringify(registry.runtimes, null, 2) + ';');
|
||||
lines.push('');
|
||||
|
||||
@@ -1121,6 +1242,7 @@ function serializeRegistry(registry, capMap) {
|
||||
lines.push(' byAgent,');
|
||||
lines.push(' byLoopPoint,');
|
||||
lines.push(' configKeys,');
|
||||
lines.push(' configSchema,');
|
||||
lines.push(' runtimes,');
|
||||
lines.push(' requiresClosure,');
|
||||
lines.push('};');
|
||||
@@ -1280,6 +1402,8 @@ module.exports = {
|
||||
computeRequiresClosure,
|
||||
topoSortSteps,
|
||||
normalizeLineEndings,
|
||||
validateConfigSliceEntry,
|
||||
VALID_CONFIG_SLICE_TYPES,
|
||||
LOOP_HOST_CONTRACT,
|
||||
VALID_LOOP_POINTS,
|
||||
POINT_ORDER,
|
||||
|
||||
@@ -35,8 +35,30 @@ const { detectSubRepos } = coreUtilsModule;
|
||||
import { CONFIG_DEFAULTS as CANONICAL_CONFIG_DEFAULTS, normalizeLegacyKeys } from './configuration.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import configSchema = require('./config-schema.cjs');
|
||||
const { VALID_CONFIG_KEYS, DYNAMIC_KEY_PATTERNS } = configSchema;
|
||||
const { VALID_CONFIG_KEYS, DYNAMIC_KEY_PATTERNS, isValidConfigKey: _isValidConfigKeyFn } = configSchema;
|
||||
import { KNOWN_RUNTIMES, KNOWN_PROVIDERS } from './model-catalog.cjs';
|
||||
// ─── Federated Config (ADR-857 phase 3b) ─────────────────────────────────────
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import federatedConfigModule = require('./federated-config.cjs');
|
||||
const { mergeFederatedConfig } = federatedConfigModule;
|
||||
// The capability-registry.cjs is generated and lives in the same gsd-core/bin/lib/ output dir.
|
||||
// Both config-loader.cjs and capability-registry.cjs land in gsd-core/bin/lib/ at build time.
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/no-unsafe-assignment
|
||||
const _capabilityRegistryReal: { configSchema?: Record<string, unknown> } = require('./capability-registry.cjs');
|
||||
|
||||
// Module-level registry reference. Defaults to the real generated registry.
|
||||
// Overridable for tests via _setFederatedRegistryForTests.
|
||||
let _capabilityRegistry: { configSchema?: Record<string, unknown> } = _capabilityRegistryReal;
|
||||
|
||||
/** Test-only seam: inject a synthetic registry. Call _resetFederatedRegistryForTests() to restore. */
|
||||
function _setFederatedRegistryForTests(reg: { configSchema?: Record<string, unknown> }): void {
|
||||
_capabilityRegistry = reg;
|
||||
}
|
||||
|
||||
/** Test-only seam: restore the real generated registry. */
|
||||
function _resetFederatedRegistryForTests(): void {
|
||||
_capabilityRegistry = _capabilityRegistryReal;
|
||||
}
|
||||
|
||||
// ─── File & Config utilities ──────────────────────────────────────────────────
|
||||
|
||||
@@ -272,6 +294,81 @@ function _resetRuntimeWarningCacheForTests(): void {
|
||||
_warnedConfigKeys.clear();
|
||||
}
|
||||
|
||||
// ─── FIX 2: Federated overlay helpers ────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Apply federated key values into a mutable config object.
|
||||
* Handles N-level dotted keys (e.g. "a.b.c" → obj.a.b.c).
|
||||
* Only adds keys that are not already present (does not clobber).
|
||||
* Inline prototype-pollution guards at every segment.
|
||||
*/
|
||||
function _applyFederatedValues(
|
||||
obj: Record<string, unknown>,
|
||||
values: Record<string, unknown>,
|
||||
validKeys: string[],
|
||||
): void {
|
||||
for (const dottedKey of validKeys) {
|
||||
// S2: inline literal guard on full key
|
||||
if (dottedKey === '__proto__' || dottedKey === 'constructor' || dottedKey === 'prototype') continue;
|
||||
const parts = dottedKey.split('.');
|
||||
if (parts.length === 1) {
|
||||
const topKey = parts[0];
|
||||
if (topKey !== '__proto__' && topKey !== 'constructor' && topKey !== 'prototype') {
|
||||
if (!Object.prototype.hasOwnProperty.call(obj, topKey)) {
|
||||
obj[topKey] = values[dottedKey];
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// N-level nested key: traverse/create intermediate objects
|
||||
let cur: Record<string, unknown> = obj;
|
||||
let ok = true;
|
||||
for (let i = 0; i < parts.length - 1; i++) {
|
||||
const seg = parts[i];
|
||||
// S2: inline literal guard on each segment
|
||||
if (seg === '__proto__' || seg === 'constructor' || seg === 'prototype') { ok = false; break; }
|
||||
if (!Object.prototype.hasOwnProperty.call(cur, seg) || cur[seg] === null) {
|
||||
cur[seg] = {};
|
||||
}
|
||||
if (typeof cur[seg] !== 'object' || Array.isArray(cur[seg])) { ok = false; break; }
|
||||
cur = cur[seg] as Record<string, unknown>;
|
||||
}
|
||||
if (!ok) continue;
|
||||
const leafKey = parts[parts.length - 1];
|
||||
// S2: inline literal guard on leaf
|
||||
if (leafKey === '__proto__' || leafKey === 'constructor' || leafKey === 'prototype') continue;
|
||||
if (!Object.prototype.hasOwnProperty.call(cur, leafKey)) {
|
||||
cur[leafKey] = values[dottedKey];
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* FIX 2: Apply the federated overlay to a base config object.
|
||||
* When validKeys is empty (current registry — all keys are central),
|
||||
* returns the baseConfig UNCHANGED (true no-op, preserves reference identity).
|
||||
* When validKeys is non-empty, applies values into a shallow clone to avoid
|
||||
* mutating shared CONFIG_DEFAULTS/module constants.
|
||||
*/
|
||||
function _applyFederatedOverlay(
|
||||
baseConfig: Record<string, unknown>,
|
||||
userConfig: Record<string, unknown>,
|
||||
): Record<string, unknown> {
|
||||
const _fedRegistrySchema = _capabilityRegistry.configSchema;
|
||||
if (!_fedRegistrySchema || typeof _fedRegistrySchema !== 'object') return baseConfig;
|
||||
const _fedOverlay = mergeFederatedConfig({
|
||||
configSchema: _fedRegistrySchema,
|
||||
isCentralKey: (key: string) => _isValidConfigKeyFn(key),
|
||||
userConfig,
|
||||
});
|
||||
// True no-op: if no federated keys, return UNCHANGED (byte-identical, no clone)
|
||||
if (_fedOverlay.validKeys.length === 0) return baseConfig;
|
||||
// Clone shallowly to avoid mutating shared constants, then apply nested values
|
||||
const cloned: Record<string, unknown> = { ...baseConfig };
|
||||
_applyFederatedValues(cloned, _fedOverlay.values, _fedOverlay.validKeys);
|
||||
return cloned;
|
||||
}
|
||||
|
||||
function loadConfig(cwd: string, options: Record<string, unknown> = {}): Record<string, unknown> {
|
||||
const activeWorkstream = Object.prototype.hasOwnProperty.call(options, 'workstream')
|
||||
? options['workstream']
|
||||
@@ -397,6 +494,31 @@ function loadConfig(cwd: string, options: Record<string, unknown> = {}): Record<
|
||||
// Deprecated keys (still accepted for migration, not in config-set)
|
||||
'depth', 'multiRepo', 'branching_strategy',
|
||||
]);
|
||||
|
||||
// FIX 3: Compute federated overlay BEFORE the unknown-key warning, so that
|
||||
// federated top-level keys are added to KNOWN_TOP_LEVEL before the check runs.
|
||||
// This is hoisted out of the try-catch below so validKeys are available here.
|
||||
let _preWarningFedValidKeys: string[] = [];
|
||||
try {
|
||||
const _fedRegistrySchemaEarly = _capabilityRegistry.configSchema;
|
||||
if (_fedRegistrySchemaEarly && typeof _fedRegistrySchemaEarly === 'object') {
|
||||
const _earlyOverlay = mergeFederatedConfig({
|
||||
configSchema: _fedRegistrySchemaEarly,
|
||||
isCentralKey: (key: string) => _isValidConfigKeyFn(key),
|
||||
userConfig: parsed,
|
||||
});
|
||||
_preWarningFedValidKeys = _earlyOverlay.validKeys;
|
||||
for (const dottedKey of _preWarningFedValidKeys) {
|
||||
const topKey = dottedKey.split('.')[0];
|
||||
if (topKey !== '__proto__' && topKey !== 'constructor' && topKey !== 'prototype') {
|
||||
KNOWN_TOP_LEVEL.add(topKey);
|
||||
}
|
||||
}
|
||||
}
|
||||
} catch {
|
||||
// Defensive: if registry access fails here, proceed without pre-warning keys
|
||||
}
|
||||
|
||||
const unknownKeys = Object.keys(parsed).filter(k => !KNOWN_TOP_LEVEL.has(k));
|
||||
if (unknownKeys.length > 0) {
|
||||
const warnKey = unknownKeys.join(',');
|
||||
@@ -429,7 +551,7 @@ function loadConfig(cwd: string, options: Record<string, unknown> = {}): Record<
|
||||
return defaults.parallelization;
|
||||
})();
|
||||
|
||||
return {
|
||||
const _baseConfig: Record<string, unknown> = {
|
||||
model_profile: get('model_profile') ?? defaults.model_profile,
|
||||
commit_docs: (() => {
|
||||
const explicit = get('commit_docs', { section: 'planning', field: 'commit_docs' });
|
||||
@@ -492,14 +614,54 @@ function loadConfig(cwd: string, options: Record<string, unknown> = {}): Record<
|
||||
claude_md_path: get('claude_md_path') || null,
|
||||
claude_md_assembly: (parsed['claude_md_assembly']) || null,
|
||||
};
|
||||
|
||||
// ─── ADR-857 phase 3b: federated config overlay ───────────────────────────
|
||||
// FIX 2: Use the pre-computed _preWarningFedValidKeys (from the FIX 3 block above)
|
||||
// plus a fresh overlay call to get values. The KNOWN_TOP_LEVEL was already updated.
|
||||
// TODAY: every UI key is still in the central config-schema, so isCentralKey()
|
||||
// returns true for all of them → validKeys is empty → _baseConfig is returned UNCHANGED
|
||||
// (true no-op: no clone, no reorder, byte-identical output).
|
||||
// This becomes a live channel once a key is atomically removed from the central schema.
|
||||
try {
|
||||
if (_preWarningFedValidKeys.length > 0) {
|
||||
// There are actual federated values — re-use the already-computed overlay
|
||||
// (we run mergeFederatedConfig again here to get the values map; the validKeys
|
||||
// are guaranteed identical since it's the same inputs).
|
||||
const _fedRegistrySchema = _capabilityRegistry.configSchema;
|
||||
if (_fedRegistrySchema && typeof _fedRegistrySchema === 'object') {
|
||||
const _fedOverlay = mergeFederatedConfig({
|
||||
configSchema: _fedRegistrySchema,
|
||||
isCentralKey: (key: string) => _isValidConfigKeyFn(key),
|
||||
userConfig: parsed,
|
||||
});
|
||||
// Apply dotted-path values (e.g. "workflow.ui_phase" → _baseConfig.workflow.ui_phase)
|
||||
// WITHOUT clobbering existing keys. N-level nesting supported.
|
||||
_applyFederatedValues(_baseConfig, _fedOverlay.values, _fedOverlay.validKeys);
|
||||
}
|
||||
}
|
||||
// Pending-migration warnings are suppressed at load time to avoid noisy output on
|
||||
// every loadConfig call. They are surfaced at registry-generation time (--check/--write).
|
||||
} catch {
|
||||
// Defensive: if the federated overlay throws for any reason, return the base config unchanged.
|
||||
// This keeps loadConfig's no-throw contract intact regardless of capability registry state.
|
||||
}
|
||||
return _baseConfig;
|
||||
} catch {
|
||||
// Fall back to ~/.gsd/defaults.json only for truly pre-project contexts (#1683)
|
||||
if (fs.existsSync(planningDir(cwd, ws))) {
|
||||
if (rootParsed) {
|
||||
// Workstream has no config.json: re-parse using root config as the sole source.
|
||||
// (FIX 2: overlay is applied recursively in the re-entrant loadConfig call)
|
||||
return loadConfig(cwd, { workstream: null });
|
||||
}
|
||||
return defaults;
|
||||
// FIX 2: Apply the federated overlay on the no-config path.
|
||||
// With the current registry (all keys central), _applyFederatedOverlay returns
|
||||
// `defaults` UNCHANGED (true no-op, preserves byte-identical output).
|
||||
try {
|
||||
return _applyFederatedOverlay(defaults, {});
|
||||
} catch {
|
||||
return defaults;
|
||||
}
|
||||
}
|
||||
try {
|
||||
const home = process.env['GSD_HOME'] || os.homedir();
|
||||
@@ -507,7 +669,7 @@ function loadConfig(cwd: string, options: Record<string, unknown> = {}): Record<
|
||||
const raw = platformReadSync(globalDefaultsPath);
|
||||
if (raw === null) throw new Error('missing');
|
||||
const globalDefaults = JSON.parse(raw) as Record<string, unknown>;
|
||||
return {
|
||||
const _globalBaseCfg: Record<string, unknown> = {
|
||||
...defaults,
|
||||
model_profile: (globalDefaults['model_profile']) ?? defaults.model_profile,
|
||||
commit_docs: (globalDefaults['commit_docs']) ?? defaults.commit_docs,
|
||||
@@ -534,8 +696,21 @@ function loadConfig(cwd: string, options: Record<string, unknown> = {}): Record<
|
||||
agent_skills: (globalDefaults['agent_skills']) || {},
|
||||
response_language: (globalDefaults['response_language']) || null,
|
||||
};
|
||||
// FIX 2: Apply federated overlay on global-defaults path.
|
||||
// With the current registry this is a true no-op (returns _globalBaseCfg unchanged).
|
||||
try {
|
||||
return _applyFederatedOverlay(_globalBaseCfg, globalDefaults);
|
||||
} catch {
|
||||
return _globalBaseCfg;
|
||||
}
|
||||
} catch {
|
||||
return defaults;
|
||||
// FIX 2: Apply federated overlay on the final fallback path.
|
||||
// With the current registry this is a true no-op (returns `defaults` unchanged).
|
||||
try {
|
||||
return _applyFederatedOverlay(defaults, {});
|
||||
} catch {
|
||||
return defaults;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -553,4 +728,6 @@ export = {
|
||||
_warnedConfigKeys,
|
||||
_gitIgnoredCache,
|
||||
RUNTIME_OVERRIDE_TIERS,
|
||||
_setFederatedRegistryForTests,
|
||||
_resetFederatedRegistryForTests,
|
||||
};
|
||||
|
||||
230
src/federated-config.cts
Normal file
230
src/federated-config.cts
Normal file
@@ -0,0 +1,230 @@
|
||||
/**
|
||||
* Federated Config — Defensive merge of capability-declared config keys
|
||||
*
|
||||
* ADR-857 phase 3b: wires the Capability Registry's configSchema into
|
||||
* loadConfig as a provably-empty no-op channel until capability keys are
|
||||
* migrated out of the central config-schema.
|
||||
*
|
||||
* Exported function:
|
||||
* mergeFederatedConfig({ configSchema, isCentralKey, userConfig })
|
||||
* → { values, validKeys, warnings }
|
||||
*
|
||||
* Design:
|
||||
* - For each key in configSchema:
|
||||
* If isCentralKey(key) → SKIP; push a pending-migration warning.
|
||||
* Else if slice is malformed → SKIP; push a warning. Never throw.
|
||||
* Else (valid federated key absent from central):
|
||||
* resolvedValue = nested userConfig lookup if present & type-matches; else slice.default.
|
||||
* Add key→resolvedValue to values; add key to validKeys.
|
||||
* - Guard all object writes with inline literal __proto__/constructor/prototype checks.
|
||||
* - Zero external dependencies; no ajv; hand-rolled type checks only.
|
||||
*
|
||||
* ADR-857 no-op guarantee:
|
||||
* With the current registry, every UI key is still present in the central
|
||||
* config-schema, so isCentralKey() returns true for all of them and values
|
||||
* is always empty. The channel is live but carries no traffic until a key
|
||||
* is atomically removed from the central schema (the cutover step).
|
||||
*
|
||||
* Dependencies: none (zero-dep module).
|
||||
*/
|
||||
|
||||
// ─── Types ─────────────────────────────────────────────────────────────────────
|
||||
|
||||
/** Shape of one entry in the capability registry's configSchema index. */
|
||||
interface ConfigSliceEntry {
|
||||
owner: string;
|
||||
type: string;
|
||||
default: unknown;
|
||||
description: string;
|
||||
values?: string[]; // required for enum type
|
||||
[key: string]: unknown;
|
||||
}
|
||||
|
||||
interface MergeFederatedConfigInput {
|
||||
/** configSchema index from the capability registry: { [key]: ConfigSliceEntry } */
|
||||
configSchema: Record<string, unknown>;
|
||||
/** Returns true if the given key is owned by the central config-schema. */
|
||||
isCentralKey: (key: string) => boolean;
|
||||
/** The raw/merged user config object already loaded in loadConfig. */
|
||||
userConfig: Record<string, unknown>;
|
||||
}
|
||||
|
||||
interface MergeFederatedConfigResult {
|
||||
/** Resolved values for federated (non-central) keys: { key → resolvedValue } */
|
||||
values: Record<string, unknown>;
|
||||
/** Array of keys that are now valid federated keys (i.e. were added to values). */
|
||||
validKeys: string[];
|
||||
/** Human-readable diagnostic strings (pending-migration, malformed-slice, type-mismatch). */
|
||||
warnings: string[];
|
||||
}
|
||||
|
||||
// ─── Allowed slice types (mirrors gen-capability-registry.cjs VALID_CONFIG_SLICE_TYPES) ──
|
||||
|
||||
const VALID_SLICE_TYPES = new Set<string>(['boolean', 'string', 'number', 'enum']);
|
||||
|
||||
// ─── Internal helpers ──────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Returns true if `slice` has a non-empty type, a `default` property, and a
|
||||
* non-empty string description. Does NOT throw.
|
||||
*/
|
||||
function _isWellFormedSlice(slice: unknown): slice is ConfigSliceEntry {
|
||||
if (typeof slice !== 'object' || slice === null || Array.isArray(slice)) return false;
|
||||
const s = slice as Record<string, unknown>;
|
||||
if (typeof s['type'] !== 'string' || s['type'].length === 0) return false;
|
||||
if (!VALID_SLICE_TYPES.has(s['type'])) return false;
|
||||
if (!Object.prototype.hasOwnProperty.call(s, 'default')) return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true if `value` matches the declared type in the slice.
|
||||
* For enum, also validates against slice.values if present.
|
||||
*/
|
||||
function _typeMatches(value: unknown, slice: ConfigSliceEntry): boolean {
|
||||
switch (slice.type) {
|
||||
case 'boolean': return typeof value === 'boolean';
|
||||
case 'string': return typeof value === 'string';
|
||||
case 'number': return typeof value === 'number';
|
||||
case 'enum':
|
||||
// Must be a string AND, if values list is present, must be in it
|
||||
if (typeof value !== 'string') return false;
|
||||
if (Array.isArray(slice.values) && slice.values.length > 0) {
|
||||
return slice.values.includes(value);
|
||||
}
|
||||
return true;
|
||||
default: return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Traverse a dotted key path through a nested config object.
|
||||
* E.g. key="workflow.ui_phase", obj={workflow:{ui_phase:false}} → {found:true, value:false}
|
||||
* Returns {found:false} if any segment is missing or not an own property.
|
||||
* Handles 1, 2, or N segments generically.
|
||||
*/
|
||||
function _getNestedValue(obj: Record<string, unknown>, key: string): { found: boolean; value: unknown } {
|
||||
const segments = key.split('.');
|
||||
let current: unknown = obj;
|
||||
for (let i = 0; i < segments.length; i++) {
|
||||
const seg = segments[i];
|
||||
// Inline literal prototype-pollution guard
|
||||
if (seg === '__proto__' || seg === 'constructor' || seg === 'prototype') {
|
||||
return { found: false, value: undefined };
|
||||
}
|
||||
if (typeof current !== 'object' || current === null) {
|
||||
return { found: false, value: undefined };
|
||||
}
|
||||
const cur = current as Record<string, unknown>;
|
||||
if (!Object.prototype.hasOwnProperty.call(cur, seg)) {
|
||||
return { found: false, value: undefined };
|
||||
}
|
||||
current = cur[seg];
|
||||
}
|
||||
return { found: true, value: current };
|
||||
}
|
||||
|
||||
// ─── Public API ────────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Defensive merge of capability-declared config slices into the loadConfig
|
||||
* return value.
|
||||
*
|
||||
* DEFENSIVE contract (never throws, even on bad capability data):
|
||||
* - Null/undefined/non-object input → returns empty result.
|
||||
* - Null/undefined/non-object userConfig → treated as {} (no overrides).
|
||||
* - Central keys are skipped with a pending-migration warning.
|
||||
* - Malformed slices are skipped with a warning.
|
||||
* - User-supplied values with wrong types (or out-of-enum values) fall back
|
||||
* to the slice default (a type-mismatch warning is pushed but the key is
|
||||
* still federated with its default; this is best-effort degraded operation).
|
||||
*/
|
||||
function mergeFederatedConfig(input: MergeFederatedConfigInput): MergeFederatedConfigResult {
|
||||
// FIX 4: Guard null/undefined/non-object input
|
||||
if (input === null || input === undefined || typeof input !== 'object') {
|
||||
return { values: Object.create(null) as Record<string, unknown>, validKeys: [], warnings: [] };
|
||||
}
|
||||
|
||||
const { configSchema, isCentralKey } = input;
|
||||
|
||||
// FIX 4: Guard null/undefined/non-object userConfig — treat as {}
|
||||
const userConfig: Record<string, unknown> =
|
||||
(input.userConfig !== null && input.userConfig !== undefined && typeof input.userConfig === 'object' && !Array.isArray(input.userConfig))
|
||||
? input.userConfig
|
||||
: {};
|
||||
|
||||
// FIX 6b: Use null-prototype object for all return paths
|
||||
const values: Record<string, unknown> = Object.create(null) as Record<string, unknown>;
|
||||
const validKeys: string[] = [];
|
||||
const warnings: string[] = [];
|
||||
|
||||
if (typeof configSchema !== 'object' || configSchema === null) {
|
||||
return { values: Object.create(null) as Record<string, unknown>, validKeys: [], warnings: [] };
|
||||
}
|
||||
|
||||
for (const key of Object.keys(configSchema)) {
|
||||
// S2: inline literal prototype-pollution guard (CodeQL barrier)
|
||||
// Guard both the full key AND all dotted-path segments
|
||||
if (key === '__proto__' || key === 'constructor' || key === 'prototype') continue;
|
||||
const _keySegments = key.split('.');
|
||||
if (_keySegments.some((s) => s === '__proto__' || s === 'constructor' || s === 'prototype')) continue;
|
||||
|
||||
const slice = configSchema[key];
|
||||
|
||||
// If this key is still in the central schema → pending migration, skip
|
||||
try {
|
||||
if (isCentralKey(key)) {
|
||||
warnings.push(
|
||||
'federated-config: key "' + key + '" is still in the central config-schema (pending-migration); ' +
|
||||
'skipping federated resolution until the central schema entry is removed',
|
||||
);
|
||||
continue;
|
||||
}
|
||||
} catch {
|
||||
// isCentralKey threw — treat as unknown, skip defensively
|
||||
warnings.push('federated-config: isCentralKey("' + key + '") threw; skipping key');
|
||||
continue;
|
||||
}
|
||||
|
||||
// Validate slice shape — skip malformed entries
|
||||
if (!_isWellFormedSlice(slice)) {
|
||||
warnings.push(
|
||||
'federated-config: config slice for key "' + key + '" is malformed (missing or invalid type/default); skipping',
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
const sliceEntry = slice;
|
||||
|
||||
// FIX 1: Resolve value using NESTED dotted-path lookup through userConfig
|
||||
let resolvedValue: unknown = sliceEntry.default;
|
||||
const { found: userHasKey, value: userValue } = _getNestedValue(userConfig, key);
|
||||
if (userHasKey && userValue !== undefined) {
|
||||
// FIX 5b: For enum, validate against slice.values if present; otherwise check type
|
||||
if (_typeMatches(userValue, sliceEntry)) {
|
||||
resolvedValue = userValue;
|
||||
} else {
|
||||
const typeDesc = sliceEntry.type === 'enum' && Array.isArray(sliceEntry.values)
|
||||
? 'enum(' + sliceEntry.values.join('|') + ')'
|
||||
: sliceEntry.type;
|
||||
warnings.push(
|
||||
'federated-config: user-supplied value for "' + key + '" has wrong type or invalid enum value ' +
|
||||
'(expected ' + typeDesc + ', got ' + typeof userValue +
|
||||
(typeof userValue === 'string' ? ' "' + String(userValue) + '"' : '') +
|
||||
'); falling back to slice default',
|
||||
);
|
||||
// resolvedValue stays as slice default
|
||||
}
|
||||
}
|
||||
|
||||
// S2: inline literal guard before writing to values
|
||||
if (key !== '__proto__' && key !== 'constructor' && key !== 'prototype') {
|
||||
values[key] = resolvedValue;
|
||||
validKeys.push(key);
|
||||
}
|
||||
}
|
||||
|
||||
return { values, validKeys, warnings };
|
||||
}
|
||||
|
||||
export = { mergeFederatedConfig };
|
||||
533
src/loop-resolver.cts
Normal file
533
src/loop-resolver.cts
Normal file
@@ -0,0 +1,533 @@
|
||||
/**
|
||||
* Loop Resolver — ADR-857 phase 3c registry-consuming query
|
||||
*
|
||||
* Given a loop point (one of the 12 canonical points from loop-host-contract.cjs),
|
||||
* filters the materialized Capability Registry by config activation and returns
|
||||
* the active hooks as a JSON envelope with a rendered-markdown field.
|
||||
*
|
||||
* REGISTRY-ONLY: no workflow calls this yet (phase-6 cutover is out of scope).
|
||||
*
|
||||
* Command surface: gsd-tools loop render-hooks <point>
|
||||
*
|
||||
* Exports (three things):
|
||||
* resolveLoopHooks({ point, registry, config }) → { point, activeHooks }
|
||||
* renderLoopHooks(resolved) → markdown string
|
||||
* cmdLoopRenderHooks(cwd, point, raw, options) — I/O entry point
|
||||
*
|
||||
* Both pure functions (resolveLoopHooks, renderLoopHooks) take explicit
|
||||
* registry/config arguments so they are trivially testable without I/O.
|
||||
*
|
||||
* Dependencies (leaf modules only — no core.cjs circular risk):
|
||||
* - node:fs / node:path (raw config.json read for capability-key activation)
|
||||
* - ./config-loader.cjs (loadConfig)
|
||||
* - ./planning-workspace.cjs (planningDir — to locate config.json)
|
||||
* - ./core.cjs (output, error)
|
||||
* - loop-host-contract.cjs (CANONICAL_POINTS via LOOP_HOST_CONTRACT)
|
||||
* - capability-registry.cjs (byLoopPoint, consumed at call time)
|
||||
*/
|
||||
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import core = require('./core.cjs');
|
||||
const { output: coreOutput, error: coreError } = core;
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import configLoaderModule = require('./config-loader.cjs');
|
||||
const { loadConfig } = configLoaderModule;
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import planningWorkspaceMod = require('./planning-workspace.cjs');
|
||||
const { planningDir, planningRoot } = planningWorkspaceMod;
|
||||
|
||||
// ─── Canonical points (derived from LOOP_HOST_CONTRACT — authoritative 12) ───
|
||||
|
||||
// FIX 2: Derive the authoritative canonical set from LOOP_HOST_CONTRACT so it
|
||||
// cannot drift from the host contract. CANONICAL_POINTS_FALLBACK is kept as an
|
||||
// alias for backward compatibility in tests and exports.
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
const _loopHostContract = require('./loop-host-contract.cjs') as { LOOP_HOST_CONTRACT: Array<{ points: string[] }> };
|
||||
const CANONICAL_POINTS: ReadonlyArray<string> = (() => {
|
||||
try {
|
||||
const contract = _loopHostContract.LOOP_HOST_CONTRACT;
|
||||
if (Array.isArray(contract)) {
|
||||
const pts: string[] = [];
|
||||
for (const step of contract) {
|
||||
if (step && Array.isArray(step.points)) {
|
||||
for (const p of step.points) {
|
||||
if (typeof p === 'string') pts.push(p);
|
||||
}
|
||||
}
|
||||
}
|
||||
if (pts.length > 0) return pts;
|
||||
}
|
||||
} catch { /* fall through to hardcoded fallback */ }
|
||||
return [
|
||||
'discuss:pre',
|
||||
'discuss:post',
|
||||
'plan:pre',
|
||||
'plan:post',
|
||||
'execute:pre',
|
||||
'execute:wave:pre',
|
||||
'execute:wave:post',
|
||||
'execute:post',
|
||||
'verify:pre',
|
||||
'verify:post',
|
||||
'ship:pre',
|
||||
'ship:post',
|
||||
];
|
||||
})();
|
||||
|
||||
// Alias for backward compatibility (tests import this name)
|
||||
const CANONICAL_POINTS_FALLBACK: ReadonlyArray<string> = CANONICAL_POINTS;
|
||||
|
||||
// FIX 2: _getCanonicalPoints now returns the authoritative CANONICAL_POINTS set
|
||||
// derived from LOOP_HOST_CONTRACT — not the registry's byLoopPoint keys.
|
||||
// The registry's byLoopPoint is only used to READ hooks, not to define valid points.
|
||||
function _getCanonicalPoints(_registry: Record<string, unknown>): ReadonlyArray<string> {
|
||||
return CANONICAL_POINTS;
|
||||
}
|
||||
|
||||
// ─── Prototype-pollution guard (inline literal, CodeQL barrier) ───────────────
|
||||
|
||||
/**
|
||||
* Traverse a dotted config key through a nested config object.
|
||||
* E.g. "workflow.ui_phase" in { workflow: { ui_phase: true } } → { found: true, value: true }
|
||||
* Returns { found: false } if any segment is a forbidden key or not an own property.
|
||||
*/
|
||||
function _getNestedConfigValue(
|
||||
config: Record<string, unknown>,
|
||||
dotKey: string,
|
||||
): { found: boolean; value: unknown } {
|
||||
const segments = dotKey.split('.');
|
||||
let current: unknown = config;
|
||||
for (const seg of segments) {
|
||||
// Inline literal prototype-pollution guard (CodeQL barrier)
|
||||
if (seg === '__proto__' || seg === 'constructor' || seg === 'prototype') {
|
||||
return { found: false, value: undefined };
|
||||
}
|
||||
if (typeof current !== 'object' || current === null) {
|
||||
return { found: false, value: undefined };
|
||||
}
|
||||
const cur = current as Record<string, unknown>;
|
||||
if (!Object.prototype.hasOwnProperty.call(cur, seg)) {
|
||||
return { found: false, value: undefined };
|
||||
}
|
||||
current = cur[seg];
|
||||
}
|
||||
return { found: true, value: current };
|
||||
}
|
||||
|
||||
// ─── Single-key activation resolver (FIX 1) ───────────────────────────────────
|
||||
|
||||
/**
|
||||
* Warn-once set for raw config.json parse errors.
|
||||
* Avoids noisy per-call stderr from a single malformed file.
|
||||
*/
|
||||
const _warnedRawConfigPaths = new Set<string>();
|
||||
|
||||
/**
|
||||
* Read a raw config.json file and perform a guarded nested-lookup of a single
|
||||
* dotted key. Returns { found: false } if the file is missing (ENOENT) or if
|
||||
* the key is absent/forbidden. On a genuine JSON parse error: warns once to
|
||||
* stderr and returns { found: false } — never throws.
|
||||
*/
|
||||
function _readRawConfigKey(
|
||||
filePath: string,
|
||||
dotKey: string,
|
||||
): { found: boolean; value: unknown } {
|
||||
try {
|
||||
const raw = fs.readFileSync(filePath, 'utf8');
|
||||
let parsed: Record<string, unknown>;
|
||||
try {
|
||||
parsed = JSON.parse(raw) as Record<string, unknown>;
|
||||
} catch {
|
||||
if (!_warnedRawConfigPaths.has(filePath)) {
|
||||
_warnedRawConfigPaths.add(filePath);
|
||||
try {
|
||||
process.stderr.write(
|
||||
`gsd-tools: warning: failed to parse ${filePath} as JSON — skipping for activation resolution\n`,
|
||||
);
|
||||
} catch { /* stderr might be closed */ }
|
||||
}
|
||||
return { found: false, value: undefined };
|
||||
}
|
||||
return _getNestedConfigValue(parsed, dotKey);
|
||||
} catch {
|
||||
// ENOENT (missing file) is expected → skip silently. All other errors → also skip (defensive).
|
||||
return { found: false, value: undefined };
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* FIX 1: Resolve the effective value for a hook's `when` key using the
|
||||
* four-level precedence:
|
||||
*
|
||||
* 1. loadConfig result (`config` arg) — guarded nested-lookup of the dotted key.
|
||||
* This is the post-cutover federated path (covers keys that loadConfig now exposes).
|
||||
* 2. Raw workstream `.planning/.../config.json` — guarded single-key lookup.
|
||||
* Workstream wins over root (mirrors loadConfig inheritance).
|
||||
* 3. Raw root `.planning/config.json` — guarded single-key lookup.
|
||||
* 4. `registry.configSchema[when]?.default` — schema default.
|
||||
* A `default: true` hook is active out-of-the-box without any config.
|
||||
* 5. Absent → inactive (return false).
|
||||
*
|
||||
* Never constructs a merged object from raw JSON keys — only reads the single
|
||||
* leaf value at the guarded dotted path. Prototype-pollution sink is eliminated.
|
||||
*/
|
||||
function _resolveActivationValue(
|
||||
dotKey: string,
|
||||
config: Record<string, unknown>,
|
||||
cwd: string | undefined,
|
||||
registry: Record<string, unknown>,
|
||||
): boolean {
|
||||
// Level 1: loadConfig result
|
||||
const fromConfig = _getNestedConfigValue(config, dotKey);
|
||||
if (fromConfig.found) return Boolean(fromConfig.value);
|
||||
|
||||
// Level 2 + 3: raw config.json files (only when cwd is available)
|
||||
if (cwd) {
|
||||
// Level 2: workstream config (planningDir respects GSD_WORKSTREAM env)
|
||||
const wsConfigPath = path.join(planningDir(cwd), 'config.json');
|
||||
// Level 3: root config (planningRoot = cwd/.planning always)
|
||||
const rootConfigPath = path.join(planningRoot(cwd), 'config.json');
|
||||
|
||||
// Workstream wins over root (mirroring loadConfig root→workstream precedence:
|
||||
// workstream overlays root, so workstream value takes precedence).
|
||||
const fromWs = _readRawConfigKey(wsConfigPath, dotKey);
|
||||
if (fromWs.found) return Boolean(fromWs.value);
|
||||
|
||||
// Only read root if it differs from the workstream path (avoids double-read
|
||||
// when no workstream is active and both paths resolve to the same file).
|
||||
if (wsConfigPath !== rootConfigPath) {
|
||||
const fromRoot = _readRawConfigKey(rootConfigPath, dotKey);
|
||||
if (fromRoot.found) return Boolean(fromRoot.value);
|
||||
}
|
||||
}
|
||||
|
||||
// Level 4: registry configSchema default
|
||||
const schemaEntry = (registry['configSchema'] as Record<string, unknown> | undefined)?.[dotKey];
|
||||
if (schemaEntry && typeof schemaEntry === 'object' && schemaEntry !== null) {
|
||||
const def = (schemaEntry as Record<string, unknown>)['default'];
|
||||
if (def !== undefined) return Boolean(def);
|
||||
}
|
||||
|
||||
// Level 5: absent → inactive
|
||||
return false;
|
||||
}
|
||||
|
||||
// ─── Types ────────────────────────────────────────────────────────────────────
|
||||
|
||||
interface HookRef {
|
||||
skill?: string;
|
||||
[key: string]: unknown;
|
||||
}
|
||||
|
||||
interface RawHook {
|
||||
capId?: unknown;
|
||||
point?: unknown;
|
||||
ref?: unknown;
|
||||
into?: unknown;
|
||||
produces?: unknown;
|
||||
consumes?: unknown;
|
||||
when?: unknown;
|
||||
onError?: unknown;
|
||||
blocking?: unknown;
|
||||
check?: unknown;
|
||||
}
|
||||
|
||||
type HookKind = 'step' | 'contribution' | 'gate';
|
||||
|
||||
interface ActiveHook {
|
||||
capId: string;
|
||||
kind: HookKind;
|
||||
ref?: HookRef;
|
||||
into?: string;
|
||||
when?: string;
|
||||
produces?: string[];
|
||||
consumes?: string[];
|
||||
blocking?: boolean;
|
||||
check?: unknown;
|
||||
onError?: string;
|
||||
}
|
||||
|
||||
interface ResolveLoopHooksInput {
|
||||
point: string;
|
||||
registry: Record<string, unknown>;
|
||||
config: Record<string, unknown>;
|
||||
/** Optional cwd — enables raw config.json fallback reads (FIX 1 precedence level 2). */
|
||||
cwd?: string;
|
||||
}
|
||||
|
||||
interface ResolveLoopHooksResult {
|
||||
point: string;
|
||||
activeHooks: ActiveHook[];
|
||||
}
|
||||
|
||||
// ─── Pure resolver ─────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Pure resolver: given a point, registry, and config, returns the active hooks.
|
||||
*
|
||||
* Throws if `point` is not one of the 12 canonical points (caller converts to
|
||||
* core.error). Never throws for malformed registry/hook entries — skips and
|
||||
* continues.
|
||||
*
|
||||
* Ordering: steps first, then contributions, then gates. Within each array,
|
||||
* the materialized registry order is preserved.
|
||||
*
|
||||
* Activation: a hook with no `when` is always active. With `when` (dotted key),
|
||||
* resolved against `config`; active iff truthy. Inactive hooks are filtered out.
|
||||
*/
|
||||
function resolveLoopHooks(input: ResolveLoopHooksInput): ResolveLoopHooksResult {
|
||||
const { point, registry, config, cwd } = input;
|
||||
|
||||
// Validate point
|
||||
const canonicalPoints = _getCanonicalPoints(registry);
|
||||
if (!canonicalPoints.includes(point)) {
|
||||
throw new Error(
|
||||
`Invalid loop point: "${point}". Valid points: ${canonicalPoints.join(', ')}`,
|
||||
);
|
||||
}
|
||||
|
||||
// Guard: registry missing byLoopPoint
|
||||
const byLoopPoint = registry['byLoopPoint'];
|
||||
if (!byLoopPoint || typeof byLoopPoint !== 'object' || Array.isArray(byLoopPoint)) {
|
||||
return { point, activeHooks: [] };
|
||||
}
|
||||
const byLoopPointMap = byLoopPoint as Record<string, unknown>;
|
||||
|
||||
// Guard: point missing in registry
|
||||
const entry = byLoopPointMap[point];
|
||||
if (!entry || typeof entry !== 'object' || Array.isArray(entry)) {
|
||||
return { point, activeHooks: [] };
|
||||
}
|
||||
const entryMap = entry as Record<string, unknown>;
|
||||
|
||||
const activeHooks: ActiveHook[] = [];
|
||||
|
||||
// Helper: check activation using single-key precedence resolver (FIX 1 + FIX 3)
|
||||
function isActive(hook: RawHook): boolean {
|
||||
const when = hook['when'];
|
||||
// No `when` → unconditional hook, always active
|
||||
if (when === undefined || when === null) return true;
|
||||
// FIX 3: `when` present but not a non-empty string → malformed registry data → INACTIVE
|
||||
if (typeof when !== 'string' || when.length === 0) return false;
|
||||
return _resolveActivationValue(when, config, cwd, registry);
|
||||
}
|
||||
|
||||
// Helper: safe string array
|
||||
function toStringArray(v: unknown): string[] {
|
||||
if (!Array.isArray(v)) return [];
|
||||
return v.filter((x): x is string => typeof x === 'string');
|
||||
}
|
||||
|
||||
// Process steps
|
||||
const stepsRaw = entryMap['steps'];
|
||||
const steps: RawHook[] = Array.isArray(stepsRaw) ? (stepsRaw as RawHook[]) : [];
|
||||
for (const hook of steps) {
|
||||
if (!hook || typeof hook !== 'object') continue;
|
||||
if (!isActive(hook)) continue;
|
||||
const capId = typeof hook['capId'] === 'string' ? hook['capId'] : '';
|
||||
const ref = (typeof hook['ref'] === 'object' && hook['ref'] !== null)
|
||||
? (hook['ref'] as HookRef)
|
||||
: undefined;
|
||||
const when = typeof hook['when'] === 'string' ? hook['when'] : undefined;
|
||||
const produces = toStringArray(hook['produces']);
|
||||
const consumes = toStringArray(hook['consumes']);
|
||||
const onError = typeof hook['onError'] === 'string' ? hook['onError'] : undefined;
|
||||
const active: ActiveHook = { capId, kind: 'step' };
|
||||
if (ref !== undefined) active.ref = ref;
|
||||
if (when !== undefined) active.when = when;
|
||||
if (produces.length > 0) active.produces = produces;
|
||||
if (consumes.length > 0) active.consumes = consumes;
|
||||
if (onError !== undefined) active.onError = onError;
|
||||
activeHooks.push(active);
|
||||
}
|
||||
|
||||
// Process contributions
|
||||
const contributionsRaw = entryMap['contributions'];
|
||||
const contributions: RawHook[] = Array.isArray(contributionsRaw) ? (contributionsRaw as RawHook[]) : [];
|
||||
for (const hook of contributions) {
|
||||
if (!hook || typeof hook !== 'object') continue;
|
||||
if (!isActive(hook)) continue;
|
||||
const capId = typeof hook['capId'] === 'string' ? hook['capId'] : '';
|
||||
const into = typeof hook['into'] === 'string' ? hook['into'] : undefined;
|
||||
const when = typeof hook['when'] === 'string' ? hook['when'] : undefined;
|
||||
const produces = toStringArray(hook['produces']);
|
||||
const consumes = toStringArray(hook['consumes']);
|
||||
const onError = typeof hook['onError'] === 'string' ? hook['onError'] : undefined;
|
||||
const active: ActiveHook = { capId, kind: 'contribution' };
|
||||
if (into !== undefined) active.into = into;
|
||||
if (when !== undefined) active.when = when;
|
||||
if (produces.length > 0) active.produces = produces;
|
||||
if (consumes.length > 0) active.consumes = consumes;
|
||||
if (onError !== undefined) active.onError = onError;
|
||||
activeHooks.push(active);
|
||||
}
|
||||
|
||||
// Process gates
|
||||
const gatesRaw = entryMap['gates'];
|
||||
const gates: RawHook[] = Array.isArray(gatesRaw) ? (gatesRaw as RawHook[]) : [];
|
||||
for (const hook of gates) {
|
||||
if (!hook || typeof hook !== 'object') continue;
|
||||
if (!isActive(hook)) continue;
|
||||
const capId = typeof hook['capId'] === 'string' ? hook['capId'] : '';
|
||||
const when = typeof hook['when'] === 'string' ? hook['when'] : undefined;
|
||||
const check = hook['check'] !== undefined ? hook['check'] : undefined;
|
||||
const blocking = typeof hook['blocking'] === 'boolean' ? hook['blocking'] : undefined;
|
||||
const onError = typeof hook['onError'] === 'string' ? hook['onError'] : undefined;
|
||||
const active: ActiveHook = { capId, kind: 'gate' };
|
||||
if (when !== undefined) active.when = when;
|
||||
if (check !== undefined) active.check = check;
|
||||
if (blocking !== undefined) active.blocking = blocking;
|
||||
if (onError !== undefined) active.onError = onError;
|
||||
activeHooks.push(active);
|
||||
}
|
||||
|
||||
return { point, activeHooks };
|
||||
}
|
||||
|
||||
// ─── Pure renderer ─────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Pure renderer: given a resolved result, returns a deterministic markdown string.
|
||||
*
|
||||
* Empty active set → returns a "no active hooks" placeholder line.
|
||||
* Steps: heading with ordinal + skill ref + capId, produces/consumes lines.
|
||||
* Contributions: labeled block.
|
||||
* Gates: check name, blocking flag, onError.
|
||||
*/
|
||||
function renderLoopHooks(resolved: ResolveLoopHooksResult): string {
|
||||
const { point, activeHooks } = resolved;
|
||||
|
||||
if (activeHooks.length === 0) {
|
||||
return `_No active hooks at ${point}._`;
|
||||
}
|
||||
|
||||
const lines: string[] = [];
|
||||
let stepOrdinal = 0;
|
||||
|
||||
for (const hook of activeHooks) {
|
||||
if (hook.kind === 'step') {
|
||||
stepOrdinal += 1;
|
||||
const refStr = hook.ref?.skill
|
||||
? `skill:${hook.ref.skill}`
|
||||
: JSON.stringify(hook.ref ?? {});
|
||||
lines.push(`### Step ${stepOrdinal}: ${refStr} (${hook.capId})`);
|
||||
if (hook.produces && hook.produces.length > 0) {
|
||||
lines.push(`- produces: ${hook.produces.join(', ')}`);
|
||||
}
|
||||
if (hook.consumes && hook.consumes.length > 0) {
|
||||
lines.push(`- consumes: ${hook.consumes.join(', ')}`);
|
||||
}
|
||||
if (hook.when) {
|
||||
lines.push(`- when: \`${hook.when}\``);
|
||||
}
|
||||
if (hook.onError) {
|
||||
lines.push(`- onError: ${hook.onError}`);
|
||||
}
|
||||
lines.push('');
|
||||
} else if (hook.kind === 'contribution') {
|
||||
lines.push(`<contribution from="${hook.capId}" into="${hook.into ?? '(unset)'}"/>`);
|
||||
if (hook.produces && hook.produces.length > 0) {
|
||||
lines.push(`- produces: ${hook.produces.join(', ')}`);
|
||||
}
|
||||
if (hook.consumes && hook.consumes.length > 0) {
|
||||
lines.push(`- consumes: ${hook.consumes.join(', ')}`);
|
||||
}
|
||||
if (hook.when) {
|
||||
lines.push(`- when: \`${hook.when}\``);
|
||||
}
|
||||
lines.push('');
|
||||
} else if (hook.kind === 'gate') {
|
||||
let checkStr = '(none)';
|
||||
if (hook.check !== undefined && hook.check !== null) {
|
||||
checkStr = typeof hook.check === 'object'
|
||||
? JSON.stringify(hook.check)
|
||||
: typeof hook.check === 'string' || typeof hook.check === 'number' || typeof hook.check === 'boolean'
|
||||
? String(hook.check)
|
||||
: '(complex)';
|
||||
}
|
||||
lines.push(`**Gate** (${hook.capId}): check=${checkStr}, blocking=${String(hook.blocking ?? false)}, onError=${hook.onError ?? 'skip'}`);
|
||||
if (hook.when) {
|
||||
lines.push(`- when: \`${hook.when}\``);
|
||||
}
|
||||
lines.push('');
|
||||
}
|
||||
}
|
||||
|
||||
// Trim trailing blank line
|
||||
while (lines.length > 0 && lines[lines.length - 1] === '') {
|
||||
lines.pop();
|
||||
}
|
||||
|
||||
return lines.join('\n');
|
||||
}
|
||||
|
||||
// ─── I/O command handler ───────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Command entry point: load registry + config, resolve + render, emit envelope.
|
||||
*
|
||||
* Envelope: { point, activeHooks, rendered }
|
||||
* On invalid point, emits core.error instead of throwing.
|
||||
*
|
||||
* Config note: FIX 1 replaced _loadMergedConfig (whole-config deep-merge) with a
|
||||
* per-hook single-key activation resolver (_resolveActivationValue). The resolver
|
||||
* checks loadConfig result first, then raw config.json files directly (workstream
|
||||
* then root), then the registry's configSchema default. This eliminates the
|
||||
* merged-object-from-untrusted-keys security concern and correctly handles
|
||||
* pre-cutover keys like `workflow.ui_phase` that live in config.json but are not
|
||||
* yet exposed through loadConfig's whitelist.
|
||||
*/
|
||||
function cmdLoopRenderHooks(
|
||||
cwd: string,
|
||||
point: string,
|
||||
raw: boolean,
|
||||
_options: Record<string, unknown> = {},
|
||||
): void {
|
||||
if (!point) {
|
||||
coreError('loop render-hooks requires a <point> argument. Valid points: ' + CANONICAL_POINTS.join(', '));
|
||||
return;
|
||||
}
|
||||
|
||||
// Load registry at call time (generated file, not at module load time)
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
const registry = require('./capability-registry.cjs') as Record<string, unknown>;
|
||||
// FIX 1: Pass loadConfig result as `config` (level 1 of precedence);
|
||||
// raw config.json reads (levels 2+3) happen per-hook inside _resolveActivationValue
|
||||
// via the `cwd` argument passed to resolveLoopHooks.
|
||||
const config = loadConfig(cwd);
|
||||
|
||||
let resolved: ResolveLoopHooksResult;
|
||||
try {
|
||||
resolved = resolveLoopHooks({ point, registry, config, cwd });
|
||||
} catch (err: unknown) {
|
||||
const msg = (err instanceof Error) ? err.message : String(err);
|
||||
coreError(msg);
|
||||
return;
|
||||
}
|
||||
|
||||
const rendered = renderLoopHooks(resolved);
|
||||
const envelope = {
|
||||
point: resolved.point,
|
||||
activeHooks: resolved.activeHooks,
|
||||
rendered,
|
||||
};
|
||||
|
||||
coreOutput(envelope, raw);
|
||||
}
|
||||
|
||||
export = {
|
||||
resolveLoopHooks,
|
||||
renderLoopHooks,
|
||||
cmdLoopRenderHooks,
|
||||
// Exported for tests
|
||||
_getNestedConfigValue,
|
||||
_resolveActivationValue,
|
||||
_readRawConfigKey,
|
||||
CANONICAL_POINTS_FALLBACK,
|
||||
CANONICAL_POINTS,
|
||||
};
|
||||
@@ -29,6 +29,8 @@ const {
|
||||
computeRequiresClosure,
|
||||
topoSortSteps,
|
||||
normalizeLineEndings,
|
||||
validateConfigSliceEntry,
|
||||
VALID_CONFIG_SLICE_TYPES,
|
||||
SCHEMA_VERSION,
|
||||
} = require('../scripts/gen-capability-registry.cjs');
|
||||
|
||||
@@ -101,10 +103,33 @@ describe('UI pilot capability', () => {
|
||||
assert.strictEqual(uiGate.capId, 'ui');
|
||||
assert.strictEqual(uiGate.blocking, true);
|
||||
|
||||
// configKeys maps the 3 UI keys to 'ui'
|
||||
// configKeys maps the 3 UI keys to 'ui' (ownership map — preserved)
|
||||
assert.strictEqual(registry.configKeys['workflow.ui_phase'], 'ui');
|
||||
assert.strictEqual(registry.configKeys['workflow.ui_review'], 'ui');
|
||||
assert.strictEqual(registry.configKeys['workflow.ui_safety_gate'], 'ui');
|
||||
|
||||
// configSchema index — new in phase 3b
|
||||
assert.ok(registry.configSchema, 'registry.configSchema should exist');
|
||||
|
||||
// workflow.ui_phase
|
||||
assert.ok(registry.configSchema['workflow.ui_phase'], 'configSchema should have workflow.ui_phase');
|
||||
assert.strictEqual(registry.configSchema['workflow.ui_phase'].owner, 'ui');
|
||||
assert.strictEqual(registry.configSchema['workflow.ui_phase'].type, 'boolean');
|
||||
assert.strictEqual(registry.configSchema['workflow.ui_phase'].default, true);
|
||||
assert.strictEqual(typeof registry.configSchema['workflow.ui_phase'].description, 'string');
|
||||
assert.ok(registry.configSchema['workflow.ui_phase'].description.length > 0);
|
||||
|
||||
// workflow.ui_review
|
||||
assert.ok(registry.configSchema['workflow.ui_review'], 'configSchema should have workflow.ui_review');
|
||||
assert.strictEqual(registry.configSchema['workflow.ui_review'].owner, 'ui');
|
||||
assert.strictEqual(registry.configSchema['workflow.ui_review'].type, 'boolean');
|
||||
assert.strictEqual(registry.configSchema['workflow.ui_review'].default, true);
|
||||
|
||||
// workflow.ui_safety_gate
|
||||
assert.ok(registry.configSchema['workflow.ui_safety_gate'], 'configSchema should have workflow.ui_safety_gate');
|
||||
assert.strictEqual(registry.configSchema['workflow.ui_safety_gate'].owner, 'ui');
|
||||
assert.strictEqual(registry.configSchema['workflow.ui_safety_gate'].type, 'boolean');
|
||||
assert.strictEqual(registry.configSchema['workflow.ui_safety_gate'].default, true);
|
||||
});
|
||||
|
||||
test('requiresClosure("ui") returns empty set (no requires)', () => {
|
||||
@@ -1200,6 +1225,7 @@ describe('FIX 1: self-consume rejection in validateConsumesGlobal', () => {
|
||||
);
|
||||
});
|
||||
|
||||
// (this test follows the series above)
|
||||
test('a step produces:["SELF.md"] and consumes:["SELF.md"] and another capability produces SELF.md at the SAME point is accepted (different hook)', () => {
|
||||
const producerCap = {
|
||||
id: 'producer-cap', role: 'feature', title: 'Producer', description: 'Produces SELF.md',
|
||||
@@ -1240,3 +1266,241 @@ describe('FIX 1: self-consume rejection in validateConsumesGlobal', () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 14. configSchema emission (ADR-857 phase 3b) ────────────────────────────
|
||||
|
||||
describe('configSchema emission (ADR-857 phase 3b)', () => {
|
||||
test('buildRegistry emits configSchema with correct shape for UI pilot', () => {
|
||||
const capDir = makeTempCapDir({ ui: UI_CAP });
|
||||
const { capMap, errors } = loadAndValidate(new Set(), capDir);
|
||||
assert.deepEqual(errors, [], 'No errors expected');
|
||||
|
||||
const registry = buildRegistry(capMap);
|
||||
assert.ok(registry.configSchema, 'registry.configSchema must exist');
|
||||
|
||||
const uiPhase = registry.configSchema['workflow.ui_phase'];
|
||||
assert.ok(uiPhase, 'configSchema must have workflow.ui_phase');
|
||||
assert.strictEqual(uiPhase.owner, 'ui');
|
||||
assert.strictEqual(uiPhase.type, 'boolean');
|
||||
assert.strictEqual(uiPhase.default, true);
|
||||
assert.ok(typeof uiPhase.description === 'string' && uiPhase.description.length > 0);
|
||||
|
||||
const uiReview = registry.configSchema['workflow.ui_review'];
|
||||
assert.ok(uiReview, 'configSchema must have workflow.ui_review');
|
||||
assert.strictEqual(uiReview.owner, 'ui');
|
||||
assert.strictEqual(uiReview.type, 'boolean');
|
||||
|
||||
const uiSafetyGate = registry.configSchema['workflow.ui_safety_gate'];
|
||||
assert.ok(uiSafetyGate, 'configSchema must have workflow.ui_safety_gate');
|
||||
assert.strictEqual(uiSafetyGate.type, 'boolean');
|
||||
});
|
||||
|
||||
test('serializeRegistry emits a configSchema block in the generated .cjs', () => {
|
||||
const capDir = makeTempCapDir({ ui: UI_CAP });
|
||||
const { capMap } = loadAndValidate(new Set(), capDir);
|
||||
const registry = buildRegistry(capMap);
|
||||
const content = serializeRegistry(registry, capMap);
|
||||
|
||||
assert.ok(content.includes('const configSchema'), 'Generated file must contain "const configSchema"');
|
||||
assert.ok(content.includes('"workflow.ui_phase"'), 'Generated file must contain "workflow.ui_phase"');
|
||||
assert.ok(content.includes('"owner"'), 'Generated file must contain "owner" field');
|
||||
assert.ok(content.includes('"type"'), 'Generated file must contain "type" field');
|
||||
assert.ok(content.includes('"default"'), 'Generated file must contain "default" field');
|
||||
assert.ok(content.includes('"description"'), 'Generated file must contain "description" field');
|
||||
assert.ok(content.includes('configSchema,'), 'Generated module.exports must include configSchema');
|
||||
});
|
||||
|
||||
test('committed capability-registry.cjs has configSchema with correct shape', () => {
|
||||
const registry = require('../gsd-core/bin/lib/capability-registry.cjs');
|
||||
assert.ok(registry.configSchema, 'capability-registry.cjs must export configSchema');
|
||||
|
||||
const uiPhase = registry.configSchema['workflow.ui_phase'];
|
||||
assert.ok(uiPhase, 'committed registry configSchema must have workflow.ui_phase');
|
||||
assert.strictEqual(uiPhase.owner, 'ui', 'owner must be "ui"');
|
||||
assert.strictEqual(uiPhase.type, 'boolean', 'type must be "boolean"');
|
||||
assert.strictEqual(uiPhase.default, true, 'default must be true');
|
||||
assert.ok(typeof uiPhase.description === 'string' && uiPhase.description.length > 0);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 15. validateConfigSliceEntry adversarial tests ───────────────────────────
|
||||
|
||||
describe('validateConfigSliceEntry adversarial cases (ADR-857 phase 3b)', () => {
|
||||
const CAP_ID = 'test-cap';
|
||||
const KEY = 'test.key';
|
||||
|
||||
test('VALID_CONFIG_SLICE_TYPES exports expected types', () => {
|
||||
const types = [...VALID_CONFIG_SLICE_TYPES];
|
||||
assert.ok(types.includes('boolean'), 'Must include boolean');
|
||||
assert.ok(types.includes('string'), 'Must include string');
|
||||
assert.ok(types.includes('number'), 'Must include number');
|
||||
assert.ok(types.includes('enum'), 'Must include enum');
|
||||
assert.strictEqual(types.length, 4, 'Must have exactly 4 types');
|
||||
});
|
||||
|
||||
test('valid boolean slice passes validation', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'boolean', default: true, description: 'ok' });
|
||||
assert.deepEqual(errors, [], 'Valid boolean slice should produce no errors, got: ' + JSON.stringify(errors));
|
||||
});
|
||||
|
||||
test('valid string slice passes validation', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'string', default: 'x', description: 'ok' });
|
||||
assert.deepEqual(errors, []);
|
||||
});
|
||||
|
||||
test('valid number slice passes validation', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'number', default: 5, description: 'ok' });
|
||||
assert.deepEqual(errors, []);
|
||||
});
|
||||
|
||||
test('REJECTED: enum slice without values list → error (FIX 5a: values required)', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'enum', default: 'x', description: 'ok' });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for enum without values list, got: ' + JSON.stringify(errors));
|
||||
assert.ok(
|
||||
errors.some((e) => e.includes('values') || e.includes('enum')),
|
||||
'Error should mention values or enum, got: ' + JSON.stringify(errors),
|
||||
);
|
||||
});
|
||||
|
||||
test('valid enum slice (with values list, default in values) passes', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, {
|
||||
type: 'enum', default: 'b', values: ['a', 'b', 'c'], description: 'ok',
|
||||
});
|
||||
assert.deepEqual(errors, []);
|
||||
});
|
||||
|
||||
test('REJECTED: bad type ("xml") → error mentioning type', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'xml', default: '<x/>', description: 'ok' });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for bad type');
|
||||
assert.ok(errors.some((e) => e.includes('type')), 'Error should mention type, got: ' + JSON.stringify(errors));
|
||||
});
|
||||
|
||||
test('REJECTED: missing type → error', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { default: true, description: 'ok' });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for missing type');
|
||||
assert.ok(errors.some((e) => e.includes('type')));
|
||||
});
|
||||
|
||||
test('REJECTED: missing default → error mentioning default', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'boolean', description: 'ok' });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for missing default');
|
||||
assert.ok(errors.some((e) => e.includes('default')), 'Error should mention default, got: ' + JSON.stringify(errors));
|
||||
});
|
||||
|
||||
test('REJECTED: boolean type with string default → error', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'boolean', default: 'true', description: 'ok' });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for boolean type with string default');
|
||||
assert.ok(errors.some((e) => e.includes('boolean') || e.includes('default')));
|
||||
});
|
||||
|
||||
test('REJECTED: string type with boolean default → error', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'string', default: false, description: 'ok' });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for string type with boolean default');
|
||||
});
|
||||
|
||||
test('REJECTED: number type with string default → error', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'number', default: 'five', description: 'ok' });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for number type with string default');
|
||||
});
|
||||
|
||||
test('REJECTED: enum type with values list, default not in values → error', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, {
|
||||
type: 'enum', default: 'z', values: ['a', 'b', 'c'], description: 'ok',
|
||||
});
|
||||
assert.ok(errors.length > 0, 'Expected rejection for enum default not in values');
|
||||
assert.ok(errors.some((e) => e.includes('enum') || e.includes('values') || e.includes('z')));
|
||||
});
|
||||
|
||||
test('REJECTED: enum type with non-string default → error', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'enum', default: 42, description: 'ok' });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for enum with non-string default');
|
||||
});
|
||||
|
||||
test('REJECTED: empty description string → error', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'boolean', default: true, description: '' });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for empty description');
|
||||
assert.ok(errors.some((e) => e.includes('description')));
|
||||
});
|
||||
|
||||
test('REJECTED: non-string description (number) → error', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'boolean', default: true, description: 42 });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for non-string description');
|
||||
assert.ok(errors.some((e) => e.includes('description')));
|
||||
});
|
||||
|
||||
test('REJECTED: missing description → error', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'boolean', default: true });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for missing description');
|
||||
assert.ok(errors.some((e) => e.includes('description')));
|
||||
});
|
||||
|
||||
test('REJECTED: null slice → error', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, null);
|
||||
assert.ok(errors.length > 0, 'Expected rejection for null slice');
|
||||
});
|
||||
|
||||
test('REJECTED: array slice → error', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, []);
|
||||
assert.ok(errors.length > 0, 'Expected rejection for array slice');
|
||||
});
|
||||
|
||||
// FIX 5a: enum-without-values and default-not-in-values
|
||||
test('REJECTED: enum with empty values array → error (FIX 5a)', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'enum', default: 'x', values: [], description: 'ok' });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for enum with empty values, got: ' + JSON.stringify(errors));
|
||||
assert.ok(errors.some((e) => e.includes('values') || e.includes('enum')));
|
||||
});
|
||||
|
||||
test('REJECTED: enum with non-string values array entries → error (FIX 5a)', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'enum', default: 'x', values: ['a', 42], description: 'ok' });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for enum with non-string values, got: ' + JSON.stringify(errors));
|
||||
assert.ok(errors.some((e) => e.includes('values') || e.includes('string')));
|
||||
});
|
||||
|
||||
test('REJECTED: enum default not in values → error (FIX 5a)', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'enum', default: 'z', values: ['a', 'b'], description: 'ok' });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for enum default not in values, got: ' + JSON.stringify(errors));
|
||||
assert.ok(errors.some((e) => e.includes('z') || e.includes('values') || e.includes('default')));
|
||||
});
|
||||
|
||||
// FIX 6a: NaN and non-finite number defaults
|
||||
test('REJECTED: NaN number default → error (FIX 6a)', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'number', default: NaN, description: 'ok' });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for NaN default, got: ' + JSON.stringify(errors));
|
||||
assert.ok(errors.some((e) => e.includes('finite') || e.includes('NaN') || e.includes('number')));
|
||||
});
|
||||
|
||||
test('REJECTED: Infinity number default → error (FIX 6a)', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'number', default: Infinity, description: 'ok' });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for Infinity default, got: ' + JSON.stringify(errors));
|
||||
assert.ok(errors.some((e) => e.includes('finite') || e.includes('number')));
|
||||
});
|
||||
|
||||
test('REJECTED: -Infinity number default → error (FIX 6a)', () => {
|
||||
const errors = validateConfigSliceEntry(CAP_ID, KEY, { type: 'number', default: -Infinity, description: 'ok' });
|
||||
assert.ok(errors.length > 0, 'Expected rejection for -Infinity default, got: ' + JSON.stringify(errors));
|
||||
});
|
||||
|
||||
test('buildRegistry throws on malformed config slice in capability', () => {
|
||||
// A capability with a config slice that has a missing default — buildRegistry must throw
|
||||
const cap = {
|
||||
...UI_CAP,
|
||||
config: {
|
||||
...UI_CAP.config,
|
||||
'workflow.bad_key': { type: 'boolean', description: 'missing default' },
|
||||
},
|
||||
};
|
||||
const capMap = new Map([['ui', cap]]);
|
||||
assert.throws(
|
||||
() => buildRegistry(capMap),
|
||||
(err) => {
|
||||
assert.ok(err instanceof Error, 'Must throw an Error');
|
||||
assert.ok(
|
||||
err.message.includes('configSchema') || err.message.includes('default') || err.message.includes('validation'),
|
||||
'Error must mention configSchema validation, got: ' + err.message,
|
||||
);
|
||||
return true;
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
451
tests/federated-config-loadconfig.test.cjs
Normal file
451
tests/federated-config-loadconfig.test.cjs
Normal file
@@ -0,0 +1,451 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* federated-config-loadconfig.test.cjs — Tests for the federated config overlay
|
||||
* wired into loadConfig (ADR-857 phase 3b).
|
||||
*
|
||||
* Tests:
|
||||
* 1. EQUIVALENCE/no-op: with the real registry, loadConfig output has NO unexpected
|
||||
* extra keys (the UI keys are central so the overlay is empty).
|
||||
* 2. FIXTURE federated key: inject a synthetic configSchema with a key NOT in
|
||||
* central schema → loadConfig surfaces it with its default.
|
||||
* 3. FIXTURE federated key with user override: user config sets the federated key
|
||||
* to a valid value → that value is used.
|
||||
* 4. MALFORMED registry: configSchema with bad slices → loadConfig returns a valid
|
||||
* config without throwing.
|
||||
*/
|
||||
|
||||
const { describe, test, beforeEach, afterEach } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const os = require('node:os');
|
||||
|
||||
const { cleanup } = require('./helpers.cjs');
|
||||
|
||||
// ─── Module under test ────────────────────────────────────────────────────────
|
||||
|
||||
const configLoader = require('../gsd-core/bin/lib/config-loader.cjs');
|
||||
const {
|
||||
loadConfig,
|
||||
_setFederatedRegistryForTests,
|
||||
_resetFederatedRegistryForTests,
|
||||
} = configLoader;
|
||||
|
||||
// ─── Helpers ──────────────────────────────────────────────────────────────────
|
||||
|
||||
function makeTempProject() {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-fed-cfg-test-'));
|
||||
fs.mkdirSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true });
|
||||
return tmpDir;
|
||||
}
|
||||
|
||||
function writeConfig(tmpDir, obj) {
|
||||
fs.writeFileSync(
|
||||
path.join(tmpDir, '.planning', 'config.json'),
|
||||
JSON.stringify(obj, null, 2),
|
||||
'utf-8',
|
||||
);
|
||||
}
|
||||
|
||||
// Keep track of temp dirs for cleanup
|
||||
let tmpDirs = [];
|
||||
|
||||
beforeEach(() => {
|
||||
tmpDirs = [];
|
||||
_resetFederatedRegistryForTests();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
_resetFederatedRegistryForTests();
|
||||
for (const d of tmpDirs) {
|
||||
try { cleanup(d); } catch { /* ignore */ }
|
||||
}
|
||||
});
|
||||
|
||||
function mkTemp() {
|
||||
const d = makeTempProject();
|
||||
tmpDirs.push(d);
|
||||
return d;
|
||||
}
|
||||
|
||||
// ─── 1. Equivalence / no-op with real registry ───────────────────────────────
|
||||
|
||||
describe('EQUIVALENCE: real registry is a no-op overlay', () => {
|
||||
test('loadConfig with an empty config.json returns base config without extra federated keys', () => {
|
||||
const tmpDir = mkTemp();
|
||||
// Write an empty config to trigger the try-branch (federated overlay path)
|
||||
writeConfig(tmpDir, {});
|
||||
const result = loadConfig(tmpDir);
|
||||
|
||||
// The result must be an object
|
||||
assert.ok(typeof result === 'object' && result !== null, 'loadConfig must return an object');
|
||||
|
||||
// Known result keys that loadConfig always provides (from the main try-branch)
|
||||
const knownKeys = [
|
||||
'model_profile', 'commit_docs', 'search_gitignored', 'branching_strategy',
|
||||
'research', 'plan_checker', 'verifier', 'parallelization', 'brave_search',
|
||||
'firecrawl', 'exa_search', 'text_mode', 'auto_advance',
|
||||
'mode', 'sub_repos', 'resolve_model_ids', 'context_window', 'phase_naming',
|
||||
'project_code', 'subagent_timeout', 'model_overrides', 'models', 'granularity',
|
||||
'granularities', 'planning', 'dynamic_routing', 'runtime', 'model_profile_overrides',
|
||||
'model_policy', 'effort', 'fast_mode', 'agent_skills', 'manager',
|
||||
];
|
||||
|
||||
for (const key of knownKeys) {
|
||||
assert.ok(
|
||||
Object.prototype.hasOwnProperty.call(result, key),
|
||||
'Expected result to have key: ' + key,
|
||||
);
|
||||
}
|
||||
|
||||
// UI-capability keys must NOT appear as new top-level keys (they're still central
|
||||
// and the overlay is empty — so these keys should not be added)
|
||||
// Note: 'workflow' IS an existing top-level key concept via VALID_CONFIG_KEYS,
|
||||
// but the nested keys like 'ui_phase' must not be present.
|
||||
const workflowSection = result['workflow'];
|
||||
if (workflowSection && typeof workflowSection === 'object') {
|
||||
// workflow section may already exist from user config but should not have ui_phase
|
||||
// in the default no-config case
|
||||
assert.ok(
|
||||
!Object.prototype.hasOwnProperty.call(workflowSection, 'ui_phase'),
|
||||
'workflow.ui_phase should not be injected by the federated overlay (key is still central)',
|
||||
);
|
||||
assert.ok(
|
||||
!Object.prototype.hasOwnProperty.call(workflowSection, 'ui_review'),
|
||||
'workflow.ui_review should not be injected by the federated overlay (key is still central)',
|
||||
);
|
||||
assert.ok(
|
||||
!Object.prototype.hasOwnProperty.call(workflowSection, 'ui_safety_gate'),
|
||||
'workflow.ui_safety_gate should not be injected by the federated overlay (key is still central)',
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test('loadConfig with a real config.json returns expected values + no unexpected keys from overlay', () => {
|
||||
const tmpDir = mkTemp();
|
||||
writeConfig(tmpDir, { model_profile: 'balanced', research: true });
|
||||
const result = loadConfig(tmpDir);
|
||||
|
||||
assert.strictEqual(result['model_profile'], 'balanced', 'model_profile from config');
|
||||
assert.strictEqual(result['research'], true, 'research from config');
|
||||
|
||||
// The overlay must not have added any unexpected keys from the registry
|
||||
// (all UI keys are central → skipped → no additions)
|
||||
// Verify a spot-check: 'ui_phase' should not exist anywhere
|
||||
assert.strictEqual(result['ui_phase'], undefined, 'ui_phase should not appear as top-level key');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 2. Fixture federated key — value = default ──────────────────────────────
|
||||
|
||||
describe('FIXTURE federated key: key not in central schema', () => {
|
||||
test('injected configSchema with non-central key → appears in loadConfig result with default', () => {
|
||||
const tmpDir = mkTemp();
|
||||
// Write an empty config.json so loadConfig enters the try-branch (federated overlay path)
|
||||
writeConfig(tmpDir, {});
|
||||
|
||||
// Inject a synthetic registry with a key not in the central schema
|
||||
_setFederatedRegistryForTests({
|
||||
configSchema: {
|
||||
'mytool.enabled': {
|
||||
owner: 'mytool',
|
||||
type: 'boolean',
|
||||
default: true,
|
||||
description: 'Enable mytool.',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = loadConfig(tmpDir);
|
||||
|
||||
// mytool is not in the central schema, so the overlay should surface it
|
||||
// The key 'mytool.enabled' is dotted → result should have result.mytool.enabled = true
|
||||
const myToolSection = result['mytool'];
|
||||
assert.ok(typeof myToolSection === 'object' && myToolSection !== null,
|
||||
'mytool section must be created for dotted federated key');
|
||||
assert.strictEqual(
|
||||
(myToolSection)['enabled'],
|
||||
true,
|
||||
'mytool.enabled must default to true from slice',
|
||||
);
|
||||
});
|
||||
|
||||
test('injected top-level (non-dotted) federated key → appears in result', () => {
|
||||
const tmpDir = mkTemp();
|
||||
// Write an empty config.json to enter the try-branch
|
||||
writeConfig(tmpDir, {});
|
||||
|
||||
_setFederatedRegistryForTests({
|
||||
configSchema: {
|
||||
'mytool_flag': {
|
||||
owner: 'mytool',
|
||||
type: 'boolean',
|
||||
default: false,
|
||||
description: 'Top-level mytool flag.',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = loadConfig(tmpDir);
|
||||
// Top-level key: result['mytool_flag'] = false (the default)
|
||||
// BUT: only added if NOT already present in _baseConfig
|
||||
// 'mytool_flag' is not in the central schema, so it should be added
|
||||
assert.strictEqual(result['mytool_flag'], false, 'mytool_flag should be set to default false');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 3. Fixture federated key — user override ────────────────────────────────
|
||||
|
||||
describe('FIXTURE federated key: user config sets the key', () => {
|
||||
test('user sets a federated key to a valid value → loadConfig uses user value', () => {
|
||||
const tmpDir = mkTemp();
|
||||
|
||||
// Write a user config with a synthetic federated key
|
||||
// The user config uses flat notation (mytool_flag: false)
|
||||
writeConfig(tmpDir, { mytool_flag: true });
|
||||
|
||||
_setFederatedRegistryForTests({
|
||||
configSchema: {
|
||||
'mytool_flag': {
|
||||
owner: 'mytool',
|
||||
type: 'boolean',
|
||||
default: false,
|
||||
description: 'Top-level mytool flag.',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = loadConfig(tmpDir);
|
||||
// The user set mytool_flag=true, which matches the type (boolean), so user value wins
|
||||
assert.strictEqual(result['mytool_flag'], true, 'User-supplied true should override default false');
|
||||
});
|
||||
|
||||
test('user sets a federated key to wrong type → loadConfig falls back to default', () => {
|
||||
const tmpDir = mkTemp();
|
||||
// Write a user config with the wrong type for the federated key
|
||||
writeConfig(tmpDir, { mytool_flag: 'not-a-bool' });
|
||||
|
||||
_setFederatedRegistryForTests({
|
||||
configSchema: {
|
||||
'mytool_flag': {
|
||||
owner: 'mytool',
|
||||
type: 'boolean',
|
||||
default: false,
|
||||
description: 'Top-level mytool flag.',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = loadConfig(tmpDir);
|
||||
// Wrong type → fallback to default (false)
|
||||
assert.strictEqual(result['mytool_flag'], false, 'Should fall back to default on type mismatch');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── FIX 1: Nested dotted-path user-override in loadConfig ───────────────────
|
||||
|
||||
describe('FIX 1: nested user config drives federated overlay in loadConfig', () => {
|
||||
test('user config { mytool: { enabled: false } } (NESTED) → loadConfig surfaces false', () => {
|
||||
const tmpDir = mkTemp();
|
||||
// Write config.json with the nested structure users actually write
|
||||
writeConfig(tmpDir, { mytool: { enabled: false } });
|
||||
|
||||
_setFederatedRegistryForTests({
|
||||
configSchema: {
|
||||
'mytool.enabled': {
|
||||
owner: 'mytool',
|
||||
type: 'boolean',
|
||||
default: true,
|
||||
description: 'Enable mytool.',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = loadConfig(tmpDir);
|
||||
const myToolSection = result['mytool'];
|
||||
assert.ok(typeof myToolSection === 'object' && myToolSection !== null,
|
||||
'mytool section must be in result');
|
||||
assert.strictEqual(
|
||||
(myToolSection)['enabled'],
|
||||
false,
|
||||
'Nested user override of false should override the default of true',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── FIX 2: Overlay applied on no-config path ────────────────────────────────
|
||||
|
||||
describe('FIX 2: overlay applied on the no-config path', () => {
|
||||
test('project with NO config.json → federated default is surfaced (non-central key)', () => {
|
||||
// Create a project dir WITHOUT a .planning/config.json
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-fed-noconfig-'));
|
||||
tmpDirs.push(tmpDir);
|
||||
fs.mkdirSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true });
|
||||
// Intentionally do NOT write a config.json
|
||||
|
||||
_setFederatedRegistryForTests({
|
||||
configSchema: {
|
||||
'mytool.enabled': {
|
||||
owner: 'mytool',
|
||||
type: 'boolean',
|
||||
default: false,
|
||||
description: 'Enable mytool (default false).',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = loadConfig(tmpDir);
|
||||
// The overlay must be applied on the no-config path: mytool.enabled should be false (the default)
|
||||
const myToolSection = result['mytool'];
|
||||
assert.ok(
|
||||
typeof myToolSection === 'object' && myToolSection !== null,
|
||||
'mytool section must be created by overlay even on no-config path, got: ' + JSON.stringify(result['mytool']),
|
||||
);
|
||||
assert.strictEqual(
|
||||
(myToolSection)['enabled'],
|
||||
false,
|
||||
'mytool.enabled must default to false on no-config path',
|
||||
);
|
||||
});
|
||||
|
||||
test('no-config path with REAL registry (all keys central) → output is byte-identical to defaults (no-op)', () => {
|
||||
// No config.json — use real registry which has all keys central
|
||||
_resetFederatedRegistryForTests();
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-fed-noconfig-real-'));
|
||||
tmpDirs.push(tmpDir);
|
||||
fs.mkdirSync(path.join(tmpDir, '.planning', 'phases'), { recursive: true });
|
||||
// No config.json
|
||||
|
||||
const result = loadConfig(tmpDir);
|
||||
assert.ok(typeof result === 'object' && result !== null, 'result must be an object');
|
||||
|
||||
// With real registry (all keys central → overlay is empty → no-op), the result
|
||||
// should be the defaults object WITHOUT any extra injected keys.
|
||||
// Spot-check: ui_phase / ui_review / ui_safety_gate must NOT be injected
|
||||
const workflowSection = result['workflow'];
|
||||
if (workflowSection && typeof workflowSection === 'object') {
|
||||
assert.ok(
|
||||
!Object.prototype.hasOwnProperty.call(workflowSection, 'ui_phase'),
|
||||
'workflow.ui_phase must not be injected on no-config path (no-op)',
|
||||
);
|
||||
}
|
||||
// model_profile must be present (it comes from defaults)
|
||||
assert.ok(Object.prototype.hasOwnProperty.call(result, 'model_profile'), 'model_profile must be present');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── FIX 3: Federated key in config.json → no unknown-key warning ─────────────
|
||||
|
||||
describe('FIX 3: federated key present in config.json → no unknown-key warning', () => {
|
||||
test('synthetic federated key in config.json → no "unknown config key" warning on stderr', () => {
|
||||
const tmpDir = mkTemp();
|
||||
// Write a config.json that contains a key matching our synthetic federated key's top-level segment
|
||||
writeConfig(tmpDir, { mytool: { enabled: true } });
|
||||
|
||||
_setFederatedRegistryForTests({
|
||||
configSchema: {
|
||||
'mytool.enabled': {
|
||||
owner: 'mytool',
|
||||
type: 'boolean',
|
||||
default: false,
|
||||
description: 'Enable mytool.',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// Capture stderr to check for unknown-key warning
|
||||
const stderrChunks = [];
|
||||
const origWrite = process.stderr.write.bind(process.stderr);
|
||||
process.stderr.write = (chunk, ...args) => {
|
||||
stderrChunks.push(typeof chunk === 'string' ? chunk : String(chunk));
|
||||
return origWrite(chunk, ...args);
|
||||
};
|
||||
|
||||
try {
|
||||
const result = loadConfig(tmpDir);
|
||||
// mytool.enabled is in the federated registry → KNOWN_TOP_LEVEL should include 'mytool'
|
||||
// → no "unknown config key(s)" warning for 'mytool'
|
||||
const stderrOutput = stderrChunks.join('');
|
||||
assert.ok(
|
||||
!stderrOutput.includes('unknown config key') || !stderrOutput.includes('mytool'),
|
||||
'Should NOT warn about mytool as an unknown key when it is a registered federated key. stderr: ' + stderrOutput,
|
||||
);
|
||||
// The value should be set from user config
|
||||
const myToolSection = result['mytool'];
|
||||
assert.ok(
|
||||
typeof myToolSection === 'object' && myToolSection !== null,
|
||||
'mytool section should be in result',
|
||||
);
|
||||
} finally {
|
||||
process.stderr.write = origWrite;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 4. Malformed registry — loadConfig still works ──────────────────────────
|
||||
|
||||
describe('MALFORMED registry: loadConfig does not throw', () => {
|
||||
test('configSchema with all malformed slices → loadConfig returns valid config, no throw', () => {
|
||||
const tmpDir = mkTemp();
|
||||
|
||||
_setFederatedRegistryForTests({
|
||||
configSchema: {
|
||||
'bad.key1': null,
|
||||
'bad.key2': 'just-a-string',
|
||||
'bad.key3': { type: 'xml', default: '<x/>' }, // invalid type
|
||||
'bad.key4': { type: 'boolean', description: 'ok' }, // missing default
|
||||
'bad.key5': {}, // missing both
|
||||
},
|
||||
});
|
||||
|
||||
let result;
|
||||
assert.doesNotThrow(() => {
|
||||
result = loadConfig(tmpDir);
|
||||
}, 'loadConfig must not throw even with all-malformed configSchema');
|
||||
|
||||
assert.ok(typeof result === 'object' && result !== null, 'result must be an object');
|
||||
// None of the bad keys should appear in the result
|
||||
assert.strictEqual(result['bad.key1'], undefined);
|
||||
assert.strictEqual(result['bad.key2'], undefined);
|
||||
const badSection = result['bad'];
|
||||
if (badSection && typeof badSection === 'object') {
|
||||
assert.strictEqual((badSection)['key1'], undefined, 'bad.key1 must not be set');
|
||||
}
|
||||
});
|
||||
|
||||
test('configSchema is a string (completely unexpected) → loadConfig still works', () => {
|
||||
const tmpDir = mkTemp();
|
||||
|
||||
_setFederatedRegistryForTests({
|
||||
configSchema: 'not-an-object',
|
||||
});
|
||||
|
||||
let result;
|
||||
assert.doesNotThrow(() => {
|
||||
result = loadConfig(tmpDir);
|
||||
}, 'loadConfig must not throw with non-object configSchema');
|
||||
|
||||
assert.ok(typeof result === 'object' && result !== null, 'result must be an object');
|
||||
});
|
||||
|
||||
test('registry throws during configSchema access → loadConfig still returns base config', () => {
|
||||
const tmpDir = mkTemp();
|
||||
|
||||
// Create a registry proxy that throws when configSchema is accessed
|
||||
const throwingRegistry = {
|
||||
get configSchema() { throw new Error('registry exploded'); },
|
||||
};
|
||||
|
||||
_setFederatedRegistryForTests(throwingRegistry);
|
||||
|
||||
let result;
|
||||
assert.doesNotThrow(() => {
|
||||
result = loadConfig(tmpDir);
|
||||
}, 'loadConfig must not throw even if registry access throws');
|
||||
|
||||
assert.ok(typeof result === 'object' && result !== null, 'result must still be an object');
|
||||
// The base config keys must be present
|
||||
assert.ok(Object.prototype.hasOwnProperty.call(result, 'model_profile'), 'model_profile must be present');
|
||||
});
|
||||
});
|
||||
696
tests/federated-config.test.cjs
Normal file
696
tests/federated-config.test.cjs
Normal file
@@ -0,0 +1,696 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* federated-config.test.cjs — Behavioral tests for the federated-config module.
|
||||
*
|
||||
* ADR-857 phase 3b. Tests cover:
|
||||
* - empty configSchema → empty result
|
||||
* - key still in central schema → skipped + pending-migration warning + NOT in values
|
||||
* - malformed slice (each variant) → skipped + warning, no throw
|
||||
* - valid federated key → value = default
|
||||
* - valid federated key with correct-type user override → user value used
|
||||
* - valid federated key with wrong-type user override → falls back to default + warning
|
||||
* - __proto__/constructor/prototype keys → ignored, no Object.prototype pollution
|
||||
*/
|
||||
|
||||
const { describe, test } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
|
||||
const { mergeFederatedConfig } = require('../gsd-core/bin/lib/federated-config.cjs');
|
||||
|
||||
// ─── Helper fixtures ──────────────────────────────────────────────────────────
|
||||
|
||||
/** A minimal well-formed boolean config slice entry. */
|
||||
const BOOLEAN_SLICE = {
|
||||
owner: 'test-cap',
|
||||
type: 'boolean',
|
||||
default: true,
|
||||
description: 'A test boolean key.',
|
||||
};
|
||||
|
||||
/** A minimal well-formed string config slice entry. */
|
||||
const STRING_SLICE = {
|
||||
owner: 'test-cap',
|
||||
type: 'string',
|
||||
default: 'hello',
|
||||
description: 'A test string key.',
|
||||
};
|
||||
|
||||
/** A minimal well-formed number config slice entry. */
|
||||
const NUMBER_SLICE = {
|
||||
owner: 'test-cap',
|
||||
type: 'number',
|
||||
default: 42,
|
||||
description: 'A test number key.',
|
||||
};
|
||||
|
||||
/** A minimal well-formed enum config slice entry (no values list). */
|
||||
const ENUM_SLICE_NO_VALUES = {
|
||||
owner: 'test-cap',
|
||||
type: 'enum',
|
||||
default: 'medium',
|
||||
description: 'A test enum key without values list.',
|
||||
};
|
||||
|
||||
/** A minimal well-formed enum config slice entry (with values list). */
|
||||
const ENUM_SLICE_WITH_VALUES = {
|
||||
owner: 'test-cap',
|
||||
type: 'enum',
|
||||
default: 'low',
|
||||
values: ['low', 'medium', 'high'],
|
||||
description: 'A test enum key with values list.',
|
||||
};
|
||||
|
||||
/** A never-central isCentralKey that always returns false. */
|
||||
const neverCentral = (_key) => false;
|
||||
|
||||
/** An always-central isCentralKey. */
|
||||
const alwaysCentral = (_key) => true;
|
||||
|
||||
// ─── 1. Empty configSchema ────────────────────────────────────────────────────
|
||||
|
||||
describe('empty configSchema', () => {
|
||||
test('empty object → empty result', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: {},
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
assert.strictEqual(Object.keys(result.values).length, 0, 'values should be empty');
|
||||
assert.deepEqual(result.validKeys, [], 'validKeys should be empty');
|
||||
assert.deepEqual(result.warnings, [], 'warnings should be empty');
|
||||
});
|
||||
|
||||
test('null configSchema → empty result (defensive)', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: null,
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
// FIX 6b: values uses null-prototype object; check it's empty
|
||||
assert.strictEqual(Object.keys(result.values).length, 0, 'values should be empty for null configSchema');
|
||||
assert.deepEqual(result.validKeys, [], 'validKeys should be empty');
|
||||
assert.deepEqual(result.warnings, [], 'warnings should be empty');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 2. Central-key skipping ──────────────────────────────────────────────────
|
||||
|
||||
describe('central-key skipping', () => {
|
||||
test('key in central schema → skipped + pending-migration warning + NOT in values', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'workflow.ui_phase': BOOLEAN_SLICE },
|
||||
isCentralKey: alwaysCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'workflow.ui_phase'),
|
||||
'central key must NOT appear in values');
|
||||
assert.deepEqual(result.validKeys, [], 'validKeys must be empty for central keys');
|
||||
assert.ok(result.warnings.length >= 1, 'Must produce at least one warning');
|
||||
assert.ok(
|
||||
result.warnings.some((w) => w.includes('pending-migration') || w.includes('central config-schema')),
|
||||
'Warning must mention pending-migration or central config-schema, got: ' + JSON.stringify(result.warnings),
|
||||
);
|
||||
});
|
||||
|
||||
test('key NOT in central schema → appears in values', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.enabled': BOOLEAN_SLICE },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
assert.ok(Object.prototype.hasOwnProperty.call(result.values, 'mytool.enabled'),
|
||||
'Non-central key must appear in values');
|
||||
assert.strictEqual(result.validKeys.length, 1);
|
||||
});
|
||||
|
||||
test('mixed central + non-central: central skipped, non-central included', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: {
|
||||
'central.key': BOOLEAN_SLICE,
|
||||
'mytool.enabled': BOOLEAN_SLICE,
|
||||
},
|
||||
isCentralKey: (key) => key === 'central.key',
|
||||
userConfig: {},
|
||||
});
|
||||
assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'central.key'),
|
||||
'central.key must not be in values');
|
||||
assert.ok(Object.prototype.hasOwnProperty.call(result.values, 'mytool.enabled'),
|
||||
'mytool.enabled must be in values');
|
||||
assert.strictEqual(result.validKeys.length, 1);
|
||||
assert.ok(result.warnings.some((w) => w.includes('pending-migration') || w.includes('central')));
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 3. Malformed slice handling ──────────────────────────────────────────────
|
||||
|
||||
describe('malformed slice handling — no throw, warning emitted', () => {
|
||||
test('slice missing type → skipped + warning, no throw', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'tool.key': { owner: 'x', default: true, description: 'x' } },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'tool.key'), 'malformed key must not be in values');
|
||||
assert.ok(result.warnings.length >= 1, 'Must warn about malformed slice');
|
||||
assert.ok(result.warnings.some((w) => w.includes('malformed') || w.includes('tool.key')),
|
||||
'Warning must mention the key, got: ' + JSON.stringify(result.warnings));
|
||||
});
|
||||
|
||||
test('slice with invalid type ("xml") → skipped + warning, no throw', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'tool.key': { owner: 'x', type: 'xml', default: '<x/>', description: 'xml key' } },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'tool.key'));
|
||||
assert.ok(result.warnings.length >= 1);
|
||||
});
|
||||
|
||||
test('slice missing default → skipped + warning, no throw', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'tool.key': { owner: 'x', type: 'boolean', description: 'x' } },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'tool.key'));
|
||||
assert.ok(result.warnings.length >= 1);
|
||||
});
|
||||
|
||||
test('slice is null → skipped + warning, no throw', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'tool.key': null },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'tool.key'));
|
||||
assert.ok(result.warnings.length >= 1);
|
||||
});
|
||||
|
||||
test('slice is a string scalar → skipped + warning, no throw', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'tool.key': 'just-a-string' },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'tool.key'));
|
||||
assert.ok(result.warnings.length >= 1);
|
||||
});
|
||||
|
||||
test('slice is a number → skipped + warning, no throw', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'tool.key': 42 },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'tool.key'));
|
||||
assert.ok(result.warnings.length >= 1);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 4. Valid federated key — default resolution ──────────────────────────────
|
||||
|
||||
describe('valid federated key — default resolution', () => {
|
||||
test('boolean key absent from userConfig → value = default (true)', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.enabled': BOOLEAN_SLICE },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
assert.strictEqual(result.values['mytool.enabled'], true, 'Should use slice default (true)');
|
||||
assert.ok(result.validKeys.includes('mytool.enabled'));
|
||||
assert.deepEqual(result.warnings, []);
|
||||
});
|
||||
|
||||
test('string key absent from userConfig → value = default ("hello")', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.name': STRING_SLICE },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
assert.strictEqual(result.values['mytool.name'], 'hello');
|
||||
assert.ok(result.validKeys.includes('mytool.name'));
|
||||
});
|
||||
|
||||
test('number key absent from userConfig → value = default (42)', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.timeout': NUMBER_SLICE },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
assert.strictEqual(result.values['mytool.timeout'], 42);
|
||||
});
|
||||
|
||||
test('enum key absent from userConfig → value = default ("medium")', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.level': ENUM_SLICE_NO_VALUES },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
assert.strictEqual(result.values['mytool.level'], 'medium');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 5. Valid federated key — correct-type user override ─────────────────────
|
||||
|
||||
describe('valid federated key — user override with correct type', () => {
|
||||
test('boolean key with boolean user override (nested) → user value used', () => {
|
||||
// FIX 1: users write nested objects, not flat dotted keys
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.enabled': BOOLEAN_SLICE },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: { mytool: { enabled: false } },
|
||||
});
|
||||
assert.strictEqual(result.values['mytool.enabled'], false, 'Should use user-supplied false');
|
||||
assert.deepEqual(result.warnings, []);
|
||||
});
|
||||
|
||||
test('string key with string user override (nested) → user value used', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.name': STRING_SLICE },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: { mytool: { name: 'custom' } },
|
||||
});
|
||||
assert.strictEqual(result.values['mytool.name'], 'custom');
|
||||
assert.deepEqual(result.warnings, []);
|
||||
});
|
||||
|
||||
test('number key with number user override (nested) → user value used', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.timeout': NUMBER_SLICE },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: { mytool: { timeout: 99 } },
|
||||
});
|
||||
assert.strictEqual(result.values['mytool.timeout'], 99);
|
||||
assert.deepEqual(result.warnings, []);
|
||||
});
|
||||
|
||||
test('enum key with in-values string user override (nested) → user value used', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.level': ENUM_SLICE_WITH_VALUES },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: { mytool: { level: 'high' } },
|
||||
});
|
||||
assert.strictEqual(result.values['mytool.level'], 'high');
|
||||
assert.deepEqual(result.warnings, []);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 6. Valid federated key — wrong-type user override ───────────────────────
|
||||
|
||||
describe('valid federated key — wrong-type user override', () => {
|
||||
test('boolean key with string user override (nested) → falls back to default + warning', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.enabled': BOOLEAN_SLICE },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: { mytool: { enabled: 'not-a-bool' } },
|
||||
});
|
||||
// Key IS in validKeys (degraded resolution — default used)
|
||||
assert.ok(Object.prototype.hasOwnProperty.call(result.values, 'mytool.enabled'),
|
||||
'Key should still appear in values (degraded)');
|
||||
assert.strictEqual(result.values['mytool.enabled'], true, 'Should fall back to default (true)');
|
||||
assert.ok(result.warnings.length >= 1, 'Should warn about type mismatch');
|
||||
assert.ok(
|
||||
result.warnings.some((w) => w.includes('wrong type') || w.includes('type')),
|
||||
'Warning should mention type mismatch, got: ' + JSON.stringify(result.warnings),
|
||||
);
|
||||
});
|
||||
|
||||
test('string key with boolean user override (nested) → falls back to default + warning', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.name': STRING_SLICE },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: { mytool: { name: true } },
|
||||
});
|
||||
assert.strictEqual(result.values['mytool.name'], 'hello', 'Should fall back to default');
|
||||
assert.ok(result.warnings.length >= 1);
|
||||
});
|
||||
|
||||
test('number key with string user override (nested) → falls back to default + warning', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.timeout': NUMBER_SLICE },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: { mytool: { timeout: 'fast' } },
|
||||
});
|
||||
assert.strictEqual(result.values['mytool.timeout'], 42, 'Should fall back to default');
|
||||
assert.ok(result.warnings.length >= 1);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 7. Prototype pollution guard ────────────────────────────────────────────
|
||||
|
||||
describe('prototype pollution guard', () => {
|
||||
test('__proto__ key in configSchema → ignored, no Object.prototype pollution', () => {
|
||||
// We can't pass __proto__ as an own-enumerable property via object literal,
|
||||
// so we use Object.create + defineProperty to simulate what a capability registry
|
||||
// might hand us if prototype pollution had been attempted upstream.
|
||||
const poisonedSchema = Object.create(null);
|
||||
Object.defineProperty(poisonedSchema, '__proto__', {
|
||||
value: { polluted: true },
|
||||
enumerable: true,
|
||||
configurable: true,
|
||||
writable: true,
|
||||
});
|
||||
// Note: 'constructor' and 'prototype' CAN be passed via plain object literals
|
||||
const schemaWithReservedKeys = {
|
||||
'constructor': BOOLEAN_SLICE,
|
||||
'prototype': STRING_SLICE,
|
||||
};
|
||||
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: schemaWithReservedKeys,
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
|
||||
// Reserved keys must not appear in values
|
||||
assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'constructor'), 'constructor must not be in values');
|
||||
assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'prototype'), 'prototype must not be in values');
|
||||
assert.ok(!result.validKeys.includes('constructor'), 'constructor must not be in validKeys');
|
||||
assert.ok(!result.validKeys.includes('prototype'), 'prototype must not be in validKeys');
|
||||
|
||||
// Object.prototype must not be polluted
|
||||
assert.strictEqual(({}).polluted, undefined, 'Object.prototype must not be polluted');
|
||||
assert.strictEqual(({}).constructor, Object, 'Object.prototype.constructor must be Object (not overwritten)');
|
||||
});
|
||||
|
||||
test('buildRegistry with poisoned keys does not pollute Object.prototype', () => {
|
||||
// Even with isCentralKey always returning false, reserved keys are guarded
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'legitimate.key': BOOLEAN_SLICE },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
assert.strictEqual(({}).polluted, undefined, 'Object.prototype.polluted must be undefined after merge');
|
||||
assert.ok(Object.prototype.hasOwnProperty.call(result.values, 'legitimate.key'), 'legitimate key must be in values');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── FIX 1: Nested dotted-path user-override lookup ──────────────────────────
|
||||
|
||||
describe('FIX 1: nested dotted-path user-override lookup', () => {
|
||||
test('user sets mytool.enabled via NESTED object → user value used', () => {
|
||||
// Nested config: { mytool: { enabled: false } } — NOT flat {"mytool.enabled": false}
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: {
|
||||
'mytool.enabled': {
|
||||
owner: 'mytool',
|
||||
type: 'boolean',
|
||||
default: true,
|
||||
description: 'Enable mytool.',
|
||||
},
|
||||
},
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: { mytool: { enabled: false } }, // NESTED
|
||||
});
|
||||
assert.strictEqual(result.values['mytool.enabled'], false, 'Nested user override should be used (false overrides true)');
|
||||
assert.deepEqual(result.warnings, [], 'No warnings for valid nested override');
|
||||
assert.ok(result.validKeys.includes('mytool.enabled'));
|
||||
});
|
||||
|
||||
test('flat {"mytool.enabled": false} does NOT match nested path lookup', () => {
|
||||
// Flat key string lookup is intentionally NOT supported per FIX 1 spec
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: {
|
||||
'mytool.enabled': {
|
||||
owner: 'mytool',
|
||||
type: 'boolean',
|
||||
default: true,
|
||||
description: 'Enable mytool.',
|
||||
},
|
||||
},
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: { 'mytool.enabled': false }, // FLAT — not found via nested traversal
|
||||
});
|
||||
// Flat key is not found by nested traversal, so default is used
|
||||
assert.strictEqual(result.values['mytool.enabled'], true, 'Flat key not found by nested traversal → default used');
|
||||
});
|
||||
|
||||
test('user sets nested 3-segment key correctly', () => {
|
||||
// Key: "a.b.c", user config: { a: { b: { c: 'override' } } }
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: {
|
||||
'a.b.c': {
|
||||
owner: 'test',
|
||||
type: 'string',
|
||||
default: 'default-val',
|
||||
description: 'Three-segment key.',
|
||||
},
|
||||
},
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: { a: { b: { c: 'override' } } },
|
||||
});
|
||||
assert.strictEqual(result.values['a.b.c'], 'override');
|
||||
assert.deepEqual(result.warnings, []);
|
||||
});
|
||||
|
||||
test('partial nested path (a.b exists but a.b.c missing) → uses default', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: {
|
||||
'a.b.c': {
|
||||
owner: 'test',
|
||||
type: 'string',
|
||||
default: 'default-val',
|
||||
description: 'Three-segment key.',
|
||||
},
|
||||
},
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: { a: { b: {} } }, // c is missing
|
||||
});
|
||||
assert.strictEqual(result.values['a.b.c'], 'default-val', 'Missing leaf should use default');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── FIX 4: null/undefined/non-object input guards ───────────────────────────
|
||||
|
||||
describe('FIX 4: null/undefined/non-object input guards', () => {
|
||||
test('null input → no throw, empty result', () => {
|
||||
assert.doesNotThrow(() => {
|
||||
const result = mergeFederatedConfig(null);
|
||||
assert.ok(result.validKeys.length === 0);
|
||||
});
|
||||
});
|
||||
|
||||
test('undefined input → no throw, empty result', () => {
|
||||
assert.doesNotThrow(() => {
|
||||
const result = mergeFederatedConfig(undefined);
|
||||
assert.ok(result.validKeys.length === 0);
|
||||
});
|
||||
});
|
||||
|
||||
test('non-object input (string) → no throw, empty result', () => {
|
||||
assert.doesNotThrow(() => {
|
||||
const result = mergeFederatedConfig('not-an-object');
|
||||
assert.ok(result.validKeys.length === 0);
|
||||
});
|
||||
});
|
||||
|
||||
test('null userConfig → treated as {} (no overrides), no throw', () => {
|
||||
assert.doesNotThrow(() => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.enabled': BOOLEAN_SLICE },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: null,
|
||||
});
|
||||
// Should use the default since userConfig is null
|
||||
assert.strictEqual(result.values['mytool.enabled'], true, 'Should use default when userConfig is null');
|
||||
assert.deepEqual(result.warnings, []);
|
||||
});
|
||||
});
|
||||
|
||||
test('undefined userConfig → treated as {} (no overrides), no throw', () => {
|
||||
assert.doesNotThrow(() => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.enabled': BOOLEAN_SLICE },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: undefined,
|
||||
});
|
||||
assert.strictEqual(result.values['mytool.enabled'], true, 'Should use default when userConfig is undefined');
|
||||
});
|
||||
});
|
||||
|
||||
test('non-object userConfig → treated as {} (no overrides), no throw', () => {
|
||||
assert.doesNotThrow(() => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.enabled': BOOLEAN_SLICE },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: 42,
|
||||
});
|
||||
assert.strictEqual(result.values['mytool.enabled'], true, 'Should use default when userConfig is non-object');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// ─── FIX 5b: enum user override validation against values list ────────────────
|
||||
|
||||
describe('FIX 5b: enum out-of-values user override → falls back to default', () => {
|
||||
test('enum user override IN values list → accepted', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.level': ENUM_SLICE_WITH_VALUES },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: { mytool: { level: 'high' } },
|
||||
});
|
||||
assert.strictEqual(result.values['mytool.level'], 'high', 'In-values override should be accepted');
|
||||
assert.deepEqual(result.warnings, []);
|
||||
});
|
||||
|
||||
test('enum user override OUT OF values list → falls back to default + warning', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.level': ENUM_SLICE_WITH_VALUES },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: { mytool: { level: 'extreme' } }, // 'extreme' not in ['low', 'medium', 'high']
|
||||
});
|
||||
assert.strictEqual(result.values['mytool.level'], 'low', 'Out-of-values override should fall back to default');
|
||||
assert.ok(result.warnings.length >= 1, 'Should warn about out-of-values override');
|
||||
assert.ok(
|
||||
result.warnings.some((w) => w.includes('type') || w.includes('enum') || w.includes('invalid')),
|
||||
'Warning should mention type/enum issue, got: ' + JSON.stringify(result.warnings),
|
||||
);
|
||||
});
|
||||
|
||||
test('enum user override is non-string → falls back to default + warning', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.level': ENUM_SLICE_WITH_VALUES },
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: { mytool: { level: 42 } }, // number, not string
|
||||
});
|
||||
assert.strictEqual(result.values['mytool.level'], 'low');
|
||||
assert.ok(result.warnings.length >= 1);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── FIX 6b: null-proto consistency in early-return paths ────────────────────
|
||||
|
||||
describe('FIX 6b: null-proto consistency on early-return paths', () => {
|
||||
test('null configSchema → values uses Object.create(null) (no __proto__ chain)', () => {
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: null,
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
// Object.create(null) has no __proto__ — prototype is null
|
||||
assert.strictEqual(Object.getPrototypeOf(result.values), null, 'values must use null prototype on null configSchema path');
|
||||
});
|
||||
|
||||
test('null input → values uses Object.create(null)', () => {
|
||||
const result = mergeFederatedConfig(null);
|
||||
assert.strictEqual(Object.getPrototypeOf(result.values), null, 'values must use null prototype on null input path');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── FIX 6c: N-level nested write + prototype pollution via dotted keys ──────
|
||||
|
||||
describe('FIX 6c: N-level nested write and prototype-pollution via dotted keys', () => {
|
||||
test('3-segment federated key is correctly nested in loadConfig overlay', () => {
|
||||
// This test verifies _getNestedValue works correctly for 3-segment keys.
|
||||
// The values object should store the key as "a.b.c" → value mapping.
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: {
|
||||
'tool.section.flag': {
|
||||
owner: 'test',
|
||||
type: 'boolean',
|
||||
default: true,
|
||||
description: 'Three-segment boolean key.',
|
||||
},
|
||||
},
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: { tool: { section: { flag: false } } },
|
||||
});
|
||||
assert.strictEqual(result.values['tool.section.flag'], false, '3-segment override should be picked up');
|
||||
assert.ok(result.validKeys.includes('tool.section.flag'));
|
||||
assert.deepEqual(result.warnings, []);
|
||||
});
|
||||
|
||||
test('__proto__ segment in dotted key does NOT pollute Object.prototype', () => {
|
||||
const schema = Object.create(null);
|
||||
// Create a key with __proto__ in the path via defineProperty
|
||||
Object.defineProperty(schema, '__proto__.x', {
|
||||
value: { owner: 'test', type: 'boolean', default: true, description: 'bad key' },
|
||||
enumerable: true, configurable: true, writable: true,
|
||||
});
|
||||
assert.doesNotThrow(() => {
|
||||
mergeFederatedConfig({
|
||||
configSchema: schema,
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
});
|
||||
// Object.prototype must not be polluted
|
||||
assert.strictEqual(({}).x, undefined, 'Object.prototype.x must not be polluted via __proto__ key');
|
||||
});
|
||||
|
||||
test('a.__proto__.b segment in dotted key does NOT pollute Object.prototype', () => {
|
||||
// The key "a.__proto__.b" should be skipped at the __proto__ segment
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: {
|
||||
// We can't define 'a.__proto__.b' as an OWN property normally; skip test via defensive path
|
||||
'a.constructor.b': {
|
||||
owner: 'test',
|
||||
type: 'boolean',
|
||||
default: true,
|
||||
description: 'constructor key',
|
||||
},
|
||||
},
|
||||
isCentralKey: neverCentral,
|
||||
userConfig: {},
|
||||
});
|
||||
// 'a.constructor.b' contains 'constructor' segment — must be skipped
|
||||
assert.ok(!result.validKeys.includes('a.constructor.b'), 'Key with constructor segment must be skipped');
|
||||
// Object.prototype.constructor must still be Object
|
||||
assert.strictEqual(({}).constructor, Object, 'Object.prototype.constructor must not be modified');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 8. Real registry — all UI keys are central (no-op guarantee) ────────────
|
||||
|
||||
describe('real registry: all UI keys are central → no-op channel', () => {
|
||||
test('with real capability-registry, all configSchema keys are skipped (pending-migration)', () => {
|
||||
const capRegistry = require('../gsd-core/bin/lib/capability-registry.cjs');
|
||||
const configSchemaFromRegistry = capRegistry.configSchema;
|
||||
|
||||
// Import the real isValidConfigKey
|
||||
const configSchemaModule = require('../gsd-core/bin/lib/config-schema.cjs');
|
||||
const { isValidConfigKey } = configSchemaModule;
|
||||
|
||||
if (!configSchemaFromRegistry || Object.keys(configSchemaFromRegistry).length === 0) {
|
||||
// Registry has no configSchema keys — no-op by definition
|
||||
return;
|
||||
}
|
||||
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: configSchemaFromRegistry,
|
||||
isCentralKey: isValidConfigKey,
|
||||
userConfig: {},
|
||||
});
|
||||
|
||||
// Every key should be skipped (pending-migration) because UI keys are still in central schema
|
||||
assert.strictEqual(Object.keys(result.values).length, 0, 'values must be empty — all keys are central (pending-migration)');
|
||||
assert.deepEqual(result.validKeys, [], 'validKeys must be empty');
|
||||
assert.ok(result.warnings.length > 0, 'Should have pending-migration warnings');
|
||||
|
||||
// Confirm each UI key specifically
|
||||
const uiKeys = ['workflow.ui_phase', 'workflow.ui_review', 'workflow.ui_safety_gate'];
|
||||
for (const key of uiKeys) {
|
||||
assert.ok(
|
||||
result.warnings.some((w) => w.includes(key)),
|
||||
'Should have a warning for ' + key + ', got: ' + JSON.stringify(result.warnings),
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 9. isCentralKey throwing defensively ────────────────────────────────────
|
||||
|
||||
describe('isCentralKey defensive behavior', () => {
|
||||
test('isCentralKey that throws → key is skipped with warning, no throw from mergeFederatedConfig', () => {
|
||||
const throwingCentralKey = () => { throw new Error('internal error'); };
|
||||
const result = mergeFederatedConfig({
|
||||
configSchema: { 'mytool.key': BOOLEAN_SLICE },
|
||||
isCentralKey: throwingCentralKey,
|
||||
userConfig: {},
|
||||
});
|
||||
// Key skipped due to isCentralKey throwing
|
||||
assert.ok(!Object.prototype.hasOwnProperty.call(result.values, 'mytool.key'), 'Key must be skipped when isCentralKey throws');
|
||||
assert.ok(result.warnings.length >= 1, 'Must produce a warning when isCentralKey throws');
|
||||
});
|
||||
});
|
||||
777
tests/loop-render-hooks.test.cjs
Normal file
777
tests/loop-render-hooks.test.cjs
Normal file
@@ -0,0 +1,777 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* loop-render-hooks.test.cjs — behavioral tests for loop-resolver.cjs.
|
||||
*
|
||||
* ADR-857 phase 3c.
|
||||
* Uses node:test + node:assert/strict.
|
||||
* Pure-function tests (resolveLoopHooks, renderLoopHooks) pass registry+config
|
||||
* directly — no I/O. End-to-end tests use cmdLoopRenderHooks + a temp project.
|
||||
*/
|
||||
|
||||
const { describe, test, before, after } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const os = require('node:os');
|
||||
const path = require('node:path');
|
||||
|
||||
const { cleanup } = require('./helpers.cjs');
|
||||
|
||||
const {
|
||||
resolveLoopHooks,
|
||||
renderLoopHooks,
|
||||
_getNestedConfigValue,
|
||||
_resolveActivationValue,
|
||||
_readRawConfigKey,
|
||||
CANONICAL_POINTS_FALLBACK,
|
||||
CANONICAL_POINTS,
|
||||
} = require('../gsd-core/bin/lib/loop-resolver.cjs');
|
||||
|
||||
// The real registry for integration tests
|
||||
const realRegistry = require('../gsd-core/bin/lib/capability-registry.cjs');
|
||||
|
||||
// ─── Synthetic registry fixtures ─────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Build a minimal synthetic registry with a single step hook at a given point.
|
||||
* Optionally include a configSchema for testing default-based activation.
|
||||
*/
|
||||
function makeRegistry({ point = 'plan:pre', steps = [], contributions = [], gates = {}, configSchema = {} } = {}) {
|
||||
const byLoopPoint = {};
|
||||
for (const p of CANONICAL_POINTS_FALLBACK) {
|
||||
byLoopPoint[p] = { steps: [], contributions: [], gates: [] };
|
||||
}
|
||||
if (steps.length) byLoopPoint[point].steps = steps;
|
||||
if (contributions.length) byLoopPoint[point].contributions = contributions;
|
||||
if (gates[point]) byLoopPoint[point].gates = gates[point];
|
||||
return { byLoopPoint, configSchema };
|
||||
}
|
||||
|
||||
// ─── Temp project helpers ─────────────────────────────────────────────────────
|
||||
|
||||
let tmpProjectDir;
|
||||
// A project with NO .planning/config.json — relies on schema defaults
|
||||
let tmpEmptyProjectDir;
|
||||
// A project where ui_phase is explicitly false in root config
|
||||
let tmpFalseConfigProjectDir;
|
||||
|
||||
before(() => {
|
||||
tmpProjectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'loop-resolver-test-'));
|
||||
const planningDir = path.join(tmpProjectDir, '.planning');
|
||||
fs.mkdirSync(planningDir, { recursive: true });
|
||||
// Write minimal config.json with all UI flags enabled
|
||||
fs.writeFileSync(
|
||||
path.join(planningDir, 'config.json'),
|
||||
JSON.stringify({ workflow: { ui_phase: true, ui_review: true, ui_safety_gate: true } }),
|
||||
'utf8',
|
||||
);
|
||||
|
||||
// Empty project — no config.json: schema defaults drive activation
|
||||
tmpEmptyProjectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'loop-resolver-empty-'));
|
||||
fs.mkdirSync(path.join(tmpEmptyProjectDir, '.planning'), { recursive: true });
|
||||
|
||||
// False config project — ui_phase explicitly false in root config
|
||||
tmpFalseConfigProjectDir = fs.mkdtempSync(path.join(os.tmpdir(), 'loop-resolver-false-'));
|
||||
const falseConfigPlanningDir = path.join(tmpFalseConfigProjectDir, '.planning');
|
||||
fs.mkdirSync(falseConfigPlanningDir, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
path.join(falseConfigPlanningDir, 'config.json'),
|
||||
JSON.stringify({ workflow: { ui_phase: false, ui_review: false, ui_safety_gate: false } }),
|
||||
'utf8',
|
||||
);
|
||||
});
|
||||
|
||||
after(() => {
|
||||
if (tmpProjectDir) cleanup(tmpProjectDir);
|
||||
if (tmpEmptyProjectDir) cleanup(tmpEmptyProjectDir);
|
||||
if (tmpFalseConfigProjectDir) cleanup(tmpFalseConfigProjectDir);
|
||||
});
|
||||
|
||||
// ─── 1. Canonical-point validation ───────────────────────────────────────────
|
||||
|
||||
describe('canonical point validation', () => {
|
||||
test('all 12 canonical points are accepted by resolveLoopHooks with empty registry', () => {
|
||||
const emptyRegistry = makeRegistry();
|
||||
const config = {};
|
||||
for (const p of CANONICAL_POINTS_FALLBACK) {
|
||||
const result = resolveLoopHooks({ point: p, registry: emptyRegistry, config });
|
||||
assert.strictEqual(result.point, p);
|
||||
assert.deepEqual(result.activeHooks, []);
|
||||
}
|
||||
});
|
||||
|
||||
test('12 canonical points total', () => {
|
||||
assert.strictEqual(CANONICAL_POINTS_FALLBACK.length, 12);
|
||||
});
|
||||
|
||||
test('invalid point throws with a clear message', () => {
|
||||
const emptyRegistry = makeRegistry();
|
||||
assert.throws(
|
||||
() => resolveLoopHooks({ point: 'plan:mid', registry: emptyRegistry, config: {} }),
|
||||
(err) => {
|
||||
assert.ok(err instanceof Error);
|
||||
assert.match(err.message, /Invalid loop point/);
|
||||
assert.match(err.message, /plan:mid/);
|
||||
return true;
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
test('empty string point throws', () => {
|
||||
const emptyRegistry = makeRegistry();
|
||||
assert.throws(
|
||||
() => resolveLoopHooks({ point: '', registry: emptyRegistry, config: {} }),
|
||||
/Invalid loop point/,
|
||||
);
|
||||
});
|
||||
|
||||
test('close typo throws', () => {
|
||||
const emptyRegistry = makeRegistry();
|
||||
assert.throws(
|
||||
() => resolveLoopHooks({ point: 'plan:pre ', registry: emptyRegistry, config: {} }),
|
||||
/Invalid loop point/,
|
||||
);
|
||||
});
|
||||
|
||||
// FIX 2: non-canonical point rejected even if the registry has it as a byLoopPoint key
|
||||
test('non-canonical point in registry byLoopPoint is still rejected', () => {
|
||||
// Craft a registry that has a synthetic non-canonical key in byLoopPoint
|
||||
const registry = {
|
||||
byLoopPoint: {
|
||||
// All canonical points (needed so the registry is well-formed)
|
||||
...Object.fromEntries(CANONICAL_POINTS_FALLBACK.map(p => [p, { steps: [], contributions: [], gates: [] }])),
|
||||
// A non-canonical key that a malformed registry might inject
|
||||
'inject:arbitrary': { steps: [{ capId: 'evil', ref: { skill: 'bad' } }], contributions: [], gates: [] },
|
||||
},
|
||||
};
|
||||
assert.throws(
|
||||
() => resolveLoopHooks({ point: 'inject:arbitrary', registry, config: {} }),
|
||||
/Invalid loop point/,
|
||||
);
|
||||
});
|
||||
|
||||
// FIX 2: all 12 canonical points are listed in the error message
|
||||
test('invalid point error lists the canonical 12', () => {
|
||||
const emptyRegistry = makeRegistry();
|
||||
assert.throws(
|
||||
() => resolveLoopHooks({ point: 'not:real', registry: emptyRegistry, config: {} }),
|
||||
(err) => {
|
||||
assert.ok(err instanceof Error);
|
||||
for (const p of CANONICAL_POINTS_FALLBACK) {
|
||||
assert.ok(err.message.includes(p), `Expected "${p}" in error message: ${err.message}`);
|
||||
}
|
||||
return true;
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
// CANONICAL_POINTS is derived from LOOP_HOST_CONTRACT, not from registry keys
|
||||
test('CANONICAL_POINTS and CANONICAL_POINTS_FALLBACK are the same 12 points', () => {
|
||||
assert.deepEqual(CANONICAL_POINTS, CANONICAL_POINTS_FALLBACK);
|
||||
assert.strictEqual(CANONICAL_POINTS.length, 12);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 2. Activation tests ─────────────────────────────────────────────────────
|
||||
|
||||
describe('activation filter', () => {
|
||||
test('hook with no "when" is always active', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'test-cap', point: 'plan:pre', ref: { skill: 'my-skill' } }],
|
||||
});
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
|
||||
assert.strictEqual(result.activeHooks.length, 1);
|
||||
assert.strictEqual(result.activeHooks[0].capId, 'test-cap');
|
||||
});
|
||||
|
||||
test('hook with when="mytool.on", config{mytool:{on:true}} → active', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'test-cap', point: 'plan:pre', ref: { skill: 'my-skill' }, when: 'mytool.on' }],
|
||||
});
|
||||
const config = { mytool: { on: true } };
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config });
|
||||
assert.strictEqual(result.activeHooks.length, 1);
|
||||
assert.strictEqual(result.activeHooks[0].kind, 'step');
|
||||
});
|
||||
|
||||
test('hook with when="mytool.on", config{mytool:{on:false}} → filtered', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'test-cap', point: 'plan:pre', ref: { skill: 'my-skill' }, when: 'mytool.on' }],
|
||||
});
|
||||
const config = { mytool: { on: false } };
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config });
|
||||
assert.strictEqual(result.activeHooks.length, 0);
|
||||
});
|
||||
|
||||
test('hook with when="mytool.on", config{} (absent key) → filtered', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'test-cap', point: 'plan:pre', ref: { skill: 'my-skill' }, when: 'mytool.on' }],
|
||||
});
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
|
||||
assert.strictEqual(result.activeHooks.length, 0);
|
||||
});
|
||||
|
||||
test('hook with when="mytool.on", config{mytool:{}} → filtered (key absent)', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'test-cap', point: 'plan:pre', ref: { skill: 'my-skill' }, when: 'mytool.on' }],
|
||||
});
|
||||
const config = { mytool: {} };
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config });
|
||||
assert.strictEqual(result.activeHooks.length, 0);
|
||||
});
|
||||
|
||||
// FIX 3: non-string `when` → INACTIVE (not always-active)
|
||||
test('hook with when=true (boolean) → inactive (FIX 3: malformed non-string when)', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'test-cap', point: 'plan:pre', ref: { skill: 'my-skill' }, when: true }],
|
||||
});
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
|
||||
assert.strictEqual(result.activeHooks.length, 0, 'non-string when=true must be treated as inactive');
|
||||
});
|
||||
|
||||
test('hook with when=42 (number) → inactive (FIX 3)', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'test-cap', point: 'plan:pre', ref: { skill: 'my-skill' }, when: 42 }],
|
||||
});
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
|
||||
assert.strictEqual(result.activeHooks.length, 0, 'non-string when=42 must be inactive');
|
||||
});
|
||||
|
||||
test('hook with when={} (object) → inactive (FIX 3)', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'test-cap', point: 'plan:pre', ref: { skill: 'my-skill' }, when: {} }],
|
||||
});
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
|
||||
assert.strictEqual(result.activeHooks.length, 0, 'non-string when={} must be inactive');
|
||||
});
|
||||
|
||||
// FIX 4: configSchema default=true → active with absent config (no cwd → level 4 applies)
|
||||
test('configSchema default=true + absent config → active', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'test-cap', point: 'plan:pre', ref: { skill: 'my-skill' }, when: 'mytool.on' }],
|
||||
configSchema: {
|
||||
'mytool.on': { type: 'boolean', default: true, description: 'Enable mytool.' },
|
||||
},
|
||||
});
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
|
||||
assert.strictEqual(result.activeHooks.length, 1, 'schema default=true should activate the hook');
|
||||
assert.strictEqual(result.activeHooks[0].capId, 'test-cap');
|
||||
});
|
||||
|
||||
// FIX 4: configSchema default=false → inactive with absent config
|
||||
test('configSchema default=false + absent config → inactive', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'test-cap', point: 'plan:pre', ref: { skill: 'my-skill' }, when: 'mytool.on' }],
|
||||
configSchema: {
|
||||
'mytool.on': { type: 'boolean', default: false, description: 'Disabled by default.' },
|
||||
},
|
||||
});
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
|
||||
assert.strictEqual(result.activeHooks.length, 0, 'schema default=false should keep hook inactive');
|
||||
});
|
||||
|
||||
// FIX 4: configSchema default=true but explicit config override=false → inactive (config wins)
|
||||
test('configSchema default=true but config override false → inactive (config wins)', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'test-cap', point: 'plan:pre', ref: { skill: 'my-skill' }, when: 'mytool.on' }],
|
||||
configSchema: {
|
||||
'mytool.on': { type: 'boolean', default: true, description: 'Enabled by default.' },
|
||||
},
|
||||
});
|
||||
const config = { mytool: { on: false } };
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config });
|
||||
assert.strictEqual(result.activeHooks.length, 0, 'explicit config=false overrides schema default=true');
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 3. UI pilot integration tests ───────────────────────────────────────────
|
||||
|
||||
describe('UI pilot integration', () => {
|
||||
test('plan:pre with workflow.ui_phase=true → ui-phase step active', () => {
|
||||
const config = { workflow: { ui_phase: true, ui_review: true, ui_safety_gate: true } };
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry: realRegistry, config });
|
||||
const uiStep = result.activeHooks.find(h => h.capId === 'ui' && h.kind === 'step');
|
||||
assert.ok(uiStep, 'Expected ui step at plan:pre');
|
||||
assert.deepEqual(uiStep.ref, { skill: 'ui-phase' });
|
||||
assert.ok(Array.isArray(uiStep.produces));
|
||||
assert.ok(uiStep.produces.includes('UI-SPEC.md'));
|
||||
});
|
||||
|
||||
test('plan:pre with workflow.ui_phase=false → ui-phase step filtered', () => {
|
||||
const config = { workflow: { ui_phase: false, ui_review: true, ui_safety_gate: true } };
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry: realRegistry, config });
|
||||
const uiStep = result.activeHooks.find(h => h.capId === 'ui' && h.kind === 'step');
|
||||
assert.strictEqual(uiStep, undefined, 'Expected ui step to be filtered');
|
||||
});
|
||||
|
||||
// FIX 4 INVERSION: empty config + real registry → ui-phase IS active (schema default=true)
|
||||
test('plan:pre with empty config + real registry → ui-phase step active by default (FIX 4)', () => {
|
||||
// realRegistry has configSchema['workflow.ui_phase'].default === true
|
||||
// So with no config and no cwd, the schema default kicks in → active
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry: realRegistry, config: {} });
|
||||
const uiStep = result.activeHooks.find(h => h.capId === 'ui' && h.kind === 'step');
|
||||
assert.ok(
|
||||
uiStep,
|
||||
'Expected ui step to be active by default (configSchema.default=true). Got: ' +
|
||||
JSON.stringify(result.activeHooks),
|
||||
);
|
||||
assert.strictEqual(uiStep.when, 'workflow.ui_phase');
|
||||
});
|
||||
|
||||
test('execute:wave:post with workflow.ui_safety_gate=true → ui gate active', () => {
|
||||
const config = { workflow: { ui_phase: true, ui_review: true, ui_safety_gate: true } };
|
||||
const result = resolveLoopHooks({ point: 'execute:wave:post', registry: realRegistry, config });
|
||||
const uiGate = result.activeHooks.find(h => h.capId === 'ui' && h.kind === 'gate');
|
||||
assert.ok(uiGate, 'Expected ui gate at execute:wave:post');
|
||||
assert.strictEqual(uiGate.blocking, true);
|
||||
assert.strictEqual(uiGate.onError, 'halt');
|
||||
});
|
||||
|
||||
test('execute:wave:post with workflow.ui_safety_gate=false → ui gate filtered', () => {
|
||||
const config = { workflow: { ui_phase: true, ui_review: true, ui_safety_gate: false } };
|
||||
const result = resolveLoopHooks({ point: 'execute:wave:post', registry: realRegistry, config });
|
||||
const uiGate = result.activeHooks.find(h => h.capId === 'ui' && h.kind === 'gate');
|
||||
assert.strictEqual(uiGate, undefined, 'Expected ui gate to be filtered');
|
||||
});
|
||||
|
||||
// FIX 4: execute:wave:post with empty config → ui gate active by schema default
|
||||
test('execute:wave:post with empty config → ui gate active by schema default', () => {
|
||||
const result = resolveLoopHooks({ point: 'execute:wave:post', registry: realRegistry, config: {} });
|
||||
const uiGate = result.activeHooks.find(h => h.capId === 'ui' && h.kind === 'gate');
|
||||
assert.ok(uiGate, 'Expected ui gate active by default (configSchema.default=true)');
|
||||
assert.strictEqual(uiGate.blocking, true);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 4. Ordering tests ────────────────────────────────────────────────────────
|
||||
|
||||
describe('hook ordering', () => {
|
||||
test('steps appear before contributions before gates', () => {
|
||||
const registry = makeRegistry({
|
||||
point: 'plan:pre',
|
||||
steps: [{ capId: 'c1', point: 'plan:pre', ref: { skill: 'sk1' } }],
|
||||
contributions: [{ capId: 'c2', point: 'plan:pre', into: 'planner' }],
|
||||
gates: { 'plan:pre': [{ capId: 'c3', point: 'plan:pre', check: { query: 'some-gate' }, blocking: false }] },
|
||||
});
|
||||
const config = {};
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config });
|
||||
assert.strictEqual(result.activeHooks.length, 3);
|
||||
assert.strictEqual(result.activeHooks[0].kind, 'step');
|
||||
assert.strictEqual(result.activeHooks[1].kind, 'contribution');
|
||||
assert.strictEqual(result.activeHooks[2].kind, 'gate');
|
||||
});
|
||||
|
||||
test('within steps, registry order is preserved', () => {
|
||||
const registry = makeRegistry({
|
||||
point: 'plan:pre',
|
||||
steps: [
|
||||
{ capId: 'cap-a', point: 'plan:pre', ref: { skill: 'a' } },
|
||||
{ capId: 'cap-b', point: 'plan:pre', ref: { skill: 'b' } },
|
||||
{ capId: 'cap-c', point: 'plan:pre', ref: { skill: 'c' } },
|
||||
],
|
||||
});
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
|
||||
assert.deepEqual(result.activeHooks.map(h => h.capId), ['cap-a', 'cap-b', 'cap-c']);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 5. Envelope shape ────────────────────────────────────────────────────────
|
||||
|
||||
describe('envelope shape', () => {
|
||||
test('envelope has point, activeHooks, rendered from renderLoopHooks', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'cap-a', point: 'plan:pre', ref: { skill: 'my-skill' }, produces: ['A.md'], consumes: ['B.md'] }],
|
||||
});
|
||||
const resolved = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
|
||||
const rendered = renderLoopHooks(resolved);
|
||||
assert.strictEqual(resolved.point, 'plan:pre');
|
||||
assert.ok(Array.isArray(resolved.activeHooks));
|
||||
assert.strictEqual(typeof rendered, 'string');
|
||||
});
|
||||
|
||||
test('empty activeHooks → rendered is non-empty placeholder string', () => {
|
||||
const registry = makeRegistry(); // all empty
|
||||
const resolved = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
|
||||
const rendered = renderLoopHooks(resolved);
|
||||
assert.strictEqual(resolved.activeHooks.length, 0);
|
||||
assert.ok(rendered.length > 0, 'rendered should be a non-empty placeholder');
|
||||
assert.match(rendered, /plan:pre/);
|
||||
});
|
||||
|
||||
test('rendered contains hook content when hooks are active', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'ui', point: 'plan:pre', ref: { skill: 'ui-phase' }, produces: ['UI-SPEC.md'], consumes: ['CONTEXT.md'], when: 'workflow.ui_phase', onError: 'skip' }],
|
||||
});
|
||||
const config = { workflow: { ui_phase: true } };
|
||||
const resolved = resolveLoopHooks({ point: 'plan:pre', registry, config });
|
||||
const rendered = renderLoopHooks(resolved);
|
||||
assert.match(rendered, /ui-phase/);
|
||||
assert.match(rendered, /ui/);
|
||||
assert.match(rendered, /UI-SPEC\.md/);
|
||||
});
|
||||
|
||||
test('rendered for UI pilot at plan:pre with all flags on', () => {
|
||||
const config = { workflow: { ui_phase: true, ui_review: true, ui_safety_gate: true } };
|
||||
const resolved = resolveLoopHooks({ point: 'plan:pre', registry: realRegistry, config });
|
||||
const rendered = renderLoopHooks(resolved);
|
||||
assert.match(rendered, /ui-phase/);
|
||||
assert.match(rendered, /UI-SPEC\.md/);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 6. Malformed registry resilience ────────────────────────────────────────
|
||||
|
||||
describe('malformed registry resilience', () => {
|
||||
test('missing byLoopPoint → no throw, empty activeHooks', () => {
|
||||
const badRegistry = {}; // no byLoopPoint
|
||||
// No throw — but point validation falls back to CANONICAL_POINTS_FALLBACK
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry: badRegistry, config: {} });
|
||||
assert.strictEqual(result.activeHooks.length, 0);
|
||||
});
|
||||
|
||||
test('null hook in steps array → skipped', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [null, { capId: 'ok', point: 'plan:pre', ref: { skill: 'ok-skill' } }, undefined],
|
||||
});
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
|
||||
assert.strictEqual(result.activeHooks.length, 1);
|
||||
assert.strictEqual(result.activeHooks[0].capId, 'ok');
|
||||
});
|
||||
|
||||
test('byLoopPoint[point] missing arrays → no throw, empty result', () => {
|
||||
const registry = { byLoopPoint: { 'plan:pre': {} } }; // no steps/contributions/gates keys
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
|
||||
assert.strictEqual(result.activeHooks.length, 0);
|
||||
});
|
||||
|
||||
test('byLoopPoint[point] has non-array steps → treated as empty', () => {
|
||||
const registry = { byLoopPoint: { 'plan:pre': { steps: 'bad', contributions: [], gates: [] } } };
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
|
||||
assert.strictEqual(result.activeHooks.length, 0);
|
||||
});
|
||||
|
||||
test('byLoopPoint[point] is null → no throw, empty result', () => {
|
||||
const registry = { byLoopPoint: { 'plan:pre': null } };
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
|
||||
assert.strictEqual(result.activeHooks.length, 0);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 7. Prototype-pollution guard ────────────────────────────────────────────
|
||||
|
||||
describe('prototype-pollution guard', () => {
|
||||
test('when="__proto__.x" does not pollute Object.prototype', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'attacker', point: 'plan:pre', ref: { skill: 'evil' }, when: '__proto__.x' }],
|
||||
});
|
||||
const config = { x: 'injected' };
|
||||
// Should not throw and should not activate (guard returns found:false)
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config });
|
||||
assert.strictEqual(result.activeHooks.length, 0);
|
||||
// Object.prototype must not be polluted
|
||||
assert.strictEqual(({}).x, undefined);
|
||||
});
|
||||
|
||||
test('when="constructor.x" does not pollute', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'attacker', point: 'plan:pre', ref: { skill: 'evil' }, when: 'constructor.x' }],
|
||||
});
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
|
||||
assert.strictEqual(result.activeHooks.length, 0);
|
||||
});
|
||||
|
||||
test('when="prototype.x" does not pollute', () => {
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'attacker', point: 'plan:pre', ref: { skill: 'evil' }, when: 'prototype.x' }],
|
||||
});
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config: {} });
|
||||
assert.strictEqual(result.activeHooks.length, 0);
|
||||
});
|
||||
|
||||
test('_getNestedConfigValue: __proto__ segment returns found:false', () => {
|
||||
const r = _getNestedConfigValue({}, '__proto__.x');
|
||||
assert.strictEqual(r.found, false);
|
||||
});
|
||||
|
||||
test('_getNestedConfigValue: constructor segment returns found:false', () => {
|
||||
const r = _getNestedConfigValue({}, 'constructor.toString');
|
||||
assert.strictEqual(r.found, false);
|
||||
});
|
||||
|
||||
test('_getNestedConfigValue: normal dotted key traversal works', () => {
|
||||
const config = { workflow: { ui_phase: true } };
|
||||
const r = _getNestedConfigValue(config, 'workflow.ui_phase');
|
||||
assert.strictEqual(r.found, true);
|
||||
assert.strictEqual(r.value, true);
|
||||
});
|
||||
|
||||
// FIX 4: raw config.json with __proto__ key does not pollute via _readRawConfigKey
|
||||
test('raw config.json with "__proto__" key does not pollute Object.prototype', () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'loop-resolver-proto-'));
|
||||
try {
|
||||
// Write a raw config.json containing __proto__ at top level and nested
|
||||
// (JSON.parse of {"__proto__":{"x":"polluted"}} does NOT set prototype in modern Node,
|
||||
// but we verify our guarded traversal returns found:false for such keys)
|
||||
const maliciousConfig = '{"__proto__":{"x":"polluted"},"workflow":{"ui_phase":true}}';
|
||||
fs.writeFileSync(path.join(tmpDir, 'config.json'), maliciousConfig, 'utf8');
|
||||
// _readRawConfigKey with '__proto__.x' should return found:false (guard)
|
||||
const r1 = _readRawConfigKey(path.join(tmpDir, 'config.json'), '__proto__.x');
|
||||
assert.strictEqual(r1.found, false, '__proto__ lookup must be guarded');
|
||||
// Normal key should work
|
||||
const r2 = _readRawConfigKey(path.join(tmpDir, 'config.json'), 'workflow.ui_phase');
|
||||
assert.strictEqual(r2.found, true);
|
||||
assert.strictEqual(r2.value, true);
|
||||
// Object.prototype must not be polluted
|
||||
assert.strictEqual(({}).x, undefined);
|
||||
} finally {
|
||||
cleanup(tmpDir);
|
||||
}
|
||||
});
|
||||
|
||||
// FIX 4: _resolveActivationValue with cwd pointing to project with __proto__ config key
|
||||
test('_resolveActivationValue: raw config with __proto__ key does not pollute', () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'loop-resolver-proto2-'));
|
||||
try {
|
||||
const planningDir = path.join(tmpDir, '.planning');
|
||||
fs.mkdirSync(planningDir, { recursive: true });
|
||||
fs.writeFileSync(
|
||||
path.join(planningDir, 'config.json'),
|
||||
'{"__proto__":{"y":"polluted2"}}',
|
||||
'utf8',
|
||||
);
|
||||
const registry = makeRegistry({
|
||||
steps: [{ capId: 'test', point: 'plan:pre', ref: { skill: 'sk' }, when: '__proto__.y' }],
|
||||
});
|
||||
const result = resolveLoopHooks({ point: 'plan:pre', registry, config: {}, cwd: tmpDir });
|
||||
assert.strictEqual(result.activeHooks.length, 0, '__proto__ when must be inactive');
|
||||
assert.strictEqual(({}).y, undefined, 'Object.prototype.y must not be polluted');
|
||||
} finally {
|
||||
cleanup(tmpDir);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 7b. Raw config.json override paths (FIX 4) ──────────────────────────────
|
||||
|
||||
describe('raw config.json override paths (FIX 4)', () => {
|
||||
// FIX 4: user sets workflow.ui_phase=false in root config.json → hook filtered
|
||||
test('root config.json with ui_phase=false overrides schema default=true → inactive', () => {
|
||||
// tmpFalseConfigProjectDir has .planning/config.json { workflow: { ui_phase: false } }
|
||||
const result = resolveLoopHooks({
|
||||
point: 'plan:pre',
|
||||
registry: realRegistry,
|
||||
config: {}, // empty loadConfig result (simulating pre-cutover)
|
||||
cwd: tmpFalseConfigProjectDir,
|
||||
});
|
||||
const uiStep = result.activeHooks.find(h => h.capId === 'ui' && h.kind === 'step');
|
||||
assert.strictEqual(
|
||||
uiStep,
|
||||
undefined,
|
||||
'root config.json override false must beat schema default=true',
|
||||
);
|
||||
});
|
||||
|
||||
// FIX 4: root config.json with ui_phase=true (explicit) → hook active
|
||||
test('root config.json with ui_phase=true → active (raw config read path)', () => {
|
||||
// tmpProjectDir has .planning/config.json { workflow: { ui_phase: true } }
|
||||
const result = resolveLoopHooks({
|
||||
point: 'plan:pre',
|
||||
registry: realRegistry,
|
||||
config: {}, // empty loadConfig result (simulating pre-cutover)
|
||||
cwd: tmpProjectDir,
|
||||
});
|
||||
const uiStep = result.activeHooks.find(h => h.capId === 'ui' && h.kind === 'step');
|
||||
assert.ok(uiStep, 'root config.json ui_phase=true should activate hook');
|
||||
});
|
||||
|
||||
// FIX 4: no config.json at all → falls through to schema default=true → active
|
||||
test('no config.json → schema default=true → hook active', () => {
|
||||
// tmpEmptyProjectDir has .planning/ directory but no config.json
|
||||
const result = resolveLoopHooks({
|
||||
point: 'plan:pre',
|
||||
registry: realRegistry,
|
||||
config: {}, // empty loadConfig result
|
||||
cwd: tmpEmptyProjectDir,
|
||||
});
|
||||
const uiStep = result.activeHooks.find(h => h.capId === 'ui' && h.kind === 'step');
|
||||
assert.ok(uiStep, 'no config.json → schema default=true → hook should be active');
|
||||
});
|
||||
|
||||
// FIX 4: _readRawConfigKey returns found:false for missing file (ENOENT — silent)
|
||||
test('_readRawConfigKey: missing file → found:false, no throw', () => {
|
||||
const result = _readRawConfigKey('/nonexistent/path/config.json', 'workflow.ui_phase');
|
||||
assert.strictEqual(result.found, false);
|
||||
});
|
||||
|
||||
// FIX 4: _readRawConfigKey returns found:false for malformed JSON, warns once
|
||||
test('_readRawConfigKey: malformed JSON → found:false, no throw', () => {
|
||||
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'loop-resolver-malformed-'));
|
||||
try {
|
||||
const malformedPath = path.join(tmpDir, 'config.json');
|
||||
fs.writeFileSync(malformedPath, '{ invalid json }', 'utf8');
|
||||
const result = _readRawConfigKey(malformedPath, 'workflow.ui_phase');
|
||||
assert.strictEqual(result.found, false, 'malformed JSON should return found:false');
|
||||
} finally {
|
||||
cleanup(tmpDir);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 8. Renderer tests ────────────────────────────────────────────────────────
|
||||
|
||||
describe('renderLoopHooks', () => {
|
||||
test('step hook renders skill ref, capId, produces, consumes', () => {
|
||||
const resolved = {
|
||||
point: 'plan:pre',
|
||||
activeHooks: [{
|
||||
capId: 'ui',
|
||||
kind: 'step',
|
||||
ref: { skill: 'ui-phase' },
|
||||
when: 'workflow.ui_phase',
|
||||
produces: ['UI-SPEC.md'],
|
||||
consumes: ['CONTEXT.md'],
|
||||
onError: 'skip',
|
||||
}],
|
||||
};
|
||||
const rendered = renderLoopHooks(resolved);
|
||||
assert.match(rendered, /Step 1/);
|
||||
assert.match(rendered, /skill:ui-phase/);
|
||||
assert.match(rendered, /\(ui\)/);
|
||||
assert.match(rendered, /UI-SPEC\.md/);
|
||||
assert.match(rendered, /CONTEXT\.md/);
|
||||
assert.match(rendered, /workflow\.ui_phase/);
|
||||
assert.match(rendered, /skip/);
|
||||
});
|
||||
|
||||
test('contribution hook renders into role', () => {
|
||||
const resolved = {
|
||||
point: 'plan:pre',
|
||||
activeHooks: [{
|
||||
capId: 'contrib-cap',
|
||||
kind: 'contribution',
|
||||
into: 'planner',
|
||||
}],
|
||||
};
|
||||
const rendered = renderLoopHooks(resolved);
|
||||
assert.match(rendered, /contribution/);
|
||||
assert.match(rendered, /contrib-cap/);
|
||||
assert.match(rendered, /planner/);
|
||||
});
|
||||
|
||||
test('gate hook renders check, blocking, onError', () => {
|
||||
const resolved = {
|
||||
point: 'execute:wave:post',
|
||||
activeHooks: [{
|
||||
capId: 'ui',
|
||||
kind: 'gate',
|
||||
check: { query: 'ui.safety-gate' },
|
||||
blocking: true,
|
||||
onError: 'halt',
|
||||
}],
|
||||
};
|
||||
const rendered = renderLoopHooks(resolved);
|
||||
assert.match(rendered, /Gate/);
|
||||
assert.match(rendered, /ui/);
|
||||
assert.match(rendered, /blocking=true/);
|
||||
assert.match(rendered, /halt/);
|
||||
});
|
||||
|
||||
test('multiple hooks in order render with correct ordinals', () => {
|
||||
const resolved = {
|
||||
point: 'plan:pre',
|
||||
activeHooks: [
|
||||
{ capId: 'cap-a', kind: 'step', ref: { skill: 'a' }, produces: ['A.md'], consumes: [] },
|
||||
{ capId: 'cap-b', kind: 'step', ref: { skill: 'b' }, produces: ['B.md'], consumes: ['A.md'] },
|
||||
],
|
||||
};
|
||||
const rendered = renderLoopHooks(resolved);
|
||||
assert.match(rendered, /Step 1/);
|
||||
assert.match(rendered, /Step 2/);
|
||||
const idx1 = rendered.indexOf('Step 1');
|
||||
const idx2 = rendered.indexOf('Step 2');
|
||||
assert.ok(idx1 < idx2, 'Step 1 should appear before Step 2');
|
||||
});
|
||||
|
||||
test('empty hooks returns placeholder containing the point name', () => {
|
||||
const rendered = renderLoopHooks({ point: 'ship:post', activeHooks: [] });
|
||||
assert.match(rendered, /ship:post/);
|
||||
assert.ok(rendered.length > 0);
|
||||
});
|
||||
|
||||
test('rendered is deterministic (same input → same output)', () => {
|
||||
const config = { workflow: { ui_phase: true, ui_review: true, ui_safety_gate: true } };
|
||||
const resolved = resolveLoopHooks({ point: 'plan:pre', registry: realRegistry, config });
|
||||
const r1 = renderLoopHooks(resolved);
|
||||
const r2 = renderLoopHooks(resolved);
|
||||
assert.strictEqual(r1, r2);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── 9. End-to-end cmdLoopRenderHooks (via gsd-tools subprocess) ─────────────
|
||||
|
||||
const { spawnSync } = require('node:child_process');
|
||||
const ROOT = path.resolve(__dirname, '..');
|
||||
const GSD_TOOLS = path.join(ROOT, 'gsd-core', 'bin', 'gsd-tools.cjs');
|
||||
|
||||
describe('cmdLoopRenderHooks end-to-end (via gsd-tools)', () => {
|
||||
test('loop render-hooks plan:pre returns JSON envelope with ui-phase step active', () => {
|
||||
const result = spawnSync(
|
||||
process.execPath,
|
||||
[GSD_TOOLS, 'loop', 'render-hooks', 'plan:pre', '--cwd', tmpProjectDir],
|
||||
{ cwd: ROOT, encoding: 'utf8' },
|
||||
);
|
||||
assert.strictEqual(result.status, 0, 'Expected exit 0. stderr: ' + (result.stderr || ''));
|
||||
const envelope = JSON.parse(result.stdout.trim());
|
||||
assert.strictEqual(envelope.point, 'plan:pre');
|
||||
assert.ok(Array.isArray(envelope.activeHooks));
|
||||
assert.strictEqual(typeof envelope.rendered, 'string');
|
||||
// With ui_phase=true in tmpProjectDir config, ui-phase step should be active
|
||||
const uiStep = envelope.activeHooks.find(h => h.capId === 'ui' && h.kind === 'step');
|
||||
assert.ok(uiStep, 'Expected ui step in activeHooks. Got: ' + JSON.stringify(envelope.activeHooks));
|
||||
assert.match(envelope.rendered, /ui-phase/);
|
||||
});
|
||||
|
||||
// FIX 4: schema-default activation — no config.json in project → ui-phase step active by default
|
||||
test('loop render-hooks plan:pre with no config.json → ui-phase step active by schema default', () => {
|
||||
const result = spawnSync(
|
||||
process.execPath,
|
||||
[GSD_TOOLS, 'loop', 'render-hooks', 'plan:pre', '--cwd', tmpEmptyProjectDir],
|
||||
{ cwd: ROOT, encoding: 'utf8' },
|
||||
);
|
||||
assert.strictEqual(result.status, 0, 'Expected exit 0. stderr: ' + (result.stderr || ''));
|
||||
const envelope = JSON.parse(result.stdout.trim());
|
||||
const uiStep = envelope.activeHooks.find(h => h.capId === 'ui' && h.kind === 'step');
|
||||
assert.ok(
|
||||
uiStep,
|
||||
'Expected ui step active by default. Got: ' + JSON.stringify(envelope.activeHooks),
|
||||
);
|
||||
assert.match(envelope.rendered, /ui-phase/);
|
||||
});
|
||||
|
||||
// FIX 4: explicit false in config.json overrides schema default
|
||||
test('loop render-hooks plan:pre with ui_phase=false in config.json → ui-phase step absent', () => {
|
||||
const result = spawnSync(
|
||||
process.execPath,
|
||||
[GSD_TOOLS, 'loop', 'render-hooks', 'plan:pre', '--cwd', tmpFalseConfigProjectDir],
|
||||
{ cwd: ROOT, encoding: 'utf8' },
|
||||
);
|
||||
assert.strictEqual(result.status, 0, 'Expected exit 0. stderr: ' + (result.stderr || ''));
|
||||
const envelope = JSON.parse(result.stdout.trim());
|
||||
const uiStep = envelope.activeHooks.find(h => h.capId === 'ui' && h.kind === 'step');
|
||||
assert.strictEqual(
|
||||
uiStep,
|
||||
undefined,
|
||||
'ui-phase step should be absent when config.json sets ui_phase=false',
|
||||
);
|
||||
});
|
||||
|
||||
test('loop render-hooks invalid-point exits non-zero', () => {
|
||||
const result = spawnSync(
|
||||
process.execPath,
|
||||
[GSD_TOOLS, 'loop', 'render-hooks', 'plan:mid', '--cwd', tmpProjectDir],
|
||||
{ cwd: ROOT, encoding: 'utf8' },
|
||||
);
|
||||
assert.notStrictEqual(result.status, 0, 'Expected non-zero exit for invalid point');
|
||||
assert.match(result.stderr, /plan:mid|Invalid loop point/);
|
||||
});
|
||||
});
|
||||
@@ -3364,6 +3364,55 @@ describe('bug #1962: normalizePhaseName preserves letter suffix case', () => {
|
||||
// (consolidated from tests/bug-1998-phase-complete-checkbox.test.cjs)
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Run `gsd-tools phase complete <phase>` for the phase-complete regression
|
||||
* suites and return its stdout.
|
||||
*
|
||||
* `phase complete` writes ROADMAP.md as its LAST step, after a read-heavy
|
||||
* parse/lock sequence (ROADMAP read, two extractCurrentMilestone STATE.md
|
||||
* parses, REQUIREMENTS read, phase-dir scan, STATE read — all before the single
|
||||
* writePlanningFileSet flush). Under the high-concurrency docker run (~672 test
|
||||
* files in parallel), a tight 10s timeout could fire mid-parse and SIGTERM the
|
||||
* subprocess BEFORE that write landed, leaving ROADMAP.md untouched. Call sites
|
||||
* that used a bare `catch {}` then silently proceeded to assert on the pristine
|
||||
* file — an intermittent "checkbox not checked" failure (bug #1998 flake).
|
||||
*
|
||||
* Two-part fix, no retry loop:
|
||||
* 1. A generous timeout so the test's own timer never kills the subprocess
|
||||
* under load (10s cold-node startup × 672-way CPU/IO contention was the
|
||||
* real culprit — all I/O is scoped to tmpDir, so there is no cross-process
|
||||
* race to blame).
|
||||
* 2. Never silently swallow a signal/timeout kill: it means the process was
|
||||
* terminated before completing its writes, so we surface it loudly with
|
||||
* context instead of letting it masquerade as an assertion failure. A
|
||||
* *clean* non-zero exit is still tolerated when `tolerateExit` is set,
|
||||
* because the ROADMAP write has already landed before any post-write step
|
||||
* that may exit non-zero in these minimal fixtures.
|
||||
*/
|
||||
function runPhaseComplete(tmpDir, { phase = '1', tolerateExit = false } = {}) {
|
||||
try {
|
||||
return execFileSync('node', [GSD_TOOLS_BIN, 'phase', 'complete', phase], {
|
||||
cwd: tmpDir,
|
||||
timeout: 60000,
|
||||
encoding: 'utf-8',
|
||||
});
|
||||
} catch (err) {
|
||||
// A signal/timeout kill terminated the process before it finished writing —
|
||||
// never tolerate it; surface it with whatever output was captured.
|
||||
if (err.killed || err.signal != null || err.code === 'ETIMEDOUT') {
|
||||
throw new Error(
|
||||
`gsd-tools phase complete ${phase} was killed before completion ` +
|
||||
`(signal=${err.signal}, code=${err.code}). ` +
|
||||
`stdout=${err.stdout || ''} stderr=${err.stderr || ''}`
|
||||
);
|
||||
}
|
||||
if (tolerateExit) {
|
||||
return `${err.stdout || ''}${err.stderr || ''}`;
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
describe('bug #1998: phase complete updates overview checkbox', () => {
|
||||
let tmpDir;
|
||||
let planningDir;
|
||||
@@ -3419,11 +3468,7 @@ describe('bug #1998: phase complete updates overview checkbox', () => {
|
||||
'| 2. Features | 0/1 | Pending | - |',
|
||||
].join('\n'));
|
||||
|
||||
try {
|
||||
execFileSync('node', [GSD_TOOLS_BIN, 'phase', 'complete', '1'], { cwd: tmpDir, timeout: 10000 });
|
||||
} catch {
|
||||
// Command may exit non-zero if STATE.md update fails, but ROADMAP.md update happens first
|
||||
}
|
||||
runPhaseComplete(tmpDir, { tolerateExit: true });
|
||||
|
||||
const result = fs.readFileSync(roadmapPath, 'utf-8');
|
||||
assert.match(result, /- \[x\] \*\*Phase 1: Foundation\*\*/, 'overview checkbox should be checked');
|
||||
@@ -3468,11 +3513,7 @@ describe('bug #1998: phase complete updates overview checkbox', () => {
|
||||
'</details>',
|
||||
].join('\n'));
|
||||
|
||||
try {
|
||||
execFileSync('node', [GSD_TOOLS_BIN, 'phase', 'complete', '1'], { cwd: tmpDir, timeout: 10000 });
|
||||
} catch {
|
||||
// May exit non-zero
|
||||
}
|
||||
runPhaseComplete(tmpDir, { tolerateExit: true });
|
||||
|
||||
const result = fs.readFileSync(roadmapPath, 'utf-8');
|
||||
assert.match(result, /- \[x\] \*\*Phase 1: Setup\*\*/, 'current milestone checkbox should be checked');
|
||||
@@ -3559,11 +3600,7 @@ describe('bug #2005: phase complete updates plan count when milestone is inside
|
||||
'</details>',
|
||||
].join('\n'));
|
||||
|
||||
try {
|
||||
execFileSync('node', [GSD_TOOLS_BIN, 'phase', 'complete', '1'], { cwd: tmpDir, timeout: 10000 });
|
||||
} catch {
|
||||
// May exit non-zero if STATE.md update fails, but ROADMAP.md update is the target
|
||||
}
|
||||
runPhaseComplete(tmpDir, { tolerateExit: true });
|
||||
|
||||
const result = fs.readFileSync(roadmapPath, 'utf-8');
|
||||
|
||||
@@ -3619,9 +3656,7 @@ describe('bug #2005: phase complete updates plan count when milestone is inside
|
||||
'</details>',
|
||||
].join('\n'));
|
||||
|
||||
try {
|
||||
execFileSync('node', [GSD_TOOLS_BIN, 'phase', 'complete', '1'], { cwd: tmpDir, timeout: 10000 });
|
||||
} catch {}
|
||||
runPhaseComplete(tmpDir, { tolerateExit: true });
|
||||
|
||||
const result = fs.readFileSync(roadmapPath, 'utf-8');
|
||||
|
||||
@@ -3709,22 +3744,7 @@ describe('bug #2526: phase complete warns about unregistered REQ-IDs', () => {
|
||||
'| REQ-001 | 1 | Pending |',
|
||||
].join('\n'));
|
||||
|
||||
let stdout = '';
|
||||
let stderr = '';
|
||||
try {
|
||||
const result = execFileSync('node', [GSD_TOOLS_BIN, 'phase', 'complete', '1'], {
|
||||
cwd: tmpDir,
|
||||
timeout: 10000,
|
||||
encoding: 'utf-8',
|
||||
});
|
||||
stdout = result;
|
||||
} catch (err) {
|
||||
stdout = err.stdout || '';
|
||||
stderr = err.stderr || '';
|
||||
throw err;
|
||||
}
|
||||
|
||||
const combined = stdout + stderr;
|
||||
const combined = runPhaseComplete(tmpDir);
|
||||
assert.match(combined, /REQ-002/, 'output should mention REQ-002 as missing from Traceability table');
|
||||
assert.match(combined, /REQ-003/, 'output should mention REQ-003 as missing from Traceability table');
|
||||
});
|
||||
@@ -3776,22 +3796,7 @@ describe('bug #2526: phase complete warns about unregistered REQ-IDs', () => {
|
||||
'| REQ-002 | 1 | Pending |',
|
||||
].join('\n'));
|
||||
|
||||
let stdout = '';
|
||||
let stderr = '';
|
||||
try {
|
||||
const result = execFileSync('node', [GSD_TOOLS_BIN, 'phase', 'complete', '1'], {
|
||||
cwd: tmpDir,
|
||||
timeout: 10000,
|
||||
encoding: 'utf-8',
|
||||
});
|
||||
stdout = result;
|
||||
} catch (err) {
|
||||
stdout = err.stdout || '';
|
||||
stderr = err.stderr || '';
|
||||
throw err;
|
||||
}
|
||||
|
||||
const combined = stdout + stderr;
|
||||
const combined = runPhaseComplete(tmpDir);
|
||||
assert.doesNotMatch(
|
||||
combined,
|
||||
/unregistered|missing.*traceability|not in.*traceability/i,
|
||||
@@ -3845,22 +3850,7 @@ describe('bug #2526: phase complete warns about unregistered REQ-IDs', () => {
|
||||
'| REQ-001 | 1 | Pending |',
|
||||
].join('\n'));
|
||||
|
||||
let stdout = '';
|
||||
let stderr = '';
|
||||
try {
|
||||
const result = execFileSync('node', [GSD_TOOLS_BIN, 'phase', 'complete', '1'], {
|
||||
cwd: tmpDir,
|
||||
timeout: 10000,
|
||||
encoding: 'utf-8',
|
||||
});
|
||||
stdout = result;
|
||||
} catch (err) {
|
||||
stdout = err.stdout || '';
|
||||
stderr = err.stderr || '';
|
||||
throw err;
|
||||
}
|
||||
|
||||
const combined = stdout + stderr;
|
||||
const combined = runPhaseComplete(tmpDir);
|
||||
assert.match(combined, /REQ-002/, 'should warn about REQ-002');
|
||||
assert.match(combined, /REQ-003/, 'should warn about REQ-003');
|
||||
assert.match(combined, /REQ-004/, 'should warn about REQ-004');
|
||||
|
||||
@@ -135,3 +135,61 @@ describe('plan-phase workflow: Artifacts this phase produces section (#22)', ()
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── (C) Top-level spawn guard (#913) ────────────────────────────────────────
|
||||
|
||||
describe('plan-phase workflow: top-level spawn guard (#913)', () => {
|
||||
// Extract the runtime_compatibility block for targeted assertions
|
||||
const rtBlock = (() => {
|
||||
const m = workflow.match(/<runtime_compatibility>([\s\S]*?)<\/runtime_compatibility>/);
|
||||
return m ? m[1] : '';
|
||||
})();
|
||||
|
||||
test('workflow has a runtime_compatibility block asserting Agent is available at top-level', () => {
|
||||
assert.ok(
|
||||
rtBlock.length > 0,
|
||||
'plan-phase must have a <runtime_compatibility> block — prevents role-collapse regression (#913)'
|
||||
);
|
||||
assert.ok(
|
||||
rtBlock.includes('Agent tool IS available') || rtBlock.includes('Agent IS available'),
|
||||
'plan-phase runtime_compatibility must assert that the Agent tool IS available at top-level Claude Code (#913)'
|
||||
);
|
||||
assert.ok(
|
||||
rtBlock.toLowerCase().includes('top-level'),
|
||||
'plan-phase runtime_compatibility must scope the IS-available assertion to top-level Claude Code (#913)'
|
||||
);
|
||||
assert.ok(
|
||||
rtBlock.includes('Always spawn') || rtBlock.includes('always spawn'),
|
||||
'plan-phase runtime_compatibility must state that plan roles must always be spawned (#913)'
|
||||
);
|
||||
assert.ok(
|
||||
rtBlock.includes('Never absorb') || rtBlock.includes('never absorb'),
|
||||
'plan-phase runtime_compatibility must state that roles must never be absorbed inline (#913)'
|
||||
);
|
||||
});
|
||||
|
||||
test('workflow states --chain/--auto suppress prompts only, not spawns', () => {
|
||||
assert.ok(
|
||||
rtBlock.includes('suppress') &&
|
||||
(rtBlock.includes('prompts only') || rtBlock.includes('interactive prompts only')),
|
||||
'plan-phase runtime_compatibility must document that --chain/--auto suppress prompts only, not spawns (#913)'
|
||||
);
|
||||
});
|
||||
|
||||
test('workflow does not contain unscoped CODEX RUNTIME orchestrator rule labels', () => {
|
||||
// All "wait for subagent" rules must apply to ALL RUNTIMES, not just Codex
|
||||
assert.ok(
|
||||
!workflow.includes('ORCHESTRATOR RULE — CODEX RUNTIME'),
|
||||
'plan-phase must not label orchestrator wait rules as "CODEX RUNTIME" — they apply to all runtimes including top-level Claude Code (#913)'
|
||||
);
|
||||
});
|
||||
|
||||
test('workflow contains ALL RUNTIMES orchestrator rule labels (count preserved)', () => {
|
||||
// Must have all 7 agent-spawn wait rules still present (none dropped during rename)
|
||||
const allRuntimesCount = (workflow.match(/ORCHESTRATOR RULE — ALL RUNTIMES/g) || []).length;
|
||||
assert.ok(
|
||||
allRuntimesCount >= 7,
|
||||
`plan-phase must have at least 7 "ORCHESTRATOR RULE — ALL RUNTIMES" labels (one per agent spawn site); found ${allRuntimesCount} (#913)`
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -81,8 +81,8 @@ const GRACE = 3000;
|
||||
// current high-water mark within GRACE (#597 tighten-only ratchet).
|
||||
// XL high-water mark is execute-phase.md — note that under LINES it was
|
||||
// plan-phase; bytes genuinely re-rank the tier, which is the point of #717.
|
||||
// actualMax=91161 (execute-phase, #891 launcher shim expansion — added 17 runtime home arms);
|
||||
// slack=1839 ≤ GRACE. plan-phase.md=88120, new-project.md=58110; both well under ceiling.
|
||||
// actualMax=92525 (execute-phase, #913 inline-fallback scope clarification);
|
||||
// slack=475 ≤ GRACE. plan-phase.md=90501 (#913 runtime_compatibility block + label rename), new-project.md=58110.
|
||||
const XL_BUDGET = 93000;
|
||||
// LARGE high-water mark is docs-update.md. actualMax=54410 (#891 launcher shim expansion);
|
||||
// slack=1590 ≤ GRACE. quick.md=45710, autonomous.md=38030.
|
||||
@@ -95,8 +95,8 @@ const DEFAULT_BUDGET = 40000;
|
||||
// Grandfathered at current sizes — see PR #2551 for the progressive-disclosure
|
||||
// pattern that future shrinks should follow. Byte counts noted for reference.
|
||||
const XL_WORKFLOWS = new Set([
|
||||
'execute-phase', // 91161 bytes (tier high-water mark; grew in #891 launcher shim expansion)
|
||||
'plan-phase', // 85068 bytes
|
||||
'execute-phase', // 92525 bytes (tier high-water mark; grew in #913 inline-fallback scope clarification)
|
||||
'plan-phase', // 90501 bytes (grew in #913 runtime_compatibility block + label rename)
|
||||
'new-project', // 55850 bytes
|
||||
]);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user