Skip to content

fix(pipeline): skip a file named nul in the control-file walk instead of aborting the index (#2646) - #2903

Merged
DeusData merged 3 commits into
DeusData:mainfrom
tanishqbaweja:fix/manifest-walk-nul-file
Nov 4, 2026
Merged

DeusData merged 3 commits into
DeusData:mainfrom
tanishqbaweja:fix/manifest-walk-nul-file

Conversation

@tanishqbaweja

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #2646.

On Windows, one file named nul in a repository makes every index_repository run return aborted_previous_preserved, so a project without an index never gets one. Git Bash leaves such a file behind after cmd > nul or 2>nul, and listing it in .gitignore does not help.

The abort comes from the semantic manifest's control-file walk. semantic_manifest_control_entry() reads the metadata of every entry it lists, and one failed read fails the whole manifest. On Win32 the plain path <dir>\nul names the NUL device, so GetFileAttributesExW fails with error 87 on the real file. The manifest returns CBM_NOT_FOUND, and cbm_pipeline_run() turns that into CBM_PIPELINE_ABORT_PRESERVE_DB. Discovery is not affected: _wstat64 reports a character device, and the entry is skipped.

The walk records only control files and descends only into directories, so it never needs the metadata of an entry that is neither. The fix, in pipeline_incremental.c only, checks the control name first and skips a non-control entry that the listing already reports as a non-directory, without reading it. A Windows listing always carries the attributes; on POSIX an entry whose d_type is DT_UNKNOWN is still read. Directories and control-named entries are read exactly as before, so a failed read on one of those still fails the walk. find_alias_files() in path_alias.c already skips !is_dir entries before reading metadata.

The skip does not depend on the name. On this Windows 11 build con, aux, prn, com1, lpt1 and nul.txt reach the real file, so of the names I tried only nul is affected. The plain path of a name ending in a dot or a space does not reach the file either, and the same skip applies to it.

Tests. Two Windows-only tests in pipeline_semantic_manifest_repro, under #ifdef _WIN32 like the POSIX-only pipeline_semantic_manifest_skips_unreadable_subdirectory next to them. Each creates <tmp>\nul through \\?\, next to app.py and a .gitignore that lists nul, and deletes it before cleanup.

  • pipeline_semantic_manifest_skips_nul_device_file_issue2646: the manifest builds and records .gitignore. On main it fails with rc == -1.
  • pipeline_full_index_survives_nul_device_file_issue2646: cbm_pipeline_run() returns 0 and indexes the function in app.py. On main it fails with run_rc == -2.

Not changed. A directory named nul still fails the walk, like any directory whose metadata cannot be read. The response still does not name the failing path, which is the second ask in the issue. This follows the approach of SanHsien/codebase-memory-mcp@ed6ae148 (a downstream fork, never opened here), reimplemented on current main with the check split into two helpers and an added full-index test.

Verification. Windows 11 Home 23H2 (build 22631.6199), i7-10750H, 16 GB RAM, repo and TEMP on an exFAT volume. MSYS2 CLANG64 clang 23.1.4 with ASan+UBSan, built with the flags that make -n -f Makefile.cbm build/c/test-runner CC=clang CXX=clang++ prints.

  • pipeline_semantic_manifest_repro: with the fix, the only failures are the git-ownership test below and, in some runs, the two route checks below; on main the two new tests fail as well (108 passed, 3 failed in one run). The failure both share is environmental: git exits 128 in pipeline_git_context_change_forces_full_and_refreshes_branch ("detected dubious ownership"), because exFAT records no file owner. The route checks in incremental_env_edit_replaces_scoped_envvars and incremental_compose_edit_replaces_build_edges also flake on this machine with and without the change: over 11 repeated runs, at least one of them failed in 10 runs on main and in 8 runs with the fix.
  • pipeline: 447 passed, 2 failed, 1 skipped. Both failures fail the same way on main: in git_context_linked_worktree git also exits 128 right after git init, and pipeline_create_free gets F-some-path because a directory matching the test's fixed /some/path exists on this machine's test drive.
  • pipeline_frozen: 16 passed. index_resilience: 7 passed, 1 skipped (POSIX-only). incremental ran no tests here, because its fixture clone fails on this machine.
  • make -f Makefile.cbm lint-ci CLANG_FORMAT=<clang-format 20.1.8> stops at cppcheck 2.21.0. It reports 18 findings, all in 9 files this change does not touch, for example compareValueOutOfTypeRangeError where long is 32 bits on Windows. With the same flags on pipeline_incremental.c alone, cppcheck is clean. The other lint-ci targets (lint-format, lint-no-suppress, lint-memory-core) pass. CI runs cppcheck 2.20.0 on Linux.
  • make -f Makefile.cbm lint-tidy-diff (clang-tidy 23.1.4): no findings on the changed lines. scripts/security-audit.sh: passed.
  • Not run here: the Linux and macOS legs, and a full make -f Makefile.cbm test (on this machine the unrelated failures above happen either way).

Checklist

  • Every commit is signed off (git commit -s)
  • Tests pass locally (make -f Makefile.cbm test): not run in full; the suites listed above were.
  • Lint passes (make -f Makefile.cbm lint-ci): on this Windows machine only cppcheck fails, on untouched files; see above.
  • New behavior is covered by a test (reproduce-first for bug fixes)

Written with an AI coding agent.

… of aborting the index (DeusData#2646)

semantic_manifest_control_entry() reads the metadata of every entry the
control-file walk lists, and one failed read fails the whole manifest.
On Win32 the plain path <dir>\nul names the NUL device, so a real file
named nul (Git Bash leaves one behind after `cmd > nul` or `2>nul`)
cannot be read: GetFileAttributesExW fails with
ERROR_INVALID_PARAMETER. The manifest returns CBM_NOT_FOUND and every
index run on the repository ends in CBM_PIPELINE_ABORT_PRESERVE_DB
(aborted_previous_preserved). Listing nul in .gitignore does not help,
because the walk visits every entry.

Only directories and control files can contribute to the manifest.
Decide the control name first and skip an entry that the listing
already shows to be neither, without reading it. A Windows listing
always carries the attributes; on POSIX an entry whose d_type is
DT_UNKNOWN is still read. Directories and control-named entries are
read exactly as before.

Signed-off-by: Tanishq <[email protected]>
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

@DeusData

DeusData commented Nov 3, 2026

Copy link
Copy Markdown
Owner

Thank you very much for this, @tanishqbaweja, and welcome! This is a careful fix at exactly the right layer: an entry the listing already shows to be neither a directory nor a control file can never contribute to the manifest, so skipping it is safe, while directories and control files are still read and still fail closed. Your two Windows tests ran on the Windows CI leg and pass.

CI was green, but another change landed in tests/test_pipeline.c on main in the meantime, so we merged current main into your branch (a plain signed-off merge commit; your change is unchanged) to get a fresh run on the exact tree that will merge. Nothing is needed from you - it merges as soon as that run is green. If you push again, please pull first so the merge commit is kept.

@DeusData
DeusData merged commit 55206a8 into DeusData:main Nov 4, 2026
26 checks passed
@DeusData

DeusData commented Nov 4, 2026

Copy link
Copy Markdown
Owner

Merged -- thank you so much for this, @tanishqbaweja, and welcome! This is a really careful first contribution, and it unblocks indexing for every Windows user who has a stray nul file in a repository.

What made it a pleasure to review:

  • You fixed the cause, not the symptom. The control-file walk read the metadata of every entry it listed, and on Windows the plain path <dir>\nul names the NUL device, so one leftover file failed the whole manifest. Deciding the control name first and skipping an entry that the listing already shows to be neither a directory nor a control file is the right fix at the right layer: such an entry can never contribute to the manifest, while directories and control files are still read, and still fail closed, exactly as before.
  • The POSIX side is handled with care: an entry whose d_type is DT_UNKNOWN still gets its metadata read, so filesystems that do not report entry types behave as before.
  • Both tests are reproduce-first and cover the user-visible symptom, not only the helper: the manifest builds with .gitignore recorded, and a full cbm_pipeline_run() indexes app.py instead of aborting. Creating the file through \\?\ and deleting it before cleanup keeps the temp tree tidy. We confirmed both tests ran and passed on the Windows CI leg.
  • The write-up is excellent: the exact error, the reserved names you tried on Windows 11 and which of them actually reach the device, what is deliberately left unchanged (a directory named nul, the diagnostic), and an honest account of the local failures you saw and why they are unrelated.

One note on the issue: this fixes the abort, which was the main ask in #2646. The second ask there -- naming the path that could not be read instead of the generic "files changed / transient" hint -- is still worth doing, and we will keep it on our list separately.

Thanks again for the thorough work, and for crediting the earlier fork approach so clearly.

ngallodev added a commit to ngallodev/codebase-memory-cli that referenced this pull request Nov 9, 2026
…ification

Adds per-batch receipts (13m1.md, 13m2.md, 13n.md) and updates ledger.json,
PLAN.md and CONTINUE.md for the 2026-11-08 upstream closeout against
DeusData 612eee2.

Jenkins release-tooling full gate, exact heads:
- 13M-1 c3c5271: DeusData#90 SUCCESS; merged to main via PR #33 (929adfe).
  DeusData#89 on 3cbefe2 failed only on the index_policy fixture fixed there.
- 13M-2 0f1927f: DeusData#93 SUCCESS. DeusData#91 on 1c5f82a failed two upstream
  Hermes hook cycle tests, fixed in 0f1927f.
- 13N 98eab3c: DeusData#94 SUCCESS. DeusData#92 on cf24b6a was aborted because that
  head lacked the 13M-2 test fix.

Native Windows was not run for any batch, so the reserved-`nul` change
(DeusData#2903) is unverified on Windows. This commit only touches docs; the code
Jenkins qualified is 98eab3c.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows: a file named nul in the repo makes every index_repository abort with aborted_previous_preserved (0.11.0)

2 participants