Skip to content

feat(quality): enforce pre-commit at worker-worktree level — config exists but hook never installed (boiling-frog #2) #158

@hadamrd

Description

@hadamrd

Problem

Pre-commit gates (ruff/mypy/pyright/format) are silently bypassed end-to-end in the forge-loop sprint loop.

Two compounding bugs:

  1. forge-loop repo itself: .pre-commit-config.yaml exists (added in ci(quality): pre-commit + Taskfile + pyright + coverage floor (closes #90 infra) #106) but .git/hooks/pre-commit was never installed. pre-commit install was never run. The config file is decoration; nothing fires on commit.
  2. Worker worktrees: Even when the main repo HAS the hook installed (e.g. Titan), git worktree add /tmp/wt-loop-N creates a fresh .git/hooks/ directory in the worktree that is empty. Worker commits therefore skip every gate, silently.

Concrete example: a worker commits a file with an unused import + missing type hints + bad formatting on the Titan repo. Locally on the operator's main checkout, git commit would block on ruff. From the worker worktree, the same commit succeeds with zero output. The PR lands; CI catches it (if CI runs the same gates) or it ships broken.

CTO observation in the loop: "we don't have ruff at the pre-commit level?" — exactly. We thought we did. We didn't. The worker brief saying "Run your project's pre-commit gates" is a verbal request to an LLM, not an enforced gate.

Acceptance criteria

  • forge-loop init detects .pre-commit-config.yaml in the target repo and runs pre-commit install. If pre-commit binary is missing, prints actionable install hint and skips (non-fatal). Idempotent: re-running init does not error if hook already exists.
  • Init reports outcome in stdout/log: one of precommit_installed, precommit_already_present, precommit_skipped_no_config, precommit_skipped_no_binary.
  • Worker dispatch, after git worktree add, propagates the pre-commit hook into the worktree. Preferred approach: re-run pre-commit install inside the worktree (canonical) — fall back to copying .git/hooks/pre-commit from the main checkout only if pre-commit binary is unavailable. Verify against current git-worktree semantics (worktrees have their own .git/hooks/ dir).
  • A new typed event worker_precommit_installed is emitted by the worker dispatch path with fields {worktree_path, method: "install"|"copy"|"skipped", reason?}. Visible in the loop's existing event stream / audit log.
  • worker.md.tmpl brief is updated with an explicit rule: git commit MUST run without --no-verify. Any --no-verify use REQUIRES a justified reason in the PR body under a ## Pre-commit bypass justification heading.
  • Critic (audit-rabbit / manifesto-based critic) flags unjustified --no-verify in worker commits as sev1.
  • .forge/quality-manifesto.md gains the rule verbatim: "No --no-verify without a justified reason in the PR body. Pre-commit gates are not optional."
  • forge-loop init run against the forge-loop repo itself installs the hook (self-host: dogfood the fix).

Test matrix

Unit tests (tests/test_init_precommit.py):

  • init on repo WITH .pre-commit-config.yaml and pre-commit available → invokes pre-commit install, emits precommit_installed.
  • init on repo WITHOUT the config → emits precommit_skipped_no_config, exits 0.
  • init when pre-commit binary missing → emits precommit_skipped_no_binary with hint, exits 0 (non-fatal).
  • init re-run when hook already present → emits precommit_already_present, no error.

Unit tests (tests/test_worker_precommit_install.py):

  • _prep_worktree on a fresh worktree creates a working .git/hooks/pre-commit.
  • Event worker_precommit_installed is emitted with the correct method field.
  • Fallback path: simulate missing pre-commit binary → confirm copy fallback fires, event records method: "copy".

Integration test (tests/test_worker_precommit_e2e.py):

  • End-to-end: spin a worktree against a fixture repo containing a .pre-commit-config.yaml with a trivial failing hook (e.g. a hook that always exits 1). Plant a dirty file. Attempt git commit. Assert: commit FAILS with the hook's error output. This is the headline test — proves the gate actually fires inside the worktree.

Adversarial / sad-path:

  • Worker template + critic test: simulate a worker PR body that uses git commit --no-verify WITHOUT a ## Pre-commit bypass justification section → assert critic returns sev1 finding tagged precommit_bypass.
  • Worker PR body WITH a properly formatted justification section → assert critic does NOT flag (sev0 / clean).
  • Race: two workers dispatched against the same main repo concurrently both get working hooks (no shared-state corruption of .git/hooks/).

Out of scope

  • Do NOT add new pre-commit hooks to forge-loop's own .pre-commit-config.yaml (separate concern; tracked elsewhere if needed).
  • Do NOT modify the Titan repo or any target repo's .pre-commit-config.yaml.
  • Do NOT introduce a new dependency on pre-commit as a hard runtime requirement of forge-loop — keep it optional with graceful skip.
  • Do NOT rewrite the critic framework; only add the one --no-verify rule and its detection.
  • Do NOT touch CI workflows or GitHub Actions (CI-side enforcement is a separate ticket).
  • Do NOT change worktree creation strategy (still git worktree add, still under /tmp/wt-loop-*).
  • Do NOT alter the worker brief's other sections beyond adding the --no-verify rule.

File pointers

  • src/forge_loop/cli/init.py or src/forge_loop/cli.py — init command (investigate which file currently hosts init).
  • src/forge_loop/worker.py_prep_worktree (or equivalent worktree-prep function). Add hook-install step + event emission.
  • src/forge_loop/briefs/worker.md.tmpl — append --no-verify rule.
  • src/forge_loop/critic/ or wherever audit-rabbit / manifesto-based critic lives — add precommit_bypass rule (investigate exact module path).
  • .forge/quality-manifesto.md — append the new rule line.
  • src/forge_loop/events.py or equivalent typed-event registry — register worker_precommit_installed (investigate).
  • tests/test_init_precommit.py (new)
  • tests/test_worker_precommit_install.py (new)
  • tests/test_worker_precommit_e2e.py (new)

Worker note

AC is wide — touches init command, worker dispatch, brief template, critic rules, manifesto, event registry, plus 3 new test files across 6+ modules. You are at HIGH risk of running out of turns before pushing. Apply COMMIT DISCIPLINE aggressively from turn 1: wip-commit every 20 turns or every 5 file edits, whichever comes first. Suggested commit slicing:

  1. init auto-install + its unit tests → commit.
  2. Worker worktree hook propagation + event + unit tests → commit.
  3. Brief template + manifesto + critic rule + adversarial tests → commit.
  4. E2E integration test → commit.

Run the EXIT CHECKLIST even if implementation feels incomplete — a partial PR with 2/4 slices landed and clearly scoped is far more valuable than a dry-exit with nothing pushed.

Original report

Why

Two distinct gaps surface together:

  1. forge-loop's pre-commit config exists (.pre-commit-config.yaml from ci(quality): pre-commit + Taskfile + pyright + coverage floor (closes #90 infra) #106) but the hook is not installed in .git/hooks/pre-commit. pre-commit install was never run. The config is decoration.
  2. Even where it IS installed (Titan), worker worktrees under /tmp/wt-loop-* get their own empty .git/hooks/ directory, so worker commits silently skip the gate.

Combined effect: the operator's intent (ruff/mypy/pyright/format on every commit) is fully bypassed by every worker. The worker brief says "Run your project's pre-commit gates" — but that's a verbal request to an LLM, not an enforced gate.

CTO observation: "we don't have ruff at the pre-commit level?" — exactly. We thought we did. We didn't.

What

Three things:

  1. forge-loop init auto-installs the hook when .pre-commit-config.yaml is present in the target repo. Idempotent.
  2. Worker dispatch installs the hook into every fresh worktree before the worker runs. Audited via a new typed worker_precommit_installed event.
  3. Worker brief enforces git commit MUST be run without --no-verify unless an explicit, justified --no-verify reason is declared in the PR body. Critic flags unjustified --no-verify as sev1.

Metadata

Metadata

Assignees

No one assigned

    Labels

    loop:readyLoop runner will autonomously attempt this issuepo:expandedPO subagent expanded the issue bodypriority:p1Important, near-term

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions