Skip to content

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

Open
tanishqbaweja wants to merge 1 commit into
DeusData:mainfrom
tanishqbaweja:fix/manifest-walk-nul-file
Open

tanishqbaweja wants to merge 1 commit into
DeusData:mainfrom
tanishqbaweja:fix/manifest-walk-nul-file

Conversation

@tanishqbaweja

Copy link
Copy Markdown

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 <slatorgator3@gmail.com>
@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.

This branch has not been deployed

No deployments
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)

1 participant