feat(manifestos): critic compliance check — manifesto violations gate auto-merge

[hadamrd]

Problem

The critic agent currently reviews PRs for surface-level correctness (build, tests, obvious bugs) but has no awareness of the project's manifestos — the canonical quality/testing rules in docs/manifestos/ (or equivalent). As a result, PRs that compile and pass tests can still merge while visibly violating rules like "no print debugging left in committed code", "tests must assert behaviour, not mock returns", or "no silent except Exception: pass". Auto-merge then ships drift.

Concrete example: a worker PR adds a try/except Exception: pass around an HTTP call, tests pass (because the exception path is never exercised), critic returns approve, and the PR auto-merges — directly violating the error-handling manifesto.

Acceptance criteria

• _critic_sdk.py loads every manifesto file under the canonical manifestos directory (investigate exact path — likely docs/manifestos/*.md or src/forge_loop/manifestos/) and renders their full text into the critic prompt via briefs/critic.md.tmpl.
• CriticReport in src/forge_loop/critic.py gains a manifesto_violations: list[ManifestoViolation] field. ManifestoViolation is a typed dataclass/TypedDict with at minimum: rule_id: str, manifesto: str, quote: str (the offending snippet from the PR diff), suggested_fix: str, severity: Literal["sev1","sev2","sev3"].
• The critic prompt template instructs the model to emit manifesto_violations as JSON; the parser in _coerce_report populates the new field (defaulting to [] when absent for back-compat).
• Any manifesto_violations entry with severity == "sev1" forces CriticReport.overall == "request_changes" and blocks auto-merge (gate lives wherever auto-merge consults the critic verdict — investigate critic_actions.py / merge gate).
• sev2/sev3 violations are reported in the PR comment but do NOT block merge.
• Back-compat: existing critic JSON without the new field still parses successfully.
• All new and existing critic tests pass.

Test matrix

Unit tests (tests/test_critic_manifesto_check.py, new):

• test_manifesto_text_rendered_into_prompt — given a fake manifestos dir, the rendered prompt contains each manifesto's body.
• test_critic_report_parses_violations_field — JSON with manifesto_violations populates typed list.
• test_critic_report_back_compat_no_field — JSON without the field parses with manifesto_violations == [].
• test_sev1_violation_forces_request_changes — coercion logic flips overall to request_changes even if model says approve.
• test_sev2_violation_does_not_block — sev2 leaves overall untouched.

Adversarial / sad-path:

• test_malformed_violation_entry_is_skipped — entry missing rule_id or severity is dropped, not crashed.
• test_unknown_severity_treated_as_sev3 — defensive default.

Integration:

• Cycle test (extend existing critic integration harness if present, else add): run critic against a fixture PR diff that contains a known manifesto violation (e.g. except Exception: pass) → expect sev1 violation + request_changes. Then a fix-up diff that removes it → expect clean report.

Auto-merge gate:

• test_automerge_blocked_on_sev1_violation — feed a CriticReport with sev1 violation into the merge gate; assert merge is refused.

Out of scope

• Authoring or editing the manifesto documents themselves.
• Teaching the worker agent to pre-check its own PRs against manifestos (separate ticket).
• Surfacing violations in the dashboard UI (separate ticket).
• Per-repo / per-tenant manifesto overrides.
• Changing severity levels of existing manifesto rules.
• Reworking the critic's overall verdict taxonomy beyond adding the sev1 → request_changes coupling.

File pointers

• src/forge_loop/_critic_sdk.py — load manifestos, inject into prompt.
• src/forge_loop/critic.py — extend CriticReport, add ManifestoViolation type, update _coerce_report and parse_report_from_text.
• src/forge_loop/briefs/critic.md.tmpl — add manifesto section + JSON schema instruction for manifesto_violations.
• src/forge_loop/critic_actions.py — auto-merge gate (investigate exact function).
• docs/manifestos/ or src/forge_loop/manifestos/ — (investigate) the manifesto source-of-truth directory.
• tests/test_critic_manifesto_check.py — new test module.
• tests/fixtures/ — (investigate) add fixture PR diffs for the cycle test if a fixtures convention exists.

Worker note

AC is wide — worker, you are at high risk of running out of turns before pushing. The work spans the prompt template, the report dataclass, the parser, the merge gate, and several test files. Apply COMMIT DISCIPLINE (wip-commit every 20 turns / 5 file-edits) aggressively from the start, and run the EXIT CHECKLIST even if implementation feels incomplete. Suggested commit slicing:

1. ManifestoViolation type + CriticReport field + parser back-compat tests.
2. Prompt template + _critic_sdk.py manifesto loading.
3. sev1 → request_changes coupling + auto-merge gate wiring.
4. Integration / cycle test.

Original report

Parent

Part of #130.

What

Critic prompt includes the manifestos. Critic verdict is request_changes (sev1) when a PR visibly violates a quality or testing rule.

Acceptance

• _critic_sdk.py renders the manifestos into the critic prompt.
• Critic outputs a new manifesto_violations: list[ManifestoViolation] field on CriticReport (rule_id + quote from PR + suggested fix).
• Any violation marked severity: sev1 blocks auto-merge.
• Tests: critic with violations / without; cycle-test that a fix PR clears the violation.

File pointers

• src/forge_loop/_critic_sdk.py
• src/forge_loop/critic.py — extend CriticReport
• src/forge_loop/briefs/critic.md.tmpl
• tests/test_critic_manifesto_check.py (new)

[hadamrd]

forge-loop attempt — ⏳ PR OPEN (~351s)

PR: #141

fingerprint: 6c44b162dc65

{
  "ts": "2026-05-28T17:28:11+00:00",
  "status": "open",
  "pr_url": "https://github.com/hadamrd/forge-loop/pull/141",
  "duration_s": 350.5,
  "note": "",
  "event_count": 0,
  "brief_fingerprint": "6c44b162dc6515ce2c22c5c798160b5b5e2626079ad692ee39bf80ce625b82b9"
}