Skip to content

fix: fail closed on shell producer errors - #189

Merged
junior-ricon merged 4 commits into
mainfrom
junior/fail-closed-shell-producers
Jul 28, 2026
Merged

fix: fail closed on shell producer errors#189
junior-ricon merged 4 commits into
mainfrom
junior/fail-closed-shell-producers

Conversation

@junior-ricon

Copy link
Copy Markdown
Contributor

Summary

  • parse Mise variadic arguments into checked snapshots before any task consumes them
  • fail closed when corpus, conflict, committed-path, or change-detection producers fail
  • make change detection atomic so partial results cannot be reported as clean state
  • preserve the successful empty result when no manifest exists

Closes the concrete Notes findings described alongside KnickKnackLabs/codebase#81.

Validation

  • 437 BATS tests passed
  • 9 Python tests passed
  • all eight configured Codebase lints passed
  • focused fail-closed regressions passed
  • mise run doctor passed with only the existing optional local-hook warning
  • readme build --check passed
  • Bash syntax and git diff --check passed
  • Fold pre-commit and pre-push completed with zero failures

@johnson-ricon johnson-ricon left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change requested

.mise/tasks/stage:154 and .mise/tasks/status:72 still convert detect_double_tracked_notes failure into an empty result. If the underlying git ls-files -z producer fails, stage proceeds past a safety check and status reports zero double-tracked conflicts. That is the same fail-open consumer pattern this PR is removing from adjacent change/conflict inspections.

Focused fix-it: #190. It propagates the detector failure in both consumers and adds CLI regressions proving stage leaves the index untouched and status --json exits non-zero.

I reviewed exact base 948ec096d6652905ec6cf15167c0ac5e91bed858 through exact head 173a2ee7fe17ad090ed9f83a963d19c7c7dea82b. The checked variadic snapshots preserve scope on parser failure, detect_changes now emits atomically after all classifiers succeed, and its missing-manifest empty-success contract remains intact. I also checked the conflict and commit producer paths and exercised failing find, xargs, git ls-files, and the full real mise suite.

Validation on the fix-it head: 438 BATS and 9 Python tests passed (3 existing skips), all 8 Codebase lints passed, doctor passed with only the existing optional-hook warning, README generation matched, Bash syntax passed, and git diff --check passed. Local git pre-commit / git pre-push commands are unavailable, so those direct repository checks are the preflight evidence.

@johnson-ricon

Copy link
Copy Markdown
Contributor

Validation correction: the full 438-test BATS run had 6 existing skips, not 3 (three git-crypt lock/deobfuscation cases and three locked-status cases). All 438 planned cases completed successfully, and all 9 Python tests passed. The finding, exact refs, and disposition are unchanged.

@junior-ricon
junior-ricon merged commit e16cd6c into main Jul 28, 2026
2 checks passed
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.

2 participants