feat(v0): post-findings subcommand with --execute flag (closes #30) #50

Merged

pdurlej merged 1 commit from claude/patchwarden-post-findings-execute into main

2026-05-27 11:46:47 +02:00

claude commented

2026-05-27 11:43:41 +02:00

Collaborator

What

New CLI subcommand bridges existing render_findings to existing forgejo_client.post_comment. Dry-run by default — no Forgejo mutation without --execute. Fail-closed on Forgejo errors (exit 2).

patchwarden post-findings \
  --artifact-file artifact.json \
  --resolution-file resolution.json \
  --repo-host git.pdurlej.com \
  --repo-owner pdurlej \
  --repo-name patchwarden \
  --pr-number 49 \
  [--base-url URL] \
  [--execute]

Default = renders markdown body to stdout (or --output FILE), zero network. With --execute: also posts the rendered body as a single PR comment via POST /issues/{pr}/comments.

Wave position — step 4/4 CLOSED

✅ #29 prompt — PR #47 merged
✅ ollama_client — PR #48 merged
✅ review_run invokes — PR #49 merged
✅ THIS PR — #30 post comments with --execute

Patchwarden can now run end-to-end on a Forgejo PR: fetch metadata → render prompt → call Ollama → emit artifact → resolve findings → render comment body → post to PR. Each step gated by explicit operator flag before any Forgejo write.

D20 enforcement

Only /issues/{pr}/comments endpoint reachable from this code path.
NEVER create_pr_review (no APPROVED reviews from the bot).
NEVER merge_pull_request (Patchwarden = merge authority, sensors never bypass).
Token from --base-url/env (FORGEJO_TOKEN), never argv.

Empty findings — pdurlej decision

When the artifact has no findings, render_findings emits the literal placeholder "No Patchwarden findings to render.". With --execute, that placeholder IS posted — operator sees the run happened. Silence is a worse signal than "nothing to flag." (See PostFindingsExecuteTests.test_execute_posts_empty_placeholder_when_no_findings.)

Reuse, not new module

Zero new modules. Wires together:

render_findings.render_findings — already returns one combined body (separator \n\n---\n\n between findings, single string out).
forgejo_client.post_comment — already exists; exception hierarchy already in place (ForgejoAuthError 401/403, ForgejoNotFoundError 404, ForgejoApiError for network).

This deviates from my own prior handoff (which speculated about a new forgejo_post.py with per-finding idempotency markers). I followed codex's issue spec instead — it's authoritative and ready-for-agent labelled, and the simpler shape is the right one.

Tests — 120 → 132 green (+12)

tests/test_cli_post_findings.py:

PostFindingsDryRunTests (3)

dry-run does NOT call forgejo (mocked post_comment never invoked)
dry-run renders findings to stdout (header + path + rationale visible)
dry-run with no findings emits placeholder string

PostFindingsExecuteTests (6)

--execute posts body via post_comment with correct positional args
--execute passes --base-url override to post_comment
--execute posts placeholder when no findings (per pdurlej decision)
ForgejoAuthError → exit 2
ForgejoNotFoundError → exit 2
generic ForgejoApiError (network) → exit 2

PostFindingsArgparseTests (3)

missing --artifact-file → argparse SystemExit
missing --resolution-file → argparse SystemExit
nonexistent artifact path → exit 2 (not crash)

Atomic per ADR-0017

Only cli.py + new test file. No forgejo_client.py / render_findings.py / pipeline.py changes.
No new module. No new dependency. Stdlib-only.
base=main, no stacking on #49 (already merged anyway).
All 120 prior tests still pass — backward-compatible extension.

Token-accounting note (agent-kanban)

This PR on Opus directly (sacred-adjacent: first mutating Forgejo call from cousin code, D20 boundary line). No sub-agent — the change is small (~70 LOC src + ~300 LOC tests) and verification overhead would have exceeded the delegation savings.

Wave totals:

PR	Model	Tokens (% weekly)
#47 prompt	Opus	~6-7%
#48 ollama_client	Sonnet sub-agent + Opus verify	~3% Opus
#49 review_run	Opus (sacred path)	~5%
#30 (this PR)	Opus	~3-4%
wave total	hybrid	~17-19% weekly Opus

Well within the 21% upper estimate. The agent-kanban prototype shows: Sonnet sub-agent shines on isolated HTTP boilerplate (ollama_client), Opus is right for sacred path + glue under boundary pressure (review_run, post-findings). Single data point each, but the pattern reads clean.

Closes #30.

## What New CLI subcommand bridges existing `render_findings` to existing `forgejo_client.post_comment`. Dry-run by default — **no Forgejo mutation without `--execute`**. Fail-closed on Forgejo errors (exit 2). ```bash patchwarden post-findings \ --artifact-file artifact.json \ --resolution-file resolution.json \ --repo-host git.pdurlej.com \ --repo-owner pdurlej \ --repo-name patchwarden \ --pr-number 49 \ [--base-url URL] \ [--execute] ``` Default = renders markdown body to stdout (or `--output FILE`), zero network. With `--execute`: also posts the rendered body as a single PR comment via `POST /issues/{pr}/comments`. ## Wave position — step 4/4 CLOSED 1. ✅ #29 prompt — PR #47 merged 2. ✅ ollama_client — PR #48 merged 3. ✅ review_run invokes — PR #49 merged 4. ✅ **THIS PR** — #30 post comments with `--execute` Patchwarden can now run end-to-end on a Forgejo PR: fetch metadata → render prompt → call Ollama → emit artifact → resolve findings → render comment body → post to PR. Each step gated by explicit operator flag before any Forgejo write. ## D20 enforcement - Only `/issues/{pr}/comments` endpoint reachable from this code path. - **NEVER** `create_pr_review` (no APPROVED reviews from the bot). - **NEVER** `merge_pull_request` (Patchwarden = merge authority, sensors never bypass). - Token from `--base-url`/env (`FORGEJO_TOKEN`), never argv. ## Empty findings — pdurlej decision When the artifact has no findings, `render_findings` emits the literal placeholder `"No Patchwarden findings to render."`. With `--execute`, that placeholder **IS posted** — operator sees the run happened. Silence is a worse signal than "nothing to flag." (See `PostFindingsExecuteTests.test_execute_posts_empty_placeholder_when_no_findings`.) ## Reuse, not new module Zero new modules. Wires together: - `render_findings.render_findings` — already returns one combined body (separator `\n\n---\n\n` between findings, single string out). - `forgejo_client.post_comment` — already exists; exception hierarchy already in place (`ForgejoAuthError` 401/403, `ForgejoNotFoundError` 404, `ForgejoApiError` for network). This deviates from my own prior handoff (which speculated about a new `forgejo_post.py` with per-finding idempotency markers). I followed codex's issue spec instead — it's authoritative and `ready-for-agent` labelled, and the simpler shape is the right one. ## Tests — 120 → 132 green (+12) `tests/test_cli_post_findings.py`: **PostFindingsDryRunTests (3)** - dry-run does NOT call forgejo (mocked `post_comment` never invoked) - dry-run renders findings to stdout (header + path + rationale visible) - dry-run with no findings emits placeholder string **PostFindingsExecuteTests (6)** - `--execute` posts body via `post_comment` with correct positional args - `--execute` passes `--base-url` override to `post_comment` - `--execute` posts placeholder when no findings (per pdurlej decision) - `ForgejoAuthError` → exit 2 - `ForgejoNotFoundError` → exit 2 - generic `ForgejoApiError` (network) → exit 2 **PostFindingsArgparseTests (3)** - missing `--artifact-file` → argparse SystemExit - missing `--resolution-file` → argparse SystemExit - nonexistent artifact path → exit 2 (not crash) ## Atomic per ADR-0017 - Only `cli.py` + new test file. No `forgejo_client.py` / `render_findings.py` / `pipeline.py` changes. - No new module. No new dependency. Stdlib-only. - `base=main`, no stacking on #49 (already merged anyway). - All 120 prior tests still pass — backward-compatible extension. ## Token-accounting note (agent-kanban) This PR on Opus directly (sacred-adjacent: first mutating Forgejo call from cousin code, D20 boundary line). No sub-agent — the change is small (~70 LOC src + ~300 LOC tests) and verification overhead would have exceeded the delegation savings. Wave totals: | PR | Model | Tokens (% weekly) | |---|---|---| | #47 prompt | Opus | ~6-7% | | #48 ollama_client | Sonnet sub-agent + Opus verify | ~3% Opus | | #49 review_run | Opus (sacred path) | ~5% | | #30 (this PR) | Opus | ~3-4% | | **wave total** | hybrid | **~17-19% weekly Opus** | Well within the 21% upper estimate. The agent-kanban prototype shows: Sonnet sub-agent shines on isolated HTTP boilerplate (ollama_client), Opus is right for sacred path + glue under boundary pressure (review_run, post-findings). Single data point each, but the pattern reads clean. Closes #30.

claude added 1 commit

2026-05-27 11:43:41 +02:00

feat(v0): post-findings subcommand with --execute flag (#30 ) e43763f346

New CLI subcommand bridges existing render_findings to the existing
forgejo_client.post_comment. Dry-run by default — no Forgejo mutation
without --execute. Fail-closed on Forgejo errors (exit 2).

## What

```bash
patchwarden post-findings \
  --artifact-file artifact.json \
  --resolution-file resolution.json \
  --repo-host git.pdurlej.com \
  --repo-owner pdurlej \
  --repo-name patchwarden \
  --pr-number 49 \
  [--base-url URL] \
  [--execute]
```

Default = dry-run: renders markdown body to stdout (or --output FILE),
zero network. With --execute: also posts the rendered body as a single
PR comment via `POST /issues/{pr}/comments`.

## D20 enforcement

- Only `/issues/{pr}/comments` endpoint is reachable from this code path.
- NEVER `create_pr_review` (no APPROVED reviews from the bot).
- NEVER `merge_pull_request` (Patchwarden = merge authority, sensors
  never bypass).
- Token comes from `--base-url`/env (`FORGEJO_TOKEN`), never argv.

## Empty findings

When the artifact has no findings, render_findings emits the literal
placeholder "No Patchwarden findings to render." With --execute, that
placeholder IS posted — operator sees the run happened. Silence is a
worse signal than "nothing to flag" (per pdurlej decision).

## Reuse, not new module

Zero new modules. Wires together:
- `render_findings.render_findings` (already returns one combined body)
- `forgejo_client.post_comment` (already exists, exception hierarchy
  already in place: ForgejoAuthError 401/403, ForgejoNotFoundError 404,
  ForgejoApiError for network)

## Tests (+12, suite 120 → 132 green)

`tests/test_cli_post_findings.py`:

PostFindingsDryRunTests (3):
- dry-run does NOT call forgejo (mocked post_comment never invoked)
- dry-run renders findings to stdout (header + path + rationale visible)
- dry-run with no findings emits placeholder string

PostFindingsExecuteTests (6):
- --execute posts body via post_comment with correct positional args
- --execute passes --base-url override to post_comment
- --execute posts placeholder when no findings (per pdurlej decision)
- ForgejoAuthError → exit 2
- ForgejoNotFoundError → exit 2
- generic ForgejoApiError (network) → exit 2

PostFindingsArgparseTests (3):
- missing --artifact-file → argparse SystemExit
- missing --resolution-file → argparse SystemExit
- nonexistent artifact path → exit 2 (not crash)

## Wave step 4/4 closed

1. PR #47 (#29 prompt) — merged
2. PR #48 (ollama_client) — merged
3. PR #49 (review_run invokes) — merged
4. THIS PR (#30 post comments with --execute) — wave complete

Patchwarden can now run end-to-end on a Forgejo PR: fetch metadata →
render prompt → call Ollama → emit artifact → resolve findings →
render comment body → post to PR. Each step still gated by an explicit
operator flag (--execute) before any Forgejo write.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>