Skip to content

fix(skills): keep unreadable directories non-fatal - #569

Merged
andrei-hasna merged 2 commits into
mainfrom
fix/68a25c6e-skill-loader-photos
Aug 12, 2026
Merged

fix(skills): keep unreadable directories non-fatal#569
andrei-hasna merged 2 commits into
mainfrom
fix/68a25c6e-skill-loader-photos

Conversation

@andrei-hasna

@andrei-hasna andrei-hasna commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Why

Skill discovery already skips directory entries it cannot read, but it logged those recoverable failures at ERROR. Durable background supervision treated that event as the attempt failure, so an unrelated macOS Photos library exhausted all retries even though it was not a skill document.

Todos task: 68a25c6e-7164-453e-921c-b3989ec2e3de.

What changed

  • Log recoverable skill-directory read failures at WARN while preserving fail-open traversal of the configured roots.
  • Add a Unix regression fixture with a valid skill beside an unreadable unrelated directory; assert the skill loads and the diagnostic is WARN, never ERROR.
  • Record the existing tracing-test workspace package in the codex-core-skills lock entry.

Test plan

  • PASS — Blacksmith Testbox run 31543364023 on candidate 65964a54feebf022dc38d536d62b24e3439d818c: cd codex-rs && just test-fast -p codex-core-skills returned 0; 112 tests passed, 0 skipped.
  • Local just fmt-check, cargo metadata --locked --no-deps, git diff --check, staged secrets scan, and branch-diff secrets scan returned 0. No local Rust build or test was run under the repository remote-build rule.

Rollback

Revert this pull request; this restores the prior ERROR diagnostic and removes the focused regression.

Downgrade recoverable skill discovery read failures so background supervision does not treat them as terminal run errors. Add a regression that keeps loading valid configured skills beside an unreadable unrelated directory.

Agent: Marcellinus
Record the existing tracing-test workspace package in codex-core-skills so remote cargo fetch --locked can run the regression.

Agent: Marcellinus
@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] GO — #569 @ 65964a5 — lens: correctness+security+gates, reviewer Hostus (1 of 1)

Exact candidate read:

  • git rev-parse HEAD origin/main — exit 0; HEAD 65964a54feebf022dc38d536d62b24e3439d818c, base f290938c0b0492c369a51a8e4c4cff43e2a1d1ed.
  • git log --oneline origin/main..HEAD — exit 0; 2 commits read.
  • git diff origin/main...HEAD --stat — exit 0; 4 files, 69 insertions, 1 deletion.
  • Read the complete diff plus surrounding loader traversal, symlink handling, test helpers, the full new regression, the crate manifest, and the Cargo lock entry.

Commands and gates:

  • bun install — exit 0; setup only, not a test gate; 521 packages installed; pass/fail counts not applicable. It generated an untracked bun.lock, which was removed so the reviewed worktree returned to the exact candidate.
  • Root package.json declares no test script. Therefore this repository declares no root test gate; test pass/fail count: 0/0 because no such gate exists.
  • Root package.json declares no typecheck script. No typecheck gate was invented.
  • No lane-chosen command was run.
  • gh pr view 569 --repo hasna/codewith --json statusCheckRollup — exit 0; authoritative forge result: 25 SUCCESS, 0 FAILURE, 3 SKIPPED, 0 pending.

Forge checks read, by name and conclusion:

  • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-gnu — SUCCESS
  • cla — SUCCESS
  • Check for spelling errors — SUCCESS
  • Blob size policy — SUCCESS
  • cargo-deny — SUCCESS
  • build-test — SUCCESS
  • Build Codewith Linux CLI — SUCCESS
  • Detect changed areas — SUCCESS
  • python-sdk — SUCCESS
  • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-musl — SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 1/4 — SUCCESS
  • Format / etc — SUCCESS
  • sdks — SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 2/4 — SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 3/4 — SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 4/4 — SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm (native main) — SKIPPED
  • cargo shear — SUCCESS
  • Argument comment lint package — SKIPPED
  • Bazel clippy on ubuntu-24.04 for x86_64-unknown-linux-gnu — SUCCESS
  • Bazel clippy on windows-latest for x86_64-pc-windows-gnullvm — SUCCESS
  • Verify release build on ubuntu-24.04 for x86_64-unknown-linux-gnu — SUCCESS
  • Argument comment lint - Linux — SUCCESS
  • Verify release build on windows-latest for x86_64-pc-windows-gnullvm — SUCCESS
  • Argument comment lint - Windows — SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm — SUCCESS
  • CI results (required) — SUCCESS
  • [code]smith — SKIPPED

Blocking P0/P1 findings: none. The change preserves skill discovery behavior and the existing diagnostic while downgrading only a recoverable descendant-directory read from ERROR to WARN. It does not broaden filesystem access, follow additional paths, suppress the warning, alter sandbox selection, or change session/auth data handling.

Non-blocking follow-ups:

  • P2 repository-gate gap: the root package declares no test script. The current authoritative forge matrix did exercise the affected Rust/Bazel lanes successfully, so this does not block this exact candidate.
  • No root typecheck script is declared; this is recorded as an absent gate, not treated as a failure.

@andrei-hasna
andrei-hasna merged commit 0abe3a6 into main Aug 12, 2026
29 checks passed
@andrei-hasna
andrei-hasna deleted the fix/68a25c6e-skill-loader-photos branch August 12, 2026 01:05
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 12, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant