Skip to content

fix(pipeline): skip an unreadable subdirectory in the control-file walk instead of aborting the whole index - #2085

Open
AmirF194 wants to merge 1 commit into
DeusData:mainfrom
AmirF194:fix/2001-manifest-walk-skip-unreadable-subdir
Open

fix(pipeline): skip an unreadable subdirectory in the control-file walk instead of aborting the whole index#2085
AmirF194 wants to merge 1 commit into
DeusData:mainfrom
AmirF194:fix/2001-manifest-walk-skip-unreadable-subdir

Conversation

@AmirF194

@AmirF194 AmirF194 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Picking up from the thread on #2001: the abort happens after discovery finishes clean, in cbm_pipeline_build_semantic_manifest's own control-file walk.

semantic_manifest_walk_controls recursively walks the whole repo looking for .gitignore/.cbmignore/package-manifest files. When it hits the locked directory and cbm_opendir fails, it returns CBM_NOT_FOUND, which propagates all the way up and aborts the entire index. discover.c's walk_dir already treats the identical failure as "nothing to discover here" and moves on; the control walk had no reason to be stricter; a permission-denied subdirectory just has no control files to contribute.

Fix skips a nested directory it can't open instead of failing the whole walk. The root call still fails closed (depth 0), so pipeline_semantic_manifest_rejects_non_directory_root keeps passing unchanged.

Added pipeline_semantic_manifest_skips_unreadable_subdirectory: a locked sibling next to a readable one, asserts the walk succeeds and still finds the sibling's .gitignore. Needs a non-root run to mean anything, since root ignores the chmod and the check would be a no-op (same reasoning as test_cli.c's existing root-demotion comment); verified that way in a container both ways.

Not fixed here, and worth flagging so it doesn't get lost: #2020 is the separate issue about the generic error hint collapsing distinct failure codes. It's real and independent of this one; I only touched the abort itself.

Fixes #2001

…lk instead of aborting the whole index

semantic_manifest_walk_controls() returns CBM_NOT_FOUND the moment
cbm_opendir() fails on any subdirectory, and that propagates all the way up
through cbm_pipeline_build_semantic_manifest(), aborting the entire
index_repository run. discover.c's walk_dir() already treats the same
failure (a permission-denied directory it can't open) as "nothing to
discover here" and moves on. The control walk has no reason to be stricter:
it only looks for .gitignore/.cbmignore/package-manifest files, which a
locked-out directory simply doesn't contribute. Skip it and keep walking,
except at depth 0, where an inaccessible manifest root is still a real
error (pipeline_semantic_manifest_rejects_non_directory_root pins that).

Signed-off-by: Amir Fathi <amirfathi.me@gmail.com>
@AmirF194
AmirF194 requested a review from DeusData as a code owner September 7, 2026 08:01
@github-actions

github-actions Bot commented Sep 7, 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 added bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges stability/performance Server crashes, OOM, hangs, high CPU/memory priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Sep 9, 2026
@DeusData

DeusData commented Sep 9, 2026

Copy link
Copy Markdown
Owner

Thank you for the focused control-file walk change and the pipeline test. We need more time to review how an unreadable directory should affect indexing and exclusion handling before giving a decision.

The review queue is currently full, so detailed feedback may take a little time. We are working through it carefully and appreciate the work you have put into supporting the project. Thank you for your patience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. stability/performance Server crashes, OOM, hangs, high CPU/memory

Projects

None yet

Development

Successfully merging this pull request may close these issues.

index_repository: one unreadable directory aborts the whole run with a generic Pipeline failed, and the hint points elsewhere (0.10.8)

2 participants