Skip to content

feat(preflight): [ STACK WARNINGS ] — flag a check stack absent at the repo root (dogfood F2) - #89

Merged
tigers1997 merged 1 commit into
mainfrom
dogfood/f2-stack-reality-preflight
Jun 26, 2026
Merged

feat(preflight): [ STACK WARNINGS ] — flag a check stack absent at the repo root (dogfood F2)#89
tigers1997 merged 1 commit into
mainfrom
dogfood/f2-stack-reality-preflight

Conversation

@tigers1997

Copy link
Copy Markdown
Owner

What

F2 from the container dogfood. A default Node/pnpm intake on a project whose manifest lives in a subdir or behind Docker produces check commands (pnpm typecheck/lint/test) that stop-run-checks.sh silently skips (skip rule 3: no package.json at root) — the whole Stop-loop self-disables with zero feedback.

Change

New check_stack_reality() preflight + BINARY_TO_MANIFEST (mirrors manifest_for() in stop-run-checks.sh). It maps each configured check command's (typecheck/lint/test) first binary — through the shlex-guarded extract_first_binaries — to its stack manifest and warns when that manifest is missing at the repo root, naming the subdir for monorepos. Rendered as a [ STACK WARNINGS ] block alongside the existing preflights. Warning-only; silent when the stack matches root or no command maps to a known manifest.

Container note is host-PATH-gated (review advisory A1): it fires only when a warned binary is absent from the host PATH (toolchain genuinely in the container). A monorepo whose docker-compose.yml only backs services (host pnpm present) gets the actionable subdir warning but not a false "hooks will no-op" note.

Also hardens the pre-existing extract_first_binaries: shlex.split is now guarded against ValueError (an unbalanced quote in a cmd_* field used to crash the configurator with a traceback). check_stack_reality reuses it rather than duplicating the extraction loop (review advisory A2).

Tests

test/cc-manifest/test-stack-reality-preflight.sh — matched-silent, unmappable-silent, monorepo-names-subdir, container-note-when-toolchain-off-host, container-note-suppressed-when-on-host (via an injected which), absent-everywhere, install-only-silent, malformed-quote-safe. Plus a malformed-quote case in test-check-commands-extracted.sh. configure.py --check clean; 103/103 fixtures; 5/5 persona snapshots; module-combo smoke; dry-run repro of the [ STACK WARNINGS ] block.

Scope

Split out of #87 per CONTRIBUTING.md ("one logical change per PR") — this is the feat/minor half; F1 (SessionStart matcher) is #88. No tested_up_to / CC_VERSION bump.

🤖 Generated with Claude Code

…t the repo root (dogfood F2)

New check_stack_reality() preflight warns at configure time when a configured
check command's (typecheck/lint/test) stack manifest is missing at the repo
root — the exact condition that makes stop-run-checks.sh silently skip at
runtime (skip rule 3). Maps each command's first binary via the shlex-guarded
extract_first_binaries() to a stack manifest (new BINARY_TO_MANIFEST, mirroring
manifest_for() in stop-run-checks.sh); names the subdir for monorepos
(frontend/package.json). When a warned binary is also absent from the host PATH
and a compose/Dockerfile is present, it notes the host hooks will no-op because
the toolchain lives in the container — host-PATH-gated, so a monorepo whose
compose file only backs services doesn't get a false warning.

Also hardens the pre-existing extract_first_binaries: shlex.split is now
guarded against ValueError (an unbalanced quote in a cmd_* field used to crash
the configurator).

Tests: test/cc-manifest/test-stack-reality-preflight.sh (8 cases incl. the
host-PATH gate via an injected which) + a malformed-quote case in
test-check-commands-extracted.sh. --check clean, 103/103 fixtures, 5 persona
snapshots, module smoke.

Split from the originally-bundled PR #87 per CONTRIBUTING.md (one logical
change per PR); F1 (the SessionStart matcher fix) ships as #88. Incorporates
review advisories A1 (host-PATH-gated container note) + A2 (reuse
extract_first_binaries).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@claude

claude Bot commented Jun 26, 2026

Copy link
Copy Markdown

VERDICT: COMMENT-ONLY

PR adds check_stack_reality() — a warning-only preflight that maps each configured check command's first binary to its stack manifest (new BINARY_TO_MANIFEST) and warns when that manifest is absent from the project root. This is the exact condition that makes stop-run-checks.sh's skip rule 3 silently self-disable. Also hardens extract_first_binaries against ValueError from shlex.split. Tests are comprehensive (8 cases) and wired into CI under test/cc-manifest/. CHANGELOG entry present. No blocking issues found.

Blocking

None.

Advisory

A1 — shutil.which('./gradlew') always returns None; container note fires spuriously

configure.py, check_stack_reality (container-note block, ~line 1069)

shutil.which does not search PATH for names containing a path separator. When the name is ./gradlew, CPython falls back to os.access('./gradlew', mode) — a check against the process CWD, not target_dir. Two consequences:

  1. If cc-configure is invoked with --dir /project from /home/user, which('./gradlew') returns None regardless of whether the wrapper exists in target_dir.
  2. Even from the project root, ./gradlew is a project-local wrapper file, not a PATH binary. The which() heuristic ("binary absent from PATH implies container toolchain") is semantically wrong for it.

Effect: any project using ./gradlew test with a missing root build.gradle and any docker-compose.yml/Dockerfile receives the container note even when Gradle is installed locally. The primary stack warning ("no build.gradle at root") remains correct; only the appended container note is a false positive.

Suggested fix: exclude relative-path entries from the which() check:

path_bins = {b for b in warned_bins if '/' not in b}
if warnings and path_bins and any(which(b) is None for b in path_bins):

A2 — redundant import shutil inside function body

configure.py, check_stack_reality, ~line 1016

shutil is already imported at module top-level (line 22). The if which is None: import shutil block is a no-op. Simplify to:

if which is None:
    which = shutil.which

@tigers1997
tigers1997 merged commit c7ffb33 into main Jun 26, 2026
4 checks passed
@tigers1997
tigers1997 deleted the dogfood/f2-stack-reality-preflight branch June 26, 2026 22:29
tigers1997 added a commit that referenced this pull request Jun 26, 2026
…dant import; tighten migration test (review follow-ups) (#90)

Three non-blocking advisories from the split dogfood PRs #88/#89:

(1) check_stack_reality's container-note gate used shutil.which(b), which
always returns None for a path-like binary like ./gradlew (a project-local
wrapper, never on PATH) — so a Gradle-wrapper project with a Dockerfile and no
root build.gradle got a spurious "toolchain in container" note. The gate now
file-checks path-like binaries ((target_dir / b).exists()) and only consults
shutil.which for bare names.

(2) the inline `import shutil` inside check_stack_reality was redundant (shutil
is imported at module top) and is removed.

(3) test-sessionstart-matcher-migration.sh case 2 now asserts the marker-clear
migrates OUT of a user's mixed matcherless group (exactly once, under
startup|clear), not merely that the user hook survives — and the F1 CHANGELOG
bullet's stale "subset" description is corrected to the shipped placement-set
pre-pass. New ./gradlew case in test-stack-reality-preflight.sh.

No behavior change for non-path stacks. --check clean, 105/105 fixtures.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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.

1 participant