Docs stage fails every run when docs scope is covered by an ignore source — add fatal preflight, remove filterGitignoredDocFiles #286

Closed
opened 2026-08-19 02:24:59 +00:00 by david · 1 comment
Owner

Summary

The docs stage (ADR-015) fails non-fatally on every run when the resolved docs scope (config.docs.paths, default README.md + docs/**/*.md) is covered by a git ignore source (.gitignore, .git/info/exclude, or a global core.excludesFile). This has been happening silently in this repo for multiple runs (#267, #274, #282), always producing the same buried error:

[docs] WARN docs stage failed (non-fatal): Command failed: git add -- README.md docs/adr/001-...md ...
The following paths are ignored by one of your .gitignore files:
docs
hint: Use -f if you really want to add them.

Full design decided in ADR-019 (amends ADR-015 §6). ADR-019 is already merged into docs/adr/ as part of this issue's groundwork — this issue tracks the remaining code implementation.

Background

discoverDocFiles() walks the filesystem directly and returns every in-scope doc file that exists, regardless of any gitignore source. runDocsGit's filterGitignoredDocFiles() was meant to drop paths that would make the subsequent git add -- <paths> hard-fail, using:

git ls-files --others --ignored --exclude-standard -z -- <docFiles>

--others matches only untracked files. Any doc file that is tracked but now lives under a path present in an active ignore source is invisible to this command, so the filter treats it as "safe to stage." git add -- <path> then hard-fails, which throws inside runDocsGit, and — per ADR-015 §6 — the docs stage swallows the error and the run continues, silently degrading every subsequent run until the misconfiguration is fixed.

Reproduced empirically (git 2.54.0): a tracked+modified file under a directory added to .git/info/exclude is invisible to git ls-files --others --ignored, but git add -- <path> on it still fails. This repo's own .git/info/exclude has /docs/ locally (unrelated to pi-loop's own .pi-loop/ rule), which triggers the bug on every run.

Documentation Required

  • ADR-019 — the accepted design for this fix (already committed to docs/adr/).
  • ADR-015 §6 — the non-fatal docs-stage guarantee being carved out (already annotated as amended).
  • ADR-009 — the existing CLI-level preflight pattern (assertPiLoopGitignore) this new preflight follows.
  • git check-ignore docs: https://git-scm.com/docs/git-check-ignore (exit code semantics: 0 = at least one path is ignored, 1 = none are ignored — not an error).

Implementation Details

1. New CLI-level fatal preflight (runCli.ts)

  • Add a new preflight check alongside the existing .pi-loop gitignore preflight (assertPiLoopGitignore), running immediately after loadConfig() resolves config.docs (currently ~line 227, before the gitignore preflight at ~line 260).
  • Skip entirely when config.docs.enabled === false or --no-docs is passed for the run.
  • Otherwise:
    1. Call discoverDocFiles({ repoRoot: cwd, paths: config.docs.paths, exclude: config.docs.exclude }) to get the exact file list the docs stage would later discover.
    2. Check every discovered file's ignore status via git check-ignore -v against all ignore sources (.gitignore, .git/info/exclude, global core.excludesFile) — batch all files into one invocation.
    3. Handle exit codes explicitly: exit 0 = some paths matched (ignored → fail); exit 1 = no paths matched (all clear → proceed, not a runner error). This differs from the ls-files-based convention used elsewhere in this codebase (non-zero exit = failure), so the seam wrapping this call needs its own invocation path that doesn't throw on exit 1.
    4. On failure: write to stderr a plain message listing every ignored file with full git check-ignore -v detail (ignore-source file, line number, pattern) — uncapped, no summarization — plus a one-line fix hint. return 1. No error.json (no run directory exists yet at this point, mirrors the existing .pi-loop gitignore preflight exactly).
  • Make the check injectable via a seam (mirroring checkPiLoopGitignoreFn in runCli.ts's deps) so it's unit-testable without spawning real git.

2. Remove filterGitignoredDocFiles (src/docs/services/runDocsGit.ts)

  • Delete filterGitignoredDocFiles() entirely.
  • Simplify the staging step to stage docFiles directly:
    if (docFiles.length > 0) {
      await runGit({ args: ['add', '--', ...docFiles], cwd });
    }
    
  • Update the JSDoc/comments in runDocsGit.ts that reference the old filter's purpose (the "gitignored paths are filtered out first" comment above the current git add step).
  • Add a code comment noting the deliberate tradeoff: runDocsGit now has zero defense against an ignored doc file reaching git add — correctness is guaranteed by the new CLI preflight being the single source of truth. Any future code path that invokes runDocsStage/runDocsGit without going through the CLI preflight would reintroduce the original crash with no safety net.

Acceptance Criteria

  • New CLI-level preflight exists in runCli.ts, runs after loadConfig(), before origin detection/issue fetch/any pipeline stage.
  • Preflight uses discoverDocFiles() + git check-ignore across all ignore sources (.gitignore, .git/info/exclude, global excludes) to check every discovered doc file.
  • Preflight is skipped entirely when config.docs.enabled === false or --no-docs is set.
  • On failure: plain stderr message with a full, uncapped per-file listing (git check-ignore -v detail per file) + fix hint; return 1; no error.json written.
  • git check-ignore exit code 1 (nothing ignored) is treated as success/proceed, not a runner error.
  • filterGitignoredDocFiles() is removed from src/docs/services/runDocsGit.ts; staging simplified to a direct git add -- <docFiles>.
  • AGENTS.md's ADR cross-reference list and M8 error-handling policy table are updated to reference ADR-019 and the new fail-fast preflight case (ADR-019 doc already merged; this issue covers the M8 table + any remaining AGENTS.md wiring notes).

Test Plan

  1. Unit test for the new preflight function using a fake ignore-checker/GitRunner seam:
    • No files ignored → preflight passes.
    • One or more files ignored → preflight fails with the full listing (every ignored file, with source/line/pattern detail).
    • git check-ignore exit code 1 (nothing ignored) is handled as a normal "clear" result, not a thrown runner error.
  2. runCli.test.ts: assert the CLI aborts with the expected stderr message and exit code 1 when the preflight fails.
  3. runCli.test.ts: assert the preflight is skipped when docs.enabled: false (YAML) and when --no-docs is passed.
  4. runDocsGit.test.ts: remove/replace the obsolete "skips gitignored doc paths when staging so git add never hard-fails" test with a test asserting the simplified direct-git add behavior (no filter step, no ls-files probe for ignored paths).

Validation is unit-tests only — no manual repro required (this repo's own .git/info/exclude misconfiguration is already diagnosed as the real-world trigger; the unit tests exercise the same logic deterministically).

## Summary The docs stage (ADR-015) fails non-fatally on **every** run when the resolved docs scope (`config.docs.paths`, default `README.md` + `docs/**/*.md`) is covered by a git ignore source (`.gitignore`, `.git/info/exclude`, or a global `core.excludesFile`). This has been happening silently in this repo for multiple runs (#267, #274, #282), always producing the same buried error: ``` [docs] WARN docs stage failed (non-fatal): Command failed: git add -- README.md docs/adr/001-...md ... The following paths are ignored by one of your .gitignore files: docs hint: Use -f if you really want to add them. ``` Full design decided in [ADR-019](docs/adr/019-fatal-docs-scope-ignore-preflight.md) (amends [ADR-015](docs/adr/015-documentation-sync-stage.md) §6). ADR-019 is already merged into `docs/adr/` as part of this issue's groundwork — this issue tracks the remaining **code** implementation. ## Background `discoverDocFiles()` walks the filesystem directly and returns every in-scope doc file that exists, regardless of any gitignore source. `runDocsGit`'s `filterGitignoredDocFiles()` was meant to drop paths that would make the subsequent `git add -- <paths>` hard-fail, using: ``` git ls-files --others --ignored --exclude-standard -z -- <docFiles> ``` `--others` matches only **untracked** files. Any doc file that is **tracked** but now lives under a path present in an active ignore source is invisible to this command, so the filter treats it as "safe to stage." `git add -- <path>` then hard-fails, which throws inside `runDocsGit`, and — per ADR-015 §6 — the docs stage swallows the error and the run continues, silently degrading every subsequent run until the misconfiguration is fixed. Reproduced empirically (git 2.54.0): a tracked+modified file under a directory added to `.git/info/exclude` is invisible to `git ls-files --others --ignored`, but `git add -- <path>` on it still fails. This repo's own `.git/info/exclude` has `/docs/` locally (unrelated to pi-loop's own `.pi-loop/` rule), which triggers the bug on every run. ## Documentation Required - [ADR-019](docs/adr/019-fatal-docs-scope-ignore-preflight.md) — the accepted design for this fix (already committed to `docs/adr/`). - [ADR-015](docs/adr/015-documentation-sync-stage.md) §6 — the non-fatal docs-stage guarantee being carved out (already annotated as amended). - [ADR-009](docs/adr/009-enforce-pi-loop-gitignore.md) — the existing CLI-level preflight pattern (`assertPiLoopGitignore`) this new preflight follows. - `git check-ignore` docs: https://git-scm.com/docs/git-check-ignore (exit code semantics: `0` = at least one path is ignored, `1` = none are ignored — **not** an error). ## Implementation Details ### 1. New CLI-level fatal preflight (`runCli.ts`) - Add a new preflight check alongside the existing `.pi-loop` gitignore preflight (`assertPiLoopGitignore`), running immediately after `loadConfig()` resolves `config.docs` (currently ~line 227, before the gitignore preflight at ~line 260). - **Skip entirely** when `config.docs.enabled === false` or `--no-docs` is passed for the run. - Otherwise: 1. Call `discoverDocFiles({ repoRoot: cwd, paths: config.docs.paths, exclude: config.docs.exclude })` to get the exact file list the docs stage would later discover. 2. Check every discovered file's ignore status via `git check-ignore -v` against **all** ignore sources (`.gitignore`, `.git/info/exclude`, global `core.excludesFile`) — batch all files into one invocation. 3. Handle exit codes explicitly: exit `0` = some paths matched (ignored → fail); exit `1` = no paths matched (all clear → proceed, **not** a runner error). This differs from the `ls-files`-based convention used elsewhere in this codebase (non-zero exit = failure), so the seam wrapping this call needs its own invocation path that doesn't throw on exit 1. 4. On failure: write to `stderr` a plain message listing **every** ignored file with full `git check-ignore -v` detail (ignore-source file, line number, pattern) — **uncapped, no summarization** — plus a one-line fix hint. `return 1`. **No `error.json`** (no run directory exists yet at this point, mirrors the existing `.pi-loop` gitignore preflight exactly). - Make the check injectable via a seam (mirroring `checkPiLoopGitignoreFn` in `runCli.ts`'s `deps`) so it's unit-testable without spawning real git. ### 2. Remove `filterGitignoredDocFiles` (`src/docs/services/runDocsGit.ts`) - Delete `filterGitignoredDocFiles()` entirely. - Simplify the staging step to stage `docFiles` directly: ```ts if (docFiles.length > 0) { await runGit({ args: ['add', '--', ...docFiles], cwd }); } ``` - Update the JSDoc/comments in `runDocsGit.ts` that reference the old filter's purpose (the "gitignored paths are filtered out first" comment above the current `git add` step). - Add a code comment noting the deliberate tradeoff: `runDocsGit` now has zero defense against an ignored doc file reaching `git add` — correctness is guaranteed by the new CLI preflight being the single source of truth. Any future code path that invokes `runDocsStage`/`runDocsGit` without going through the CLI preflight would reintroduce the original crash with no safety net. ## Acceptance Criteria - [ ] New CLI-level preflight exists in `runCli.ts`, runs after `loadConfig()`, before origin detection/issue fetch/any pipeline stage. - [ ] Preflight uses `discoverDocFiles()` + `git check-ignore` across all ignore sources (`.gitignore`, `.git/info/exclude`, global excludes) to check every discovered doc file. - [ ] Preflight is skipped entirely when `config.docs.enabled === false` or `--no-docs` is set. - [ ] On failure: plain `stderr` message with a full, uncapped per-file listing (`git check-ignore -v` detail per file) + fix hint; `return 1`; no `error.json` written. - [ ] `git check-ignore` exit code `1` (nothing ignored) is treated as success/proceed, not a runner error. - [ ] `filterGitignoredDocFiles()` is removed from `src/docs/services/runDocsGit.ts`; staging simplified to a direct `git add -- <docFiles>`. - [ ] `AGENTS.md`'s ADR cross-reference list and M8 error-handling policy table are updated to reference ADR-019 and the new fail-fast preflight case (ADR-019 doc already merged; this issue covers the M8 table + any remaining AGENTS.md wiring notes). ## Test Plan 1. **Unit test for the new preflight function** using a fake ignore-checker/`GitRunner` seam: - No files ignored → preflight passes. - One or more files ignored → preflight fails with the full listing (every ignored file, with source/line/pattern detail). - `git check-ignore` exit code `1` (nothing ignored) is handled as a normal "clear" result, not a thrown runner error. 2. **`runCli.test.ts`**: assert the CLI aborts with the expected `stderr` message and exit code `1` when the preflight fails. 3. **`runCli.test.ts`**: assert the preflight is skipped when `docs.enabled: false` (YAML) and when `--no-docs` is passed. 4. **`runDocsGit.test.ts`**: remove/replace the obsolete "skips gitignored doc paths when staging so git add never hard-fails" test with a test asserting the simplified direct-`git add` behavior (no filter step, no `ls-files` probe for ignored paths). Validation is unit-tests only — no manual repro required (this repo's own `.git/info/exclude` misconfiguration is already diagnosed as the real-world trigger; the unit tests exercise the same logic deterministically).
david closed this issue 2026-08-19 03:32:59 +00:00
Author
Owner

pi-loop opened and merged a pull request for this issue: #289

pi-loop opened and merged a pull request for this issue: https://git.excelera.net/david/pi-loop/pulls/289
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
david/pi-loop#286
No description provided.