diff --git a/.github/scripts/probe_issue.py b/.github/scripts/probe_issue.py index e90aef0..5989aab 100644 --- a/.github/scripts/probe_issue.py +++ b/.github/scripts/probe_issue.py @@ -352,6 +352,16 @@ def test_deploy(charm, juju: jubilant.Juju): about integration test behaviour. Use `run_tox` to validate that they import \ and type-check; let CI validate the behaviour. +**Be direct.** Prefer straightforward tests over clever workarounds. Do not \ +dynamically generate config files, spawn subprocesses, or write meta-tests \ +that test pytest itself. Configure the charm's `pyproject.toml` directly and \ +write tests that use the charm's own configuration. If you need to test \ +different configurations, use the multiple charms available — there are four \ +(`kepler`, `kosmos`, `meteor`, `micron`), each with its own `pyproject.toml` \ +and test directories. This is especially useful for differential testing: \ +configure one charm one way and another differently, then run the same test \ +in both. + ### Differential testing with xfail Sometimes a claim is best tested by showing that the SAME test behaves \ @@ -422,30 +432,32 @@ def test_deploy(charm, juju: jubilant.Juju): The happy path is the default: if you make file changes, the workflow treats \ that as IMPLEMENT and proceeds to path enforcement. However, you MUST still \ -emit a literal `IMPLEMENTATION_REASONING:` marker in your output — the \ -workflow parses for this exact string at the start of a line. Without it, the \ -run fails even if you made all the right changes. +emit a literal `IMPLEMENTATION_RESULT:` marker in your output — the \ +workflow parses for this exact string. Without it, the run fails even if you \ +made all the right changes. **After `run_tox` passes for all modified charms, end your output with a line \ -that starts with `IMPLEMENTATION_REASONING:` followed by your reasoning.** \ -The first line of your reasoning becomes the PR title, so start with a \ -condensed title — a short phrase like "Try foo in bar tests" or "log_level \ -filters DEBUG from captured logs". Then continue with the full reasoning on \ -subsequent lines. For example: +that starts with `IMPLEMENTATION_RESULT:` followed by a JSON object with two \ +fields:** -``` -IMPLEMENTATION_REASONING: log_level=INFO does not affect Jubilant's captured logs when log_cli_level is already INFO +- `title`: a compact PR title — a short phrase, not a full sentence. \ +Examples: "Try foo in bar tests", "log_level filters DEBUG from captured \ +logs". Must not exceed 70 characters. +- `body`: the full PR description in markdown. Enumerate the claims you \ +identified (A, B, C), state which you tested and why, what you believe is \ +true, what the PR tests, and what green (or red) CI means for each claim. \ +Use proper markdown: headers (`##`), bullet points, code blocks (fenced \ +with triple backticks), and paragraphs separated by blank lines. + +For example: -The doc claims A: ... and B: ... I believe ... I added a test asserting ... -If CI passes, ... If CI fails, ... +``` +IMPLEMENTATION_RESULT: {"title": "log_level filters DEBUG from captured logs", "body": "## Claims\\n\\n- **A**: log_level=INFO retains INFO logs in the captured section\\n- **B**: without it, DEBUG logs appear from log_file_level\\n\\nI believe the doc is correct. I added a test asserting ...\\n\\nIf CI passes, ... If CI fails, ..."} ``` The reasoning is a core part of the adversarial approach: the reviewer needs it \ -to interpret the CI results. Enumerate the claims you identified (A, B, C), \ -state which you tested and why, what you believe is true, what the PR tests, \ -and what green (or red) CI means for each claim. Write it in plain \ -conversational English (see Voice below). Do not use markdown headers or \ -formatting — just plain text after the marker. +to interpret the CI results. Write it in plain conversational English (see Voice \ +below). If `run_tox` fails and you cannot fix the issue, emit \ `IMPLEMENTATION_BLOCKER: ` instead. Do not \ @@ -600,10 +612,9 @@ def parse_decision(output: str) -> dict[str, str]: The blocker is the explicit opt-out: if an `IMPLEMENTATION_BLOCKER:` line is present, the decision is BLOCKED. Otherwise the decision is IMPLEMENT (the - happy path is the default), and the reasoning is taken from the required - `IMPLEMENTATION_REASONING:` line. The reasoning is a core part of the - adversarial approach — the reviewer needs it to interpret the CI results — - so its absence is a genuine failure, not something to paper over. + happy path is the default), and the result is taken from the required + `IMPLEMENTATION_RESULT:` line, which contains a JSON object with `title` + and `body` fields. """ blocker_match = re.search( r"^IMPLEMENTATION_BLOCKER:\s*(.+?)\s*$", @@ -616,22 +627,32 @@ def parse_decision(output: str) -> dict[str, str]: raise ValueError("IMPLEMENTATION_BLOCKER must not be empty.") return {"decision": "BLOCKED", "blocker": blocker} - reasoning_match = re.search( - r"^IMPLEMENTATION_REASONING:\s*(.*)$", + result_match = re.search( + r"^IMPLEMENTATION_RESULT:\s*(\{.*\})\s*$", output, - re.MULTILINE | re.DOTALL, + re.MULTILINE, ) - if not reasoning_match: + if not result_match: raise ValueError( - "IMPLEMENT requires an IMPLEMENTATION_REASONING line. The reasoning " - "is a core part of the adversarial approach — the reviewer needs it to " - "interpret the CI results. If the agent stopped without one, that is a " - "genuine failure worth investigating, not something to paper over." + "IMPLEMENT requires an IMPLEMENTATION_RESULT line with a JSON " + "object containing 'title' and 'body' fields. If the agent stopped " + "without one, that is a genuine failure worth investigating, not " + "something to paper over." ) - reasoning = reasoning_match.group(1).strip() - if not reasoning: - raise ValueError("IMPLEMENTATION_REASONING must not be empty.") - return {"decision": "IMPLEMENT", "reasoning": reasoning} + import json + + try: + result = json.loads(result_match.group(1)) + except json.JSONDecodeError as e: + raise ValueError(f"IMPLEMENTATION_RESULT contains invalid JSON: {e}") from e + + title = result.get("title", "").strip() + body = result.get("body", "").strip() + if not title: + raise ValueError("IMPLEMENTATION_RESULT 'title' must not be empty.") + if not body: + raise ValueError("IMPLEMENTATION_RESULT 'body' must not be empty.") + return {"decision": "IMPLEMENT", "title": title, "body": body} # --------------------------------------------------------------------------- @@ -678,10 +699,16 @@ def main(argv: list[str] | None = None) -> int: help="Path to write $GITHUB_OUTPUT lines to.", ) parser.add_argument( - "--reasoning-file", + "--title-file", + type=Path, + default=None, + help="Path to write the PR title to.", + ) + parser.add_argument( + "--body-file", type=Path, default=None, - help="Path to write the IMPLEMENTATION_REASONING text to.", + help="Path to write the PR body to.", ) parser.add_argument( "--blocker-file", @@ -773,8 +800,11 @@ def _run_probe(args) -> int: write_github_output(args.github_output, {"decision": result["decision"]}) # 8. Write reasoning/blocker to files for the workflow to read safely. - if result["decision"] == "IMPLEMENT" and args.reasoning_file: - args.reasoning_file.write_text(result["reasoning"], encoding="utf-8") + if result["decision"] == "IMPLEMENT": + if args.title_file: + args.title_file.write_text(result["title"], encoding="utf-8") + if args.body_file: + args.body_file.write_text(result["body"], encoding="utf-8") if result["decision"] == "BLOCKED" and args.blocker_file: args.blocker_file.write_text(result["blocker"], encoding="utf-8") @@ -782,7 +812,8 @@ def _run_probe(args) -> int: if result["decision"] == "BLOCKED": print(f"IMPLEMENTATION_BLOCKER: {result['blocker']}") else: - print(f"IMPLEMENTATION_REASONING: {result['reasoning']}") + print(f"TITLE: {result['title']}") + print(f"BODY: {result['body']}") return 0 diff --git a/.github/workflows/probe-issue.yaml b/.github/workflows/probe-issue.yaml index 3565024..abb2405 100644 --- a/.github/workflows/probe-issue.yaml +++ b/.github/workflows/probe-issue.yaml @@ -85,7 +85,8 @@ jobs: --branch "$branch" \ --repo-root "$GITHUB_WORKSPACE" \ --github-output "$GITHUB_OUTPUT" \ - --reasoning-file "$RUNNER_TEMP/reasoning.md" \ + --title-file "$RUNNER_TEMP/title.txt" \ + --body-file "$RUNNER_TEMP/body.md" \ --blocker-file "$RUNNER_TEMP/blocker.md" \ --timeout 1200 @@ -145,18 +146,18 @@ jobs: git add --all git commit -m "Probe #$ISSUE_NUMBER" git push --force --set-upstream origin "$branch" - # The first line of the reasoning is the PR title. - title=$(head -1 "$RUNNER_TEMP/reasoning.md") + # The title and body are written to separate files by probe_issue.py. + title=$(cat "$RUNNER_TEMP/title.txt") # Create the PR, or update if it already exists. pr_url=$(gh pr create \ --repo "$REPOSITORY" \ --head "$branch" \ --title "$title" \ - --body-file "$RUNNER_TEMP/reasoning.md" \ + --body-file "$RUNNER_TEMP/body.md" \ 2>/dev/null || \ gh pr edit --repo "$REPOSITORY" "$branch" \ --title "$title" \ - --body-file "$RUNNER_TEMP/reasoning.md") + --body-file "$RUNNER_TEMP/body.md") echo "pr_url=$pr_url" >> "$GITHUB_OUTPUT" - name: Comment on issue diff --git a/AGENT_DESIGN.md b/AGENT_DESIGN.md index 7909795..c05de2e 100644 --- a/AGENT_DESIGN.md +++ b/AGENT_DESIGN.md @@ -56,7 +56,7 @@ The `fetch_url` tool is the other exception: it lets the agent fetch content fro - Compose the prompt: system constraints, runtime context, task instructions, untrusted content (delimited), output contract. - Stage the agent and tools: copy `.github/agent/probe-issue.md` to `.opencode/agents/` and `.github/tools/run_tox.ts` and `.github/tools/fetch_url.ts` to `.opencode/tools/`. - Run OpenCode with `--auto` (auto-approve permissions not explicitly denied) and a scrubbed environment: `PATH`, `HOME`, `USER`, `SHELL`, `LANG`, `OPENROUTER_API_KEY` only. No `GITHUB_TOKEN`, no `ACTIONS_ID_TOKEN_*`. `--auto` is required because the agent runs non-interactively — without it, tools that default to `"ask"` (like `glob`, `grep`, `list`) would prompt for approval and hang forever. Explicit `deny` rules (`bash`, `network`, `web`, `task`) are still enforced. The run is bounded by a 20-minute wall-clock timeout (1200s). If OpenCode exceeds it, the script converts the timeout into a `BLOCKED` decision with a clear "timed out" message rather than crashing — so the issue gets a useful comment instead of a bare workflow failure. The agent's step limit (`steps: 50`) is the other bound. - - Parse the decision: the happy path is the default. If an `IMPLEMENTATION_BLOCKER:` line is present, the decision is `BLOCKED` and the blocker text is written to a file. Otherwise the decision is `IMPLEMENT`; the `IMPLEMENTATION_REASONING:` text is required and written to a file — the reasoning is a core part of the adversarial approach, so its absence is a genuine failure, not something to paper over. + - Parse the decision: the happy path is the default. If an `IMPLEMENTATION_BLOCKER:` line is present, the decision is `BLOCKED` and the blocker text is written to a file. Otherwise the decision is `IMPLEMENT`; the `IMPLEMENTATION_RESULT:` line is required and contains a JSON object with `title` and `body` fields — the title is a compact PR title and the body is the full markdown PR description. The reasoning is a core part of the adversarial approach, so its absence is a genuine failure, not something to paper over. 7. Cleanup: remove `.opencode/agents/probe-issue.md` and `.opencode/tools/run_tox.ts` and `.opencode/tools/fetch_url.ts` so they do not appear as changed paths. 8. If `BLOCKED`: comment on the issue with the blocker reason. Done. 9. If `IMPLEMENT`: enforce changed paths (inline bash in the YAML, not a Python file the agent could tamper with). @@ -66,7 +66,7 @@ The `fetch_url` tool is the other exception: it lets the agent fetch content fro - Verify `.git/` was not modified (checks `git diff` and `git ls-files` for `.git/` paths). Reject if any `.git/` files were changed — this prevents the agent from planting hooks that would fire during `git add` or `git push`. 10. Configure git credentials using `GITHUB_TOKEN` — only now, after enforcement passes and the agent has exited. 11. `git add --all`, commit, push branch `probe/issue-`. -12. `gh pr create` with the first line of the agent's reasoning as the title, the reasoning file as the PR body. The body does not include `Closes #`. GitHub requires approval before running CI workflows on PRs created by `GITHUB_TOKEN`. +12. `gh pr create` with the title and body from the agent's `IMPLEMENTATION_RESULT` JSON output. The body does not include `Closes #`. GitHub requires approval before running CI workflows on PRs created by `GITHUB_TOKEN`. 13. Comment on the issue with the result (PR link, blocker, or failure message). This step always runs. ## Prompt composition @@ -78,7 +78,7 @@ Six sections, composed by the Python script: 3. Charm development context: project structure, dependency management (including how to pin versions via `pyproject.toml` and `run_tox`'s `uv lock`), `run_tox` scope, unit test patterns, integration test patterns, and linting conventions. This gives the agent the toolchain knowledge it needs without having to reverse-engineer it by reading files. 4. Task instructions: the adversarial testing strategy (see below), including guidance to not fetch URLs (docs are already in the prompt), not read infrastructure files, limit exploration to 10 files, and handle version-dependent claims. 5. Untrusted content: issue title, body, comments, fetched docs, all wrapped in `` markers. -6. Output contract: the happy path is the default — if the agent makes file changes, the workflow treats that as `IMPLEMENT` and proceeds to path enforcement, no marker required. The agent only emits `IMPLEMENTATION_BLOCKER: ` when it cannot proceed. When implementing, `IMPLEMENTATION_REASONING:` is required — a concise chain of reasoning for the PR body, written in plain conversational English (see Voice below). The reasoning is a core part of the adversarial approach: the reviewer needs it to interpret the CI results, so the workflow fails the run if it is absent rather than opening a PR with a placeholder body. +6. Output contract: the happy path is the default — if the agent makes file changes, the workflow treats that as `IMPLEMENT` and proceeds to path enforcement, no marker required. The agent only emits `IMPLEMENTATION_BLOCKER: ` when it cannot proceed. When implementing, `IMPLEMENTATION_RESULT:` is required — a JSON object with a compact `title` and a markdown `body` for the PR description, written in plain conversational English (see Voice below). The reasoning is a core part of the adversarial approach: the reviewer needs it to interpret the CI results, so the workflow fails the run if it is absent rather than opening a PR with a placeholder body. The prompt is transported to OpenCode as a file (`--file prompt.md`), not as argv, to avoid OS argument length limits with large issue or docs content. @@ -99,7 +99,7 @@ Do not break existing tests. Modify charms and tests minimally to add the test. ## PR body -The PR title is the first line of the agent's reasoning (e.g. `foo happens when bar is integrated with baz`). +The PR title is a compact phrase from the agent's `IMPLEMENTATION_RESULT` JSON (e.g. `foo happens when bar is integrated with baz`). The PR body is the `body` field from the same JSON, formatted as markdown. The PR body must contain the chain of reasoning so a reviewer can interpret the CI results. The agent writes: what the doc claims, what it believes is true, what the PR tests, and what green (or red) CI means for the doc, in plain conversational English. The reasoning must cover both directions so the reviewer can interpret either outcome. For example: