Skip to content

Gate integrity: vacuous acceptance criteria, Exclusions enforcement never worked (regression of #66), owloop metadata committed into user repos #78

Description

@caoergou

Scope note (edited): originally filed as a monolithic post-mortem that duplicated parts of #72 / #73 / #76. Now narrowed to the three net-new findings from the same run that #75's children don't cover, plus a design addendum to the #37 roadmap. The duplicated material has been moved into comments on #76 (report root causes), #72 (token accounting), and #73 (spec-gen grounding). Parent: #75.

Evidence source: a real 5-spec owloop go refactor run (Claude Code adapter) that ended success with every verification gate passing — yet none of the three enforcement mechanisms below actually did their job.

Finding 1 — Vacuous acceptance criteria pass the gate silently (extends #64)

The spec generator itself emitted ACs like:

- [ ] `cd backend && ruff check app/api/issue.py 2>&1 | tail -1` → error count does not increase versus baseline (6)

Net effect: for 4 of the 5 specs, "verification gate passed (2 checks)" meant one real check (pytest exit code) plus one rubber stamp — indistinguishable from a real pass in logs and report. If success can't be trusted, the operator must re-review everything manually and the product's core promise is gone.

Fix:

  1. Gate: support → <substring> output assertions and a numeric-compare form, failing on mismatch.
  2. Spec lint: reject AC commands ending in exit-code-swallowing pipes (| tail, | head, || true, ...).
  3. Red/green AC classification (generalizes beyond refactor-style numeric baselines): each AC is declared either a goal (must be red before work starts, green after — a natural falsifiability check the engine can run at iteration 0) or an invariant (must stay green throughout). An AC that is green before any work and carries no assertion is flagged as vacuous.

Finding 2 — Exclusions enforcement (#66) never worked: template/parser contract mismatch

_restore_exclusions (verification.py:118, the #66 fix) exists and runs at the right time — and has never restored anything:

  • The spec template emits prose bullets: - Do NOT modify `backend/pyproject.toml` or `backend/uv.lock`
  • get_spec_exclusions (spec_queue.py:356) does item.strip().strip("")`, yielding the whole English sentence as a "pathspec".
  • git ls-files -- "Do NOT modify ..." matches zero files → the restore is a silent no-op.

Observed: iteration 1's commit contains a ±370-line uv.lock churn, violating the spec's own exclusion. #66 has been reopened with this evidence.

Fix: parser extracts all backtick-quoted tokens from a bullet (and/or lint enforces bare-path bullets); then upgrade from best-effort restore to a hard gate checkgit diff --name-only ∩ exclusions ≠ ∅ → fail the iteration or strip + warn. A blocklist that only restores is still trusted verification.

Finding 3 — owloop's own metadata gets committed into the user's repo

Iteration 1's commit also contains logs/events.jsonl — owloop's event log, written to the root of the user's project.

Chain: .owloop/ is untracked in the main repo → absent in a fresh worktree → resolve_owloop_dir (paths.py) falls back to the legacy root layout → logs land in <worktree>/logs/git add -A (engine.py:1127) commits them. Because log_dir is a @property re-resolved against the current cwd, the event stream also split across three files (3 events in the main repo, 1 in the worktree root, the rest in the worktree's .owloop/) — the direct cause of #76's 3-event timeline.

Note: fix/ignore-owloop-paths-in-dirty-check patches only the dirty check (engine.py:1164, :!.owloop); the commit path is unpatched, and the fallback logs/ isn't under .owloop anyway.

Fix:

  1. mkdir .owloop as the first step after worktree creation; drop the legacy fallback inside the engine.
  2. Resolve metadata paths once at engine construction as absolute paths.
  3. Commit with git add -A -- . ':!.owloop' (mirroring the dirty-check fix).

Design addendum — Phase 1.5 for the #37 roadmap: enforcement that can say "no"

Small, composable additions rather than a parallel pipeline:

  • Red/green AC classification (Finding 1, fix 3).
  • Scope policy for commits: default allowlist = ## Files ∪ new files under declared directories; out-of-scope diff → saved as a patch artifact + report entry, not silently committed. Policy-per-spec-type, so greenfield specs (which legitimately create unpredictable files) aren't hard-blocked.
  • Single run ledger: all run facts (events, summary, spec statuses, last_good..head commit range) written to one location in the main repo (.owloop/runs/<session>/), path resolved once; the report renders only from the ledger — eliminates the wrong-source bug class behind Generated HTML report is inaccurate and lacks AI summary of completed work #76.
  • MockAdapter e2e contract test: temp-repo run that deliberately (a) mutates an excluded file, (b) starts with a dirty main workspace, (c) contains one always-true AC. Assert: commit contents == allowlist; report matches worktree facts; the always-true AC is rejected; the event ledger is a single file. This is the regression net for the template↔parser and write-path↔read-path contract breaks that unit tests can't catch (Findings 1–3 are all of this class).

Known limits of this analysis

This run was happy-path only (0 failed iterations, no rollback/stall/parallel workers, single adapter). Not yet covered and worth separate design attention: failure-path report quality (what the operator sees after a 2 a.m. blocked/stalled stop), allowlist-commit × rollback-patch interaction, and token/budget normalization across ACP adapters.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions