Add worktree config section schema and shape validation #320

New issue

david · 2026-11-05T21:40:54Z

david commented

2026-11-05 21:40:54 +00:00

Summary

Add strict schema and shape validation for the new worktree: section (enabled, dir) in piloop-config.yaml — the parsing half of making pi-loop's worktree directory configurable (ADR-022). Downstream steps (parse-time dir validation, worktree routing, early-fetch gate) all build on this section being parsed.

Background

pi-loop is a Node.js/TypeScript orchestrator that runs each issue in a git worktree. Today worktrees are created only for bare repos, with the parent directory hardcoded to dirname(originalCwd); normal clones run in place. ADR-022 (docs/adr/022-configurable-worktree-dir.md) introduces a worktree: section so operators can choose where worktrees land and enable worktree mode for normal clones. This step covers config-file parsing only: no filesystem checks yet, no worktree-creation changes.

The config loader lives in src/config/services/configFile.ts:

parseConfigFileYaml(contents, filePath) is a pure function that strictly validates the YAML and throws ConfigFileError on unknown keys or wrong types. Private helpers: requireRecord, assertOnlyKeys, requireBoolean, requireString, requireEnum, requirePositiveInt, requireStringArray.
Sections parse into plain objects — e.g. the docs: section parses into DocsFileConfig with DOCS_KEYS = ['enabled', 'paths', 'exclude'], using those same helpers.
The result shape ConfigFileResult is also returned by emptyConfigFileResult() when the file is absent.
loadConfigFile(cwd) reads cwd/piloop-config.yaml and calls parseConfigFileYaml; errors flow to reportConfigFileError in src/cli/services/runCli.ts → error.json with kind: config, a stderr message, exit 1.
src/config/index.ts re-exports the config surface (types like DocsFileConfig).

Decision table this step enforces at the parser level (ADR-022 §2):

Section	`enabled`	`dir`	Parser behavior
absent	—	—	empty result (`worktree: {}`), no error
present	absent → `true`	present	parses; `dir` value validated by a later step
present	`true`	absent	`ConfigFileError` — no silent fallback
present	`false`	present or absent	parses; `dir` never validated

Depends on: none — this is the first step of the series.

Implementation Details

Files: src/config/services/configFile.ts, src/config/index.ts; tests in src/config/services/configFile.test.ts.

Add the exported interface (mirrors DocsFileConfig):

export interface WorktreeFileConfig {
  /** `worktree.enabled` — whether a worktree is created (default true). */
  enabled?: boolean;
  /** `worktree.dir` — absolute parent directory for new worktrees. */
  dir?: string;
}

Add worktree: WorktreeFileConfig to ConfigFileResult, and worktree: {} to emptyConfigFileResult().
Add 'worktree' to TOP_LEVEL_KEYS; add const WORKTREE_KEYS = ['enabled', 'dir'] as const (same style as DOCS_KEYS).
Parse the section alongside the existing docs block: requireRecord(raw.worktree, 'worktree', filePath) → assertOnlyKeys(section, WORKTREE_KEYS, 'worktree', filePath) → requireBoolean(section.enabled, 'worktree.enabled', filePath) when present → requireString(section.dir, 'worktree.dir', filePath) when present.

Cross-field rule (no hidden defaults):

const enabled = section.enabled ?? true;
if (enabled && dir === undefined) {
  throw new ConfigFileError(
    `${filePath}: "worktree.dir" is required when "worktree.enabled" is true ` +
      `(set "worktree.enabled: false" to use the default worktree location)`,
  );
}

Do not validate the value of dir when enabled is false — a disabled setting must never fail a run.
Export the WorktreeFileConfig type from src/config/index.ts alongside DocsFileConfig.

Acceptance Criteria

worktree: { dir: /x } (no enabled) parses and yields worktree.dir === '/x'.
worktree: { enabled: true } without dir throws ConfigFileError naming worktree.dir.
worktree: { enabled: false } without dir parses without error.
worktree: { enabled: false, dir: 'relative/path' } parses without error (parser performs no dir checks).
worktree: { dir: /x, mode: y } (unknown key) throws ConfigFileError naming worktree.mode.
worktree: true (not a mapping) throws ConfigFileError from requireRecord.
worktree: { enabled: "yes" } throws ConfigFileError from requireBoolean.
npm run lint and npm test pass; no existing tests modified.

Test Plan

Add unit tests in src/config/services/configFile.test.ts covering the seven parse cases above (the file already contains preset-config tests and an example-YAML describe block — follow the existing style). Run npm test and confirm the new tests pass and the existing config tests are untouched.

## Summary Add strict schema and shape validation for the new `worktree:` section (`enabled`, `dir`) in `piloop-config.yaml` — the parsing half of making pi-loop's worktree directory configurable (ADR-022). Downstream steps (parse-time `dir` validation, worktree routing, early-fetch gate) all build on this section being parsed. ## Background pi-loop is a Node.js/TypeScript orchestrator that runs each issue in a git worktree. Today worktrees are created **only for bare repos**, with the parent directory hardcoded to `dirname(originalCwd)`; normal clones run in place. ADR-022 (`docs/adr/022-configurable-worktree-dir.md`) introduces a `worktree:` section so operators can choose where worktrees land and enable worktree mode for normal clones. This step covers config-file parsing only: no filesystem checks yet, no worktree-creation changes. The config loader lives in `src/config/services/configFile.ts`: - `parseConfigFileYaml(contents, filePath)` is a **pure** function that strictly validates the YAML and throws `ConfigFileError` on unknown keys or wrong types. Private helpers: `requireRecord`, `assertOnlyKeys`, `requireBoolean`, `requireString`, `requireEnum`, `requirePositiveInt`, `requireStringArray`. - Sections parse into plain objects — e.g. the `docs:` section parses into `DocsFileConfig` with `DOCS_KEYS = ['enabled', 'paths', 'exclude']`, using those same helpers. - The result shape `ConfigFileResult` is also returned by `emptyConfigFileResult()` when the file is absent. - `loadConfigFile(cwd)` reads `cwd/piloop-config.yaml` and calls `parseConfigFileYaml`; errors flow to `reportConfigFileError` in `src/cli/services/runCli.ts` → `error.json` with `kind: config`, a stderr message, exit 1. - `src/config/index.ts` re-exports the config surface (types like `DocsFileConfig`). Decision table this step enforces at the parser level (ADR-022 §2): | Section | `enabled` | `dir` | Parser behavior | | --- | --- | --- | --- | | absent | — | — | empty result (`worktree: {}`), no error | | present | absent → `true` | present | parses; `dir` value validated by a later step | | present | `true` | absent | `ConfigFileError` — no silent fallback | | present | `false` | present or absent | parses; `dir` never validated | **Depends on:** none — this is the first step of the series. ## Implementation Details Files: `src/config/services/configFile.ts`, `src/config/index.ts`; tests in `src/config/services/configFile.test.ts`. 1. Add the exported interface (mirrors `DocsFileConfig`): ```ts export interface WorktreeFileConfig { /** `worktree.enabled` — whether a worktree is created (default true). */ enabled?: boolean; /** `worktree.dir` — absolute parent directory for new worktrees. */ dir?: string; } ``` 2. Add `worktree: WorktreeFileConfig` to `ConfigFileResult`, and `worktree: {}` to `emptyConfigFileResult()`. 3. Add `'worktree'` to `TOP_LEVEL_KEYS`; add `const WORKTREE_KEYS = ['enabled', 'dir'] as const` (same style as `DOCS_KEYS`). 4. Parse the section alongside the existing `docs` block: `requireRecord(raw.worktree, 'worktree', filePath)` → `assertOnlyKeys(section, WORKTREE_KEYS, 'worktree', filePath)` → `requireBoolean(section.enabled, 'worktree.enabled', filePath)` when present → `requireString(section.dir, 'worktree.dir', filePath)` when present. 5. Cross-field rule (no hidden defaults): ```ts const enabled = section.enabled ?? true; if (enabled && dir === undefined) { throw new ConfigFileError( `${filePath}: "worktree.dir" is required when "worktree.enabled" is true ` + `(set "worktree.enabled: false" to use the default worktree location)`, ); } ``` 6. Do **not** validate the value of `dir` when `enabled` is false — a disabled setting must never fail a run. 7. Export the `WorktreeFileConfig` type from `src/config/index.ts` alongside `DocsFileConfig`. ## Acceptance Criteria - [ ] `worktree: { dir: /x }` (no `enabled`) parses and yields `worktree.dir === '/x'`. - [ ] `worktree: { enabled: true }` without `dir` throws `ConfigFileError` naming `worktree.dir`. - [ ] `worktree: { enabled: false }` without `dir` parses without error. - [ ] `worktree: { enabled: false, dir: 'relative/path' }` parses without error (parser performs no `dir` checks). - [ ] `worktree: { dir: /x, mode: y }` (unknown key) throws `ConfigFileError` naming `worktree.mode`. - [ ] `worktree: true` (not a mapping) throws `ConfigFileError` from `requireRecord`. - [ ] `worktree: { enabled: "yes" }` throws `ConfigFileError` from `requireBoolean`. - [ ] `npm run lint` and `npm test` pass; no existing tests modified. ## Test Plan Add unit tests in `src/config/services/configFile.test.ts` covering the seven parse cases above (the file already contains preset-config tests and an example-YAML `describe` block — follow the existing style). Run `npm test` and confirm the new tests pass and the existing config tests are untouched.

david added this to the Configurable worktree directory milestone

2026-11-05 21:40:54 +00:00

david added the

enhancement

ready

labels

2026-11-05 21:40:54 +00:00

david referenced this issue

2026-11-05 21:46:26 +00:00

Add parse-time directory validation helper for worktree.dir #321

david referenced this issue

2026-11-05 21:46:26 +00:00