issue-320: Add worktree config section schema and shape validation #326

david · 2026-11-06T01:49:03Z

david commented

2026-11-06 01:49:03 +00:00

320: Add worktree config section schema and shape validation

Closes #320 — 320.

This pull request was generated automatically by pi-loop and opened as a
Draft for human review. Do not merge without reviewing the changes.

Generated in 18m 05s.

Review findings

I have both findings in view. Both are small, well-scoped fixes:

Medium — add the worktree: block to FULL_YAML and assert result.worktree in the full-mapping test.
Low — reword the WorktreeFileConfig JSDoc to mirror the ADR-022 §2 decision table.

Applying both now:

Both edits applied. Now checking for other consumers of FULL_YAML and other full-shape assertions that could be affected:

FULL_YAML is only used in the full-mapping test, which now covers the new section. Running the project gates:

Build passes. Now the test suite and lint:

All 2187 tests pass. Now lint and format checks on the changed files:

All gates green. Quick sanity check of the final diff:

Both fixes are minimal, applied, and all gates are green. Both findings are resolved.

Review Findings — 320

Summary

Stage 1 implements Slice 1 of the configurable-worktree plan (ADR-022): the worktree: section schema, strict key/type validation, and the "dir required when enabled" cross-field rule, with tests covering the full ADR-022 §2 decision table. All gates ran and passed: build ✅, npm test (2187/2187) ✅, npm run lint ✅, and prettier check on the changed files ✅ (gated E2E not run — requires live creds/LLM and is out of scope for this slice). No critical or high issues found; two minor gaps below. Remediated (this stage): both findings fixed — the worktree: block was added to the FULL_YAML canonical fixture and asserted in the full-mapping test, and the WorktreeFileConfig JSDoc was reworded to mirror the ADR-022 §2 decision table. Re-ran build, full test suite (2187/2187), lint, and prettier — all green. Nothing remains unresolved.

Critical

(empty)

High

(empty)

Medium

src/config/services/configFile.test.ts:20 — The FULL_YAML fixture is documented as "Full, valid example covering every schema key (ADR-013 §schema)" and the happy-path test "maps every schema key into the EnvSource-shaped map and flag fields" asserts the mapping of every key, but the new worktree: section is not in the fixture and result.worktree is never asserted in the full-shape test, so the canonical full-schema regression fixture (and its doc comment) is now stale. Suggested fix: add a worktree:\n enabled: true\n dir: /srv/pi-loop/wt block to FULL_YAML and assert expect(result.worktree).toEqual({ enabled: true, dir: '/srv/pi-loop/wt' }) in the full-mapping test.

Low

src/config/services/configFile.ts:42 — The WorktreeFileConfig JSDoc is confusing and partly self-contradictory: it says "when the section is present without enabled, worktrees are enabled by default" while the next sentence says an absent enabled without dir is a ConfigFileError — absent enabled is only "enabled" when a dir is present, per the ADR-022 §2 decision table. Suggested fix: reword to mirror the decision table, e.g. "every field is optional; when enabled is absent it is treated as true for the cross-field check, so a section without dir is a ConfigFileError (no hidden defaults); enabled: false never requires or validates dir".

Documentation

Updated files:

README.md

## 320: Add worktree config section schema and shape validation Closes #320 — [320](https://git.excelera.net/david/pi-loop/issues/320). > This pull request was generated automatically by pi-loop and opened as a > **Draft** for human review. Do not merge without reviewing the changes. Generated in 18m 05s. ## Review findings I have both findings in view. Both are small, well-scoped fixes: 1. **Medium** — add the `worktree:` block to `FULL_YAML` and assert `result.worktree` in the full-mapping test. 2. **Low** — reword the `WorktreeFileConfig` JSDoc to mirror the ADR-022 §2 decision table. Applying both now: Both edits applied. Now checking for other consumers of `FULL_YAML` and other full-shape assertions that could be affected: `FULL_YAML` is only used in the full-mapping test, which now covers the new section. Running the project gates: Build passes. Now the test suite and lint: All 2187 tests pass. Now lint and format checks on the changed files: All gates green. Quick sanity check of the final diff: Both fixes are minimal, applied, and all gates are green. Both findings are resolved. # Review Findings — 320 ## Summary Stage 1 implements Slice 1 of the configurable-worktree plan (ADR-022): the `worktree:` section schema, strict key/type validation, and the "dir required when enabled" cross-field rule, with tests covering the full ADR-022 §2 decision table. All gates ran and passed: build ✅, `npm test` (2187/2187) ✅, `npm run lint` ✅, and prettier check on the changed files ✅ (gated E2E not run — requires live creds/LLM and is out of scope for this slice). No critical or high issues found; two minor gaps below. **Remediated (this stage): both findings fixed — the `worktree:` block was added to the `FULL_YAML` canonical fixture and asserted in the full-mapping test, and the `WorktreeFileConfig` JSDoc was reworded to mirror the ADR-022 §2 decision table. Re-ran build, full test suite (2187/2187), lint, and prettier — all green. Nothing remains unresolved.** ## Critical (empty) ## High (empty) ## Medium - [x] src/config/services/configFile.test.ts:20 — The `FULL_YAML` fixture is documented as "Full, valid example covering every schema key (ADR-013 §schema)" and the happy-path test "maps every schema key into the EnvSource-shaped map and flag fields" asserts the mapping of every key, but the new `worktree:` section is not in the fixture and `result.worktree` is never asserted in the full-shape test, so the canonical full-schema regression fixture (and its doc comment) is now stale. Suggested fix: add a `worktree:\n enabled: true\n dir: /srv/pi-loop/wt` block to `FULL_YAML` and assert `expect(result.worktree).toEqual({ enabled: true, dir: '/srv/pi-loop/wt' })` in the full-mapping test. ## Low - [x] src/config/services/configFile.ts:42 — The `WorktreeFileConfig` JSDoc is confusing and partly self-contradictory: it says "when the section is present without `enabled`, worktrees are enabled by default" while the next sentence says an absent `enabled` without `dir` is a `ConfigFileError` — absent `enabled` is only "enabled" when a `dir` is present, per the ADR-022 §2 decision table. Suggested fix: reword to mirror the decision table, e.g. "every field is optional; when `enabled` is absent it is treated as `true` for the cross-field check, so a section without `dir` is a `ConfigFileError` (no hidden defaults); `enabled: false` never requires or validates `dir`". ## Documentation Updated files: - README.md

david added 3 commits

2026-11-06 01:49:03 +00:00

320: Add worktree config section schema and shape validation 2c8c55a77c