Skip to content

fix: container/monorepo dogfood hardening — SessionStart freeze-loss (F1) + [ STACK WARNINGS ] preflight (F2) - #87

Closed
tigers1997 wants to merge 3 commits into
mainfrom
dogfood/container-stack-hardening
Closed

fix: container/monorepo dogfood hardening — SessionStart freeze-loss (F1) + [ STACK WARNINGS ] preflight (F2)#87
tigers1997 wants to merge 3 commits into
mainfrom
dogfood/container-stack-hardening

Conversation

@tigers1997

Copy link
Copy Markdown
Owner

Why

Dogfooding cc-configure into a containerized project (Python + FastAPI backend + Vite frontend, both via Docker; no root package.json) surfaced two issues. The local review verdict was "safe to commit" — true — but the configurator silently under-delivered, and one shipped hook had a latent freeze-loss bug.

F1 — fix(commands): SessionStart marker-clear freeze-loss

The microbit-enforcer SessionStart marker-clear shipped with no matcher, so it fired on every SessionStart source (startup, resume, clear, compact). That silently wiped .claude/.frozen / .guarded / .careful on --resume and after compaction — i.e. mid-session, exactly when a long session most needs a /freeze to hold. Now "matcher": "startup|clear" (source set confirmed against code.claude.com/docs/en/hooks-guide): markers clear only on a brand-new session or /clear, and persist across resume/compaction.

F2 — feat(preflight): [ STACK WARNINGS ]

A default Node/pnpm intake on a containerized/monorepo project 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. New check_stack_reality() (+ BINARY_TO_MANIFEST, mirroring manifest_for() in stop-run-checks.sh) warns at configure time when a configured check command's manifest is missing at root, names the subdir for monorepos (frontend/package.json), and notes when a docker-compose.yml/Dockerfile means the host-side hooks will no-op. Warning-only; never blocks; silent when the stack matches root or no command maps to a known manifest (bare tsc/pytest/ruff).

Verification

  • configure.py --check ✓ · 104/104 fixtures ✓ · 5/5 persona snapshots ✓ · module-combo smoke ✓
  • Reproduced the dogfood scenario end-to-end: containerized layout (frontend/package.json + docker-compose.yml, no root manifest) → both warnings render; silent with a root package.json.

Scope

No tested_up_to / CC_VERSION bump (no-lone-bumps). F3 — container-aware execution (point check/format commands at docker compose exec <svc>) — is intentionally deferred as a separate design item.

Files

  • templates/commands/microbit-enforcer/settings-patch.json — matcher + comment
  • configure.pyBINARY_TO_MANIFEST, check_stack_reality(), [ STACK WARNINGS ] render
  • templates/core/CLAUDE.md, examples/python-uv-fastapi/{CLAUDE.md,.claude/settings.json} — doc/example sync
  • test/microbit-enforcer/test-sessionstart-matcher.sh, test/cc-manifest/test-stack-reality-preflight.sh
  • CHANGELOG.md

🤖 Generated with Claude Code

…(F1) + [ STACK WARNINGS ] preflight (F2)

Surfaced by dogfooding cc-configure into a containerized project
(Python + FastAPI backend + Vite frontend, both via Docker; no root
package.json).

F1 (fix, commands): the microbit-enforcer SessionStart marker-clear shipped
with no matcher, so it fired on every SessionStart source — including
resume and compact — silently deleting .claude/.frozen/.guarded/.careful
mid-session, exactly when a long session most needs the freeze to hold.
Now scoped to "matcher": "startup|clear" so markers clear only on a fresh
session or /clear and persist across --resume and compaction. Sources
verified against code.claude.com/docs/en/hooks-guide.

F2 (feat, preflight): new check_stack_reality() + BINARY_TO_MANIFEST emit a
[ STACK WARNINGS ] block when a configured check command's stack manifest
is missing at the repo root — the exact condition that makes
stop-run-checks.sh silently skip (skip rule 3). Names the subdir for
monorepos and flags containerized layouts. Warning-only; silent when the
stack matches root or no command maps to a known manifest.

Tests: new test/microbit-enforcer/test-sessionstart-matcher.sh and
test/cc-manifest/test-stack-reality-preflight.sh. Full suite green
(104 fixtures, 5 persona snapshots, module-combo smoke, --check).
No tested_up_to / version bump (no-lone-bumps).

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

claude Bot commented Jun 26, 2026

Copy link
Copy Markdown

VERDICT: BLOCK

PR adds the `startup|clear` matcher to the microbit-enforcer SessionStart hook (F1) and a new `check_stack_reality()` preflight (F2). The matcher fix is correct and the preflight design is sound, but two blocking issues need to land before merge.

Blocking

B1 — `shlex.split` unguarded in `check_stack_reality` — crashes the configurator on any cmd value with an unclosed quote

`configure.py:1015` — `shlex.split(val)` is called bare inside the loop over all six `cmd_*` form fields. `shlex.split` raises `ValueError: No closing quotation` on input like `pnpm test --grep 'foo` (a common copy-paste mistake). There is no `try/except` around the call and no outer catch in `main()` at the call site (configure.py:2710). The `except OSError` at line 1028 covers only `iterdir()`. One bad form field turns a preflight warning into a full configurator crash with a raw traceback.

Fix: `try: parts = shlex.split(val)\nexcept ValueError: parts = val.strip().split()` (or `continue` on bad input). The pre-existing `extract_first_binaries()` (~line 984) has the same gap; worth fixing in the same pass.

B2 — Four user-facing files still describe the old (now-wrong) marker lifecycle

The PR's core fix is that freeze/guard/careful markers survive `--resume` and compaction, clearing only on `startup|clear`. `templates/core/CLAUDE.md` was correctly softened to "transient session state" — but four other shipped files were not updated and now contradict the behavior this PR introduces:

  • `templates/commands/freeze/SKILL.md:37-39` — "Frozen state does not persist across Claude Code session restarts: the `SessionStart` hook clears all microbit markers." This is now the opposite of the truth.
  • `templates/commands/guard/SKILL.md:41-43` — "The configurator clears `.guarded` on session start (SessionStart hook)."
  • `templates/commands/careful/SKILL.md:45` — "Careful patterns persist for the session, cleared on session start."
  • `templates/commands/microbit-enforcer/microbit-enforcer.sh:14-16` — "a SessionStart hook clears all three files. Markers are session-scoped, not persistent."

A user reading `freeze/SKILL.md` will expect their freeze to be gone after a resume; the fix makes it survive. The model loading these skill files mid-session will reason about the wrong lifecycle. These are shipped templates, so the stale text reaches every project that installs the microbit commands.

Advisory

A1 — `check_stack_reality` warning text blames `stop-run-checks.sh` for commands it never runs

`configure.py:1012-1013` iterates all six `cmd_*` keys including `cmd_install`, `cmd_build`, `cmd_dev`. When any of those triggers a manifest-missing warning, lines 1044-1047 tell the user "stop-run-checks.sh runs from the root and will silently skip these" — but `stop-run-checks.sh:51-55` only contains the three check commands (typecheck/lint/test). A user with `pip install -r requirements.txt` in `cmd_install` and no `pyproject.toml` gets a warning blaming the Stop hook, which would never run `pip` at all. The dim footer at `configure.py:2716` repeats the false attribution. Recommend either narrowing the scan to the three stop-hook keys or adjusting the warning text (and footer) to name the correct hooks that actually run `cmd_install`/`cmd_build`/`cmd_dev`.

… (B2), stack-check scope (A1)

B1: extract_first_binaries + check_stack_reality wrapped shlex.split in
try/except ValueError. An unbalanced quote in a cmd_* field (e.g.
`pnpm test --grep 'foo`) raised ValueError uncaught and crashed the
configurator with a raw traceback; now degrades to a plain split.

B2: freeze/guard/careful SKILL.md and the microbit-enforcer.sh header
described the old "cleared on every SessionStart" lifecycle, which the F1
matcher fix makes wrong; corrected to "cleared on startup//clear,
preserved across --resume and compaction."

A1: check_stack_reality now scans only the three commands stop-run-checks.sh
actually runs (typecheck/lint/test), not install/build/dev — so the warning
text no longer misattributes an install/build/dev skip to the Stop hook.

Tests: + install-only-silent and malformed-quote-safe cases in
test-stack-reality-preflight.sh; + a malformed-quote case in
test-check-commands-extracted.sh. Full sweep 104/104, --check clean,
5/5 persona snapshots, dry-run repro intact.

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 two well-contained dogfood fixes: F1 scopes the microbit-enforcer SessionStart hook to startup|clear so freeze/guard/careful markers survive --resume and compaction; F2 adds a [ STACK WARNINGS ] preflight that catches a check-command stack whose manifest lives in a subdir or container rather than the repo root. Both fixes include targeted tests and the CHANGELOG entries are present and correct.

Blocking

None.

Advisory

examples/python-uv-fastapi/.claude/skills/{freeze,guard,careful}/SKILL.md — stale lifecycle language contradicts the F1 fix

The three template files were updated with the new nuance (markers survive --resume/compaction, cleared only on startup or /clear), but the corresponding generated copies inside examples/python-uv-fastapi/ were not regenerated. They still carry the old text:

  • examples/python-uv-fastapi/.claude/skills/freeze/SKILL.md: "Frozen state does not persist across Claude Code session restarts: the SessionStart hook clears all microbit markers (.frozen / .guarded / .careful)."
  • examples/python-uv-fastapi/.claude/skills/guard/SKILL.md: "The configurator clears .guarded on session start (SessionStart hook)." — no resume/compact nuance.
  • examples/python-uv-fastapi/.claude/skills/careful/SKILL.md: "Careful patterns persist for the session, cleared on session start." — ditto.

examples/python-uv-fastapi/EXAMPLE_README.md documents that the example is regenerated by running python3 configure.py --config examples/python-uv-fastapi/.claude-config.json --dir examples/python-uv-fastapi. Running that and committing the three skill-file diffs would fix this in one shot.

…review A2)

The freeze/guard/careful SKILL.md copies and the microbit-enforcer.sh
header under examples/python-uv-fastapi/.claude/ still carried the old
"cleared on every SessionStart" lifecycle text; corrected to match the
template fix (cleared on startup/clear, preserved across --resume and
compaction). Example-mirror doc sync; no code change.

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

claude Bot commented Jun 26, 2026

Copy link
Copy Markdown

VERDICT: BLOCK

@claude

claude Bot commented Jun 26, 2026

Copy link
Copy Markdown

VERDICT: BLOCK

Blocking

B1 — Scope creep: two independent changes in one PR

CONTRIBUTING.md:43 states: "One logical change per PR. Reviewers (human and AI) struggle with mixed-purpose PRs; merge friction goes up; rollback gets messy."

This PR bundles:

  • F1 — a fix (patch-tier): adds "matcher": "startup|clear" to templates/commands/microbit-enforcer/settings-patch.json + doc updates. Runtime hook behavior correction; touches only JSON + markdown.
  • F2 — a feat (minor-tier): adds BINARY_TO_MANIFEST + check_stack_reality() (~90 lines of Python) + new test files to configure.py. New configure-time preflight warning; touches Python, tests, CI-wired test fixtures.

No files are shared between F1 and F2. They have different Conventional Commit types, different semver implications, and independent rollback surfaces. Recommend splitting into two PRs.


Advisory

A1 — Container note fires false-positive for monorepo + incidental compose file

configure.py:1062 — The container note is appended whenever if warnings: (any manifest-mismatch warning) AND a compose/Dockerfile is present at root. This misfires for a common layout: pnpm configured with frontend/package.json (monorepo subdir) and a docker-compose.yml present only for backing services (Postgres, Redis, etc.) — the host-side toolchain works perfectly fine. The user gets warning #1 ("found in frontend/, cd frontend && ...", which is actionable) plus warning #2 ("hooks will no-op — point cmd_* at docker compose exec"), which is incorrect: the hooks will run because pnpm is on the host.

The container note is appropriate only when the manifest is absent everywhere (the else branch at ~line 1056). Restrict it:

  • Track whether any "absent everywhere" case was hit (a boolean flag in the loop).
  • Gate the container note on that flag rather than if warnings:.

A2 — check_stack_reality() duplicates extract_first_binaries() extraction loop

configure.py:1016–1025 — The 10-line bins = set() loop in check_stack_reality() is a reinvention of extract_first_binaries(). The two functions share the same key set (cmd_typecheck, cmd_lint, cmd_test) and the same shlex-fallback logic. If a new key is added to extract_first_binaries() in the future, check_stack_reality() silently misses it. Suggest replacing the loop with a call to extract_first_binaries() and taking .values(). This also eliminates the redundant import shlex inside check_stack_reality().

@tigers1997

Copy link
Copy Markdown
Owner Author

Superseded by a split per CONTRIBUTING.md ("one logical change per PR") — the round-3 review correctly flagged this as a mixed fix+feat bundle.

Closing this one.

@tigers1997 tigers1997 closed this Jun 26, 2026
@tigers1997
tigers1997 deleted the dogfood/container-stack-hardening branch June 26, 2026 21:33
tigers1997 added a commit that referenced this pull request Jun 26, 2026
…t the repo root (dogfood F2) (#89)

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>
tigers1997 added a commit that referenced this pull request Jun 26, 2026
…tartup|clear (dogfood F1) (#88)

* fix(commands): scope microbit-enforcer SessionStart marker-clear to startup|clear (dogfood F1)

The SessionStart hook that clears the freeze/guard/careful markers shipped
with no matcher, firing on every source — including resume and compact — so
a /freeze was silently lost mid-session after compaction or --resume. Now
"matcher": "startup|clear": markers clear only on a fresh session or /clear
and survive --resume and compaction.

Sources verified against code.claude.com/docs/en/hooks-guide (startup,
resume, clear, compact; matcher filters on source). The freeze/guard/careful
SKILL.md notes, the microbit-enforcer.sh header, and the
templates/core/CLAUDE.md gitignore note are corrected to match, across both
the templates and the python-uv-fastapi example mirror. New
test/microbit-enforcer/test-sessionstart-matcher.sh.

Split from the originally-bundled PR #87 per CONTRIBUTING.md (one logical
change per PR); F2 (stack-reality preflight) ships as a separate PR.

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

* fix(retrofit): migrate the matcherless SessionStart marker-clear group on retrofit (PR #88 review)

Existing installs carry a matcherless SessionStart marker-clear group;
_merge_hook_groups keys by matcher, so cc-configure --retrofit would append
the new startup|clear group beside the old one and the stale matcherless group
kept wiping markers on resume/compact — re-negating the freeze-loss fix on
every upgrade (the same class as PR #73's standalone->bundled bug, for a
matcher change).

_merge_hook_groups now does a pre-pass: for each configurator command it
collects the matcher(s) the new template places it under, then strips that
command from any existing group whose matcher is not one of them (a stale
placement). This preserves the deliberate same-command-under-multiple-matchers
pattern (the mcp drift-check ships under both startup and resume — the
no-duplication / self-heal tests still expect 4 SessionStart groups) and never
touches a user's own command.

New test/retrofit-hooks/test-sessionstart-matcher-migration.sh. Also aligns
test-sessionstart-matcher.sh to the heredoc / sys.path '.' style used by the
merge tests (review advisory). --check clean, 104/104 fixtures.

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

---------

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