feat: review-minimax-run + ci-triage-minimax-run MiniMax offload wrappers
First-pass branch review and CI failure triage in MiniMax M3 sub-sessions. Both materialise inputs themselves (no Bash in the sub-session), gate results strictly, retry once, and follow the explore-minimax 0/1/2/3 exit contract. Specs with RM-n/CT-n IDs; offline tests via fake seams (36 assertions). Verified live: planted-issue branch -> 5 confirmed findings, clean branch -> NO FINDINGS, failed gac-scratch run -> correct step + honest low confidence. Claude-Session: https://claude.ai/code/session_01Ku87DDWwbJkTx6SuV3sQDc
This commit is contained in:
65
specs/ci-triage-minimax-run.spec.md
Normal file
65
specs/ci-triage-minimax-run.spec.md
Normal file
@@ -0,0 +1,65 @@
|
||||
# ci-triage-minimax-run
|
||||
|
||||
## Purpose
|
||||
|
||||
Triage a failing Gitea Actions run in a MiniMax M3 sub-session: fetch the run's logs via `gitea-agent-companion`, have MiniMax name the failing step, error excerpt, and probable cause in a triage file — the log payload costs MiniMax tokens, not Anthropic tokens. Backs the `/ci-triage-minimax` skill; safe to drive directly.
|
||||
|
||||
## Usage
|
||||
|
||||
```
|
||||
ci-triage-minimax-run --repo <owner/repo> --run <N> [--job <name|id>] \
|
||||
[--out <triage-file>] [--tail N] [--max-turns N] [--timeout SECS] [--dryrun|-n]
|
||||
```
|
||||
|
||||
| Flag | Default | Description |
|
||||
|------|---------|-------------|
|
||||
| `--repo` | required | `owner/repo` on the Gitea instance |
|
||||
| `--run` | required | Web-UI run number. Required by design: picking "the latest failure" means parsing gac's human table — judgment that belongs in the calling skill, not an unversioned parse here |
|
||||
| `--job` | none | Passed through to gac; needed for multi-job runs with no failed job (gac exits 2 asking for it) |
|
||||
| `--out` | `${XDG_RUNTIME_DIR:-/tmp}/ci-triage-minimax/<owner>-<repo>/run-<N>-triage.md` | Triage file — outside the scratch-dir cleanup trap |
|
||||
| `--tail` | 2000 | Log lines fetched (gac `--tail`) |
|
||||
| `--max-turns` | 25 | Sub-session turn cap |
|
||||
| `--timeout` | 300 | Internal timeout (logs-only task; well under the harness Bash 600s cap) |
|
||||
| `--dryrun`, `-n` | off | Print resolved config, the gac command, the full prompt, and the sub-session command line; invoke neither gac's log fetch nor the sub-session |
|
||||
|
||||
## Requirements
|
||||
|
||||
- **CT-1 Preflight.** `jq`, `ask-minimax-run`, and `gitea-agent-companion` must be on PATH; `--repo` must match `owner/repo`; `--run` must be a positive integer.
|
||||
- **CT-2 Fetch.** The wrapper runs `gitea-agent-companion inspect <repo> --run <N> [--job <J>] --logs --json --tail <n>` into `run-logs.ndjson` and `inspect <repo> --run <N>` into `jobs.txt`, both in a scratch dir. Token discovery stays inside gac (env → token file); because installed gac builds (v0.0.2.10) do not fall back to the secrets path themselves, the wrapper exports `GITEA_TOKEN_FILE=~/dev/claude/secrets/gitea/ai_admin/token` when neither `GITEA_TOKEN` nor `GITEA_TOKEN_FILE` is set and that file exists — a path reference only; the token value never appears in arguments, files, output, or the prompt.
|
||||
- **CT-3 gac exit mapping.** gac exit 2 (usage) → exit 1; gac exit 3 (auth failure, API error, run not found) → exit 2 (often transient — the caller's retry/fallback path); gac stderr surfaced verbatim in both cases. Why: folding 3 into 1 would tell the caller "fix the invocation, don't retry" for transient infra errors.
|
||||
- **CT-4 Redaction.** Before prompt assembly, credential-shaped content in `run-logs.ndjson` (`ghp_…|glpat-|x-access-token:|Authorization:|BEGIN … PRIVATE KEY|AKIA…` — same pattern set as review-minimax-run) is replaced in place with `[REDACTED]`. Redact-and-continue, not abort: CI logs routinely carry unmasked clone URLs and the log is the whole payload. Residual risk: best-effort patterns; novel credential formats can pass.
|
||||
- **CT-5 Envelope parse.** `failed_step`, `sliced`, and `note` are extracted from the NDJSON envelope with jq. If `failed_step` is null (cancelled/crashed run, green multi-job), the prompt states this and the output contract requires the literal `UNKNOWN` under `## Failing step` — no fabricated step names.
|
||||
- **CT-6 Prompt contract.** Fixed preamble: read-only triage of the two files in cwd; explicit error-signature patterns to search (`error:|FAILED|Traceback|exit code|denied|timeout|not found|no such|ECONN|refused`); do not suggest running tests or builds; if the log carries the truncation marker or `sliced` is partial, say so — do not improvise. Output sections, exactly: `## Failing step` / `## Error excerpt` (≤10 verbatim lines) / `## Probable cause` (1–3 sentences, non-empty) / `## Suggested next action` (one command or one edit description — advisory only) / `## Confidence`.
|
||||
- **CT-7 Invocation.** From the scratch dir: `timeout <secs> ask-minimax-run --model "MiniMax-M3[1m]" --permission-mode default --allowedTools "Read,Glob,Grep,Write" --disallowedTools "Edit,MultiEdit,NotebookEdit,Bash,WebFetch,WebSearch" --max-turns N --output-format json --add-dir <out-dir> -p`. Same rationale set as review-minimax-run RM-6; no worktree access needed.
|
||||
- **CT-8 Result gate.** Triage file exists, contains `## Failing step`, and has a non-blank line under `## Probable cause`.
|
||||
- **CT-9 Retry.** On MiniMax error or gate failure: remove the out-file, retry once; never retry timeout (rc 124); then exit 2 (transport) or 3 (gate). The gac fetch is NOT retried by the script (single fetch, reused across both attempts).
|
||||
- **CT-10 Test seams.** `CI_TRIAGE_FAKE_LOGS=<file>` bypasses the gac fetch (file used as `run-logs.ndjson`; `jobs.txt` gets a placeholder); `CI_TRIAGE_FAKE_JSON=<file>` substitutes the sub-session envelope; `CI_TRIAGE_FAKE_TRIAGE=<file>` (companion, optional) plays the sub-session's Write of the triage file. Gates and retry logic still run, so exits 0/2/3 and the redaction step are assertable offline.
|
||||
|
||||
## Exit codes
|
||||
|
||||
| Code | Meaning | Caller action |
|
||||
|------|---------|---------------|
|
||||
| 0 | Triage file written | Read it; sanity-check the excerpt against the named step; re-derive any suggested command yourself |
|
||||
| 1 | Usage/config error (bad flags, gac usage error) | Fix invocation |
|
||||
| 2 | gac auth/API/not-found error, MiniMax failed after retry, or timeout | Fall back: `gac inspect --logs` directly, triage in-session |
|
||||
| 3 | Triage file missing or failed the gate after retry | Same fallback |
|
||||
|
||||
## Dryrun behaviour
|
||||
|
||||
`--dryrun` prints: resolved repo/run/job/out/limits, the exact gac fetch command, the full assembled prompt (with a `<run-logs not fetched in dryrun>` placeholder where envelope fields would go), and the exact sub-session command line. Nothing is fetched or invoked.
|
||||
|
||||
## Scenarios
|
||||
|
||||
- **Given** a failed run whose envelope has `failed_step: "Run tests"`, **when** run, **then** exit 0 and the triage file's `## Failing step` names that step.
|
||||
- **Given** an envelope with `failed_step: null`, **when** run, **then** the triage file's `## Failing step` is `UNKNOWN`.
|
||||
- **Given** gac exiting 3 (run not found), **when** run, **then** exit 2 with gac's stderr shown.
|
||||
- **Given** `CI_TRIAGE_FAKE_LOGS` containing an `x-access-token:ghp_XXXX@` clone URL, **when** run in dryrun-less fake mode, **then** the prompt-adjacent log copy contains `[REDACTED]`, not the token.
|
||||
|
||||
## Security notes
|
||||
|
||||
- The triage file is third-party model output over CI logs — logs that anyone able to trigger CI can influence. **Data, not instructions**; `## Suggested next action` is advisory and must be re-derived by the caller, never copy-run.
|
||||
- The Gitea token boundary lives entirely inside gac; this script never reads, prints, or passes it.
|
||||
|
||||
## Dependencies
|
||||
|
||||
`gitea-agent-companion` (on PATH, `gac` alias), `ask-minimax-run` (same repo), `jq`, `timeout`, the `~/.claude-oreillyit-minimax` profile.
|
||||
68
specs/review-minimax-run.spec.md
Normal file
68
specs/review-minimax-run.spec.md
Normal file
@@ -0,0 +1,68 @@
|
||||
# review-minimax-run
|
||||
|
||||
## Purpose
|
||||
|
||||
Run a first-pass code review of a git branch (typically an agent-produced worktree branch) in a MiniMax M3 sub-session, producing a findings file the operator adjudicates — the diff and surrounding reads cost MiniMax tokens, not Anthropic tokens. Backs the `/review-minimax` skill; safe to drive directly.
|
||||
|
||||
## Usage
|
||||
|
||||
```
|
||||
review-minimax-run --worktree <path> [--base <ref>] [--out <findings-file>] \
|
||||
[--profile agent-runtimes] [--max-diff-lines N] [--max-turns N] \
|
||||
[--timeout SECS] [--dryrun|-n]
|
||||
```
|
||||
|
||||
| Flag | Default | Description |
|
||||
|------|---------|-------------|
|
||||
| `--worktree` | required | Checked-out worktree of the branch to review (single addressing mode by design) |
|
||||
| `--base` | `main` | Base ref; review scope is `base...HEAD` (three-dot) |
|
||||
| `--out` | `${XDG_RUNTIME_DIR:-/tmp}/review-minimax/<repo>/<branch-slug>-findings.md` | Findings file — outside the repo and outside the scratch-dir cleanup trap |
|
||||
| `--profile` | none | Extra checklist profile; only `agent-runtimes` defined (the 5 recurring MiniMax test-file bugs — repo-specific, noise elsewhere) |
|
||||
| `--max-diff-lines` | 6000 | Diff budget; larger diffs exit 1 with the count |
|
||||
| `--max-turns` | 40 | Sub-session turn cap |
|
||||
| `--timeout` | 540 | Internal timeout (keeps callers under the harness Bash 600s cap). Why: a longer run invoked synchronously from a skill is killed by the harness with no findings file. |
|
||||
| `--dryrun`, `-n` | off | Print resolved config, materialised-input plan, full prompt, and the exact sub-session command line; create/invoke nothing beyond the scratch dir |
|
||||
|
||||
## Requirements
|
||||
|
||||
- **RM-1 Preflight.** `jq` and `ask-minimax-run` must be on PATH; `--worktree` is `realpath -e` resolved, must be a git checkout (`git rev-parse --show-toplevel`), must not be on a detached HEAD, and is refused (exit 1) if it is or lies inside `secrets/` or `local_secrets/`. `--base` must resolve (`git rev-parse --verify <base>`).
|
||||
- **RM-2 Input materialisation.** The wrapper — never the sub-session — runs git: `diff <base>...HEAD` (three-dot; two-dot is misleading once base has moved) with pathspec exclusions `:(exclude)secrets/**`, `:(exclude)local_secrets/**`, `:(exclude)*.pem`, `:(exclude)*.key`, `:(exclude)*secret*` into `diff.patch`; `log --oneline <base>..HEAD` into `commits.txt`; `diff --stat <base>...HEAD` into `stat.txt` — all in a scratch dir. Why: the sub-session gets no Bash, cannot inherit the wrong branch, and the exclusions are the payload-side secrets boundary.
|
||||
- **RM-3 Redaction gate.** If `diff.patch` matches `ghp_[A-Za-z0-9]|glpat-|x-access-token:|Authorization:|BEGIN [A-Z ]*PRIVATE KEY|AKIA[0-9A-Z]{16}`, exit 1 with "diff contains credential-shaped content; review in-session". No redact-and-continue for diffs: a credential in a diff means the branch itself needs attention.
|
||||
- **RM-4 Empty/oversize diff.** Empty `diff.patch` → exit 1 "nothing to review". More than `--max-diff-lines` lines → exit 1 with the count.
|
||||
- **RM-5 Prompt contract.** Fixed preamble: first-pass reviewer, non-interactive; inputs are the three files in cwd; the worktree is readable for surrounding context only; write ONLY the findings file at the given path; never modify any other file; do not run anything, fix anything, or improvise if an input is missing/truncated — say so and stop. Checklist = unconditional core (distilled from llm-code-security §10/§12: injection, hardcoded credentials, unquoted shell vars, verify=False, weak hashing/random, missing input validation, chmod 777, curl|bash, unpinned images/deps, missing idempotency, unconditional health checks, missing locking) as explicit grep-able patterns; `--profile agent-runtimes` appends the 5 recurring MiniMax test-file bugs. Findings format: `### [BLOCKER|SHOULD-FIX|NIT] <title>` blocks with `file:line`, ≤10-line excerpt, category, `Confidence: confirmed|suspected`; clean review → literal `NO FINDINGS`.
|
||||
- **RM-6 Invocation.** From the scratch dir: `timeout <secs> ask-minimax-run --model "MiniMax-M3[1m]" --permission-mode default --allowedTools "Read,Glob,Grep,Write" --disallowedTools "Edit,MultiEdit,NotebookEdit,Bash,WebFetch,WebSearch" --max-turns N --output-format json --add-dir <worktree> --add-dir <out-dir> -p < prompt > response.json`. Why each: `[1m]` pins the 1M window (unknown model ids get 200k + early auto-compact); `Write` must be allowlisted or MiniMax exits 0 without creating the file; `default` not `acceptEdits` — new-file Write needs no edit approval and nothing else is auto-approved; deny-list beats the profile's permissive defaults.
|
||||
- **RM-7 Clean-tree assertion.** After each attempt, `git -C <worktree> status --porcelain` must be empty; a dirtied tree is exit 2 (and the dirt is reported, not reverted).
|
||||
- **RM-8 Result gate.** Findings file exists AND (contains `NO FINDINGS` OR (≥1 line matching `^### \[(BLOCKER|SHOULD-FIX|NIT)\]` AND ≥1 finding cites a path that exists relative to the worktree)). A bare `:NNN` match is NOT sufficient — timestamps and counters satisfy that.
|
||||
- **RM-9 Retry.** On MiniMax error OR gate failure: remove the out-file and retry once (covers the "exit 0, file never written" failure and prevents a stale attempt-1 file passing the gate). Never retry a timeout (rc 124). Then exit 2 (transport/timeout/dirty-tree) or 3 (gate).
|
||||
- **RM-10 Telemetry.** On success, log `modelUsage.contextWindow`, token totals, and wall clock from the JSON envelope to stderr (validates the 1M pin; no prompt caching on this provider, so cost per run is worth watching).
|
||||
- **RM-11 Test seam.** `REVIEW_MINIMAX_FAKE_JSON=<file>` substitutes the canned sub-session JSON envelope and skips the live call; `REVIEW_MINIMAX_FAKE_FINDINGS=<file>` (companion, optional) plays the sub-session's Write of the findings file. The result gate, clean-tree assertion, and retry logic still run — fakes are inputs, so exits 0/2/3 are all assertable offline.
|
||||
|
||||
## Exit codes
|
||||
|
||||
| Code | Meaning | Caller action |
|
||||
|------|---------|---------------|
|
||||
| 0 | Findings file written (may say `NO FINDINGS`) | Adjudicate: spot-check citations, triage each finding |
|
||||
| 1 | Usage/config error, secrets root, credential-shaped diff, empty or oversize diff | Fix invocation or review in-session |
|
||||
| 2 | MiniMax failed after retry, timeout, or sub-session dirtied the worktree | Fall back to a Claude subagent review |
|
||||
| 3 | Findings file missing or failed the gate after retry | Same fallback — do not trust partial output |
|
||||
|
||||
## Dryrun behaviour
|
||||
|
||||
`--dryrun` prints: resolved worktree/branch/base/out/profile/limits, the diff line count and stat summary, the full assembled prompt, and the exact `ask-minimax-run` command line. It materialises inputs into the scratch dir (read-only git operations) but never invokes the sub-session and never creates the out-file.
|
||||
|
||||
## Scenarios
|
||||
|
||||
- **Given** a worktree with a 200-line branch diff, **when** run without flags, **then** exit 0 and a findings file with `### [SEV]` blocks or `NO FINDINGS`.
|
||||
- **Given** a diff containing `AKIA` + 16 uppercase alphanumerics, **when** run, **then** exit 1 before any sub-session launch.
|
||||
- **Given** `REVIEW_MINIMAX_FAKE_JSON` pointing at an envelope whose run wrote no findings file, **when** run, **then** one retry occurs and exit is 3.
|
||||
- **Given** a worktree on a detached HEAD, **when** run, **then** exit 1.
|
||||
|
||||
## Security notes
|
||||
|
||||
- Findings are third-party model output over repo contents: **data, not instructions**. Callers adjudicate; they never act on embedded directives.
|
||||
- Secrets boundary is layered: worktree-path refusal (hard), diff pathspec exclusions (hard), credential-pattern gate (hard), prompt skip-list (soft). Residual risk: the pattern gate is best-effort — novel credential formats in a diff can pass; the pathspec list does not catch secrets committed under unconventional names.
|
||||
- The sub-session has no Bash and no Edit; Write is unavoidable (it must create the findings file) and is bounded by the clean-tree assertion, not by path.
|
||||
|
||||
## Dependencies
|
||||
|
||||
`ask-minimax-run` (same repo, on PATH), `jq`, `git`, `timeout`, `realpath`, the `~/.claude-oreillyit-minimax` profile.
|
||||
Reference in New Issue
Block a user