feat(auto-agents): default-flip prefetch + preclone, teach task-implementor to use them (Phase 4)

Two complementary changes that should compound to ~260-400 s
wallclock reduction per implementer cycle, based on the live
PR #30 post-mortem.

1. tools/dispatch_implementer.py
   - _is_prefetch_enabled / _is_preclone_enabled now default
     to ON when the env var is unset or empty. Only the
     explicit falsy literals (0, false, no, off) opt out;
     this is the Phase 4 acceptance gate documented in
     docs/development/auto-agents-tier-2-3-plan.md § 5b.
   - New _env_falsy_explicit helper joins _env_truthy so the
     unset / empty-string / falsy-literal cases are handled
     symmetrically. Each predicate gained a docstring with
     the full behaviour matrix.

2. .opencode/agents/task-implementor.md
   - New "Pre-fetched context" table at the top of Main task
     listing every section the dispatcher may populate (PR
     description, diff, CI status, comments, reviews, linked
     issues, Epic, pre-cloned working copy) and which
     procedure step it short-circuits.
   - Each numbered step in pr_fix + issue_impl now leads with
     "If ## Pre-fetched ... is in your prompt, use it verbatim
     — skip the GET". The original GET / webfetch /
     git-isolator-util fallback is preserved for the explicit
     opt-out path AND for the transient-failure case where
     the dispatcher attempted the fetch but Forgejo returned
     500 (the section renders an "unavailable" placeholder).
   - This MD update is what makes the default-flip actually
     save wallclock — without it the worker would still
     curl for data already in its prompt.

Test updates:

- test_dispatch_implementer.py::TestPrefetchEnvFlag was
  reworked. The old test_legacy_prompt_when_flag_unset
  asserted the off-by-default behaviour we just inverted;
  it's replaced by two regression guards
  (test_prefetch_enabled_by_default_when_flag_unset +
  test_preclone_enabled_by_default_when_flag_unset) that
  pin the new default explicitly.
- test_falsy_values_disable_prefetch parametrize dropped
  "" (empty string falls through to default-ON, not falsy).
- New test_empty_string_falls_through_to_default pins the
  empty-string-equals-unset contract for both env vars.
- test_clone_section_no_handle_when_flag_disabled now sets
  IMPLEMENTER_DISPATCHER_PRECLONE=0 explicitly instead of
  relying on the unset-default that no longer means "off".
- test_implementer_prompt_snapshot.py autouse fixture
  switched from delenv to setenv("...", "0") so legacy-snapshot
  tests pin to the opt-out path. Tests that exercise the rich
  prompt continue to override to "1".

Full suite: 1062 passed, 3 skipped. Subagent archive walker
landed in dc968481 will capture the complete task-tool tree
from the next dispatcher cycle so we can measure the impact
end-to-end rather than from live-poll snapshots.

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
2026-05-10 21:30:22 -04:00
parent dc96848174
commit 555a3469a7
5 changed files with 226 additions and 26 deletions
+26 -8
View File
@@ -198,15 +198,33 @@ See the `implementer-helpers` skill (`SKILL.md`) for full subcommand documentati
### Main task
This is where actual implementation happens. Choose the appropriate procedure based on `work_type` from the subsections below:
This is where actual implementation happens. Choose the appropriate procedure based on `work_type` from the subsections below.
**Pre-fetched context (read the prompt FIRST, before any GET / webfetch / git-isolator-util call).** As of 2026-05-10 the dispatcher defaults to populating the prompt with pre-fetched Forgejo data and a pre-cloned working copy. Look for these section headings in your prompt:
| Section heading | What it carries | If present, skip this step |
|----------------------------------------------|-------------------------------------------------------|------------------------------------------------------------------|
| `## Pre-fetched PR description` | Title + full body of the PR (`pr_fix`, `request_changes_pr`) | The opening "Read the PR" GET below |
| `## Pre-fetched issue body` | Full body of the linked issue (`issue_impl`) | The opening "Read the issue" GET below |
| `## Pre-fetched diff` (between `BEGIN_PR_DIFF` / `END_PR_DIFF`) | Unified diff of changed files | Any "discover what changed" `read` / `grep` you'd otherwise do |
| `## Pre-fetched CI status` | Latest CI status + per-check detail | The opening "Fetch CI failure details" GET |
| `## Pre-fetched PR comments` / `## Pre-fetched issue comments` | Paginated comment history | The opening "Read all PR comments" GET |
| `## Pre-fetched active REQUEST_CHANGES reviews` | Active review-state-change feedback (`request_changes_pr`) | The opening "Read all reviews" GET |
| `## Pre-fetched linked issues` | Body + metadata of issues this PR closes / refs | A GET on each `Closes #N` target |
| `## Pre-fetched Epic` | Body of the parent Epic (when `Epic: #N` in body) | A GET on the Epic |
| `## Pre-cloned working copy` | `repo_dir` of a working clone the dispatcher set up | The `git-isolator-util` call in step 3 / 5 below |
If a section heading appears but its body says **"The dispatcher attempted to pre-fetch this section but it was not available"**, the fetch failed (transient API error, network blip) — fall through to the original GET / webfetch / `git-isolator-util` step. The dispatcher's `## Data completeness` block (when present) lists the specific failures so you know which sections to backfill. If a section heading is **entirely absent** from your prompt, an operator has explicitly opted out via `IMPLEMENTER_DISPATCHER_PREFETCH=0` / `IMPLEMENTER_DISPATCHER_PRECLONE=0` (typically for a bisect or rollback) — also fall through to the original GET-based step.
This is a **performance** change, not a correctness change: the pre-fetched data is functionally identical to what you'd `curl` yourself. Reading the prompt costs zero tool calls. Re-fetching data already in the prompt burns ~10-15 s per redundant GET and was the dominant cost in the pre-2026-05-10 implementer post-mortems.
#### Procedure: `issue_impl` (New Issue Implementation)
1. **Read the issue.** GET `{forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/issues/{work_number}` — read title, body, labels, milestone, and metadata section (branch name, commit message format). Paginate all comments to understand full context and any subtask structure.
1. **Read the issue.** **If `## Pre-fetched issue body` is in your prompt, use it verbatim — skip the GET.** Otherwise, GET `{forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/issues/{work_number}` — read title, body, labels, milestone, and metadata section (branch name, commit message format). Paginate all comments to understand full context and any subtask structure (or use `## Pre-fetched issue comments` when present).
2. **Determine branch name.** Extract the branch name from the issue's Metadata section if present. If absent, derive one: `feature/issue-{work_number}-{kebab-slug-of-title}`.
3. **Create isolated clone.** Call `git-isolator-util` with `create_branch: true`, `base_branch: master`, and the determined `branch_name` (see Subagents section for prompt template).
3. **Create isolated clone.** **If `## Pre-cloned working copy` is present with a non-empty `repo_dir`, use that path verbatim — the dispatcher has already cloned and checked out the branch for you. Skip the `git-isolator-util` call entirely.** Otherwise, call `git-isolator-util` with `create_branch: true`, `base_branch: master`, and the determined `branch_name` (see Subagents section for prompt template).
4. **Implement the code.** Load the `cleverthis-guidelines` skill for CONTRIBUTING.md rules and follow them strictly. Key rules:
- Source in `src/cleveragents/`, Behave unit tests in `features/`, Robot Framework integration/e2e tests in `robot/`
@@ -248,15 +266,15 @@ This is where actual implementation happens. Choose the appropriate procedure ba
#### Procedure: `pr_fix` (PR Fix)
1. **Read the PR.** GET `{forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/pulls/{work_number}` — read description, head branch, head SHA, and CI state. Set `branch_name` to the PR's head branch.
1. **Read the PR.** **If `## Pre-fetched PR description` is in your prompt, use it verbatim — read description, head branch, head SHA, and CI state from there. Skip the GET.** Otherwise, GET `{forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/pulls/{work_number}` — read description, head branch, head SHA, and CI state. Set `branch_name` to the PR's head branch (or to the `head_ref` in the pre-fetched section).
2. **Read all reviews.** GET `{forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/pulls/{work_number}/reviews?limit=50&page=N` — paginate fully. For any review in `REQUEST_CHANGES` state, GET its comments to understand the specific feedback.
2. **Read all reviews.** **If `## Pre-fetched active REQUEST_CHANGES reviews` is in your prompt, use it verbatim — it already includes per-review comments. Skip the GET.** Otherwise, GET `{forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/pulls/{work_number}/reviews?limit=50&page=N` — paginate fully. For any review in `REQUEST_CHANGES` state, GET its comments to understand the specific feedback.
3. **Read all PR comments.** GET `{forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/issues/{work_number}/comments?limit=50&page=N` — paginate fully.
3. **Read all PR comments.** **If `## Pre-fetched PR comments` is in your prompt, use it verbatim. Skip the GET.** Otherwise, GET `{forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/issues/{work_number}/comments?limit=50&page=N` — paginate fully.
4. **Fetch CI failure details.** GET `{forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/commits/{head_sha}/statuses?limit=50&page=N` — paginate fully. For each failing status that has a `target_url`, webfetch that URL to retrieve the failure logs.
4. **Fetch CI failure details.** **If `## Pre-fetched CI status` and `## Pre-fetched CI per-check detail` are in your prompt, use them verbatim — the per-check section already carries failure URLs and (when the dispatcher could reach them) the fetched failure-log excerpts. Skip both the GET and the per-status `webfetch`.** Otherwise, GET `{forgejo_url}/api/v1/repos/{forgejo_owner}/{forgejo_repo}/commits/{head_sha}/statuses?limit=50&page=N` — paginate fully. For each failing status that has a `target_url`, webfetch that URL to retrieve the failure logs.
5. **Create isolated clone.** Call `git-isolator-util` with `create_branch: false` and `branch: {branch_name}` (the PR's head branch, resolved in step 1).
5. **Create isolated clone.** **If `## Pre-cloned working copy` is present with a non-empty `repo_dir`, use that path verbatim — the dispatcher has already cloned the PR's head branch and checked it out for you. Skip the `git-isolator-util` call entirely; proceed to step 6 with `repo_dir` taken from the pre-cloned section.** Otherwise, call `git-isolator-util` with `create_branch: false` and `branch: {branch_name}` (the PR's head branch, resolved in step 1).
6. **Fix the issues.** Address all CI failures and all unresolved reviewer feedback. Never partially address reviewer comments — every `REQUEST_CHANGES` concern must be fully resolved.
+81
View File
@@ -97,6 +97,87 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
### Changed
- **Default-flip `IMPLEMENTER_DISPATCHER_PREFETCH` and
`IMPLEMENTER_DISPATCHER_PRECLONE` to ON (2026-05-10, Phase 4
acceptance).** Both env-var flags now default to ON when unset
(matching the auto-agents-tier-2-3-plan § 5b acceptance gate).
Operators who need the legacy title-only prompt / in-session
`git-isolator-util` clone for a bisect or rollback set the
corresponding env var to `0` explicitly. Behaviour summary:
| env var value | `_is_prefetch_enabled()` | Notes |
|---|---|---|
| unset | True | new default (post-flip) |
| empty string `""` | True | falls through to default |
| `0` / `false` / `no` / `off` | False | explicit opt-out |
| `1` / `true` / `yes` / `on` | True | explicit opt-in |
Same matrix applies to `IMPLEMENTER_DISPATCHER_PRECLONE`. The
``_env_truthy`` helper was renamed in spirit (kept as-is for
the truthy-side path) and joined by a new
``_env_falsy_explicit`` helper that only treats the four
falsy literals as opt-outs — empty string and unset both
fall through to the new default.
Why this matters: the post-mortem of the PR #30 implementer
runs showed that without prefetch the ``task-implementor``
subagent spent the first ~3-4 minutes on curl + grep + read
calls just to discover head SHA, base branch, and changed
files — all of which the dispatcher already has from its own
Forgejo calls. The pre-clone elimination saves another
~60-100 s for ``pr_fix`` work by skipping the
``git-isolator-util`` subagent's clone-then-checkout dance.
Combined estimated saving from one cycle: ~260-400 s.
Concretely:
- ``tools/dispatch_implementer.py``: ``_is_prefetch_enabled``
and ``_is_preclone_enabled`` rewritten to invert the
default. Each grew a docstring with the full behaviour
matrix.
- ``tests/auto_agents/test_implementer_prompt_snapshot.py``:
the autouse fixture switched from ``delenv`` to
``setenv("...", "0")`` so legacy-snapshot tests pin to the
opt-out path explicitly. Tests that exercise the rich
prompt continue to override with ``setenv("...", "1")``.
- ``tests/auto_agents/test_dispatch_implementer.py``:
``test_legacy_prompt_when_flag_unset`` was reworked into
two regression guards
(``test_prefetch_enabled_by_default_when_flag_unset`` +
``test_preclone_enabled_by_default_when_flag_unset``) that
pin the new default. The
``test_falsy_values_disable_prefetch`` parametrize dropped
`""` (empty string is no longer a falsy opt-out). A new
``test_empty_string_falls_through_to_default`` test pins
the empty-string-equals-unset contract for both flags.
``test_clone_section_no_handle_when_flag_disabled`` now
sets ``IMPLEMENTER_DISPATCHER_PRECLONE=0`` explicitly
instead of relying on the unset-default that no longer
means "off".
- **task-implementor.md: read the prompt FIRST before any
Forgejo GET / webfetch / `git-isolator-util` call (2026-05-10).**
The MD now includes a "Pre-fetched context" table at the top
of the Main task section listing every prompt section the
dispatcher may populate (PR description, diff, CI status,
comments, reviews, linked issues, Epic, pre-cloned working
copy) and which procedure step that section short-circuits.
Each numbered step in the ``pr_fix`` and ``issue_impl``
procedures now leads with **"If `## Pre-fetched ...` is in
your prompt, use it verbatim — skip the GET"**. The fallback
to the original GET-based step is preserved for the explicit
opt-out path (``IMPLEMENTER_DISPATCHER_PREFETCH=0``) and for
the transient-failure case where the dispatcher attempted
the fetch but couldn't reach Forgejo (the section renders an
``"unavailable"`` placeholder).
Why this matters: without this MD update the prefetched
data would land in the prompt but the worker would still
``curl`` for it anyway — defeating the purpose of the
default-flip above. The two changes are complementary; the
``task-implementor.md`` change is what makes the
``dispatch_implementer.py`` change actually save wallclock.
- **Tighten `implementation-worker.md` skill allowlist and Step 0
(2026-05-10).** Post-P0 rerun against PR #30 showed the wrapper
still loading the heavier `auto-agents-system` skill (~29 KB,
+56 -11
View File
@@ -141,10 +141,35 @@ def _issue_item(number: int = 42, title: str = "Add JWT refresh") -> dict[str, A
class TestPrefetchEnvFlag:
def test_legacy_prompt_when_flag_unset(
self, driver, cfg, monkeypatch
def test_prefetch_enabled_by_default_when_flag_unset(
self, driver, monkeypatch
):
"""Post-2026-05-10: the default is **ON**. An unset env var
falls through to the dispatcher's documented default (rich
prefetched prompt). This is the regression guard for the
default-flip itself — if a future refactor accidentally
re-introduces the off-by-default behaviour, this test fails
loudly. Tests that need the legacy prompt set the env var to
``"0"`` explicitly (see ``test_legacy_prompt_when_flag_falsy``
below)."""
monkeypatch.delenv("IMPLEMENTER_DISPATCHER_PREFETCH", raising=False)
assert driver._is_prefetch_enabled() is True
def test_preclone_enabled_by_default_when_flag_unset(
self, driver, monkeypatch
):
"""Symmetric to the prefetch default-flip: pre-clone is also
ON when the env var is unset post-2026-05-10."""
monkeypatch.delenv("IMPLEMENTER_DISPATCHER_PRECLONE", raising=False)
assert driver._is_preclone_enabled() is True
def test_legacy_prompt_when_flag_falsy(self, driver, cfg, monkeypatch):
"""Explicit ``IMPLEMENTER_DISPATCHER_PREFETCH=0`` (the
opt-out path for a bisect / rollback / dry-run) restores
the legacy title-only prompt. The legacy code path must
continue to work for the foreseeable future — this is the
rollback safety net."""
monkeypatch.setenv("IMPLEMENTER_DISPATCHER_PREFETCH", "0")
item = _pr_item()
group = driver.WORK_GROUPS[0]
prompt = driver._implementation_prompt_dispatch(cfg, item, group)
@@ -152,13 +177,6 @@ class TestPrefetchEnvFlag:
assert "PR Compliance Checklist" in prompt
assert "_dispatcher_implementer_context" not in item
def test_legacy_prompt_when_flag_falsy(self, driver, cfg, monkeypatch):
monkeypatch.setenv("IMPLEMENTER_DISPATCHER_PREFETCH", "0")
item = _pr_item()
group = driver.WORK_GROUPS[0]
prompt = driver._implementation_prompt_dispatch(cfg, item, group)
assert "Pre-fetched" not in prompt
def test_prefetch_prompt_when_flag_truthy(
self, driver, cfg, monkeypatch, fake_implementer_api
):
@@ -183,14 +201,36 @@ class TestPrefetchEnvFlag:
@pytest.mark.parametrize("value", ["1", "true", "TRUE", "yes", "on"])
def test_truthy_values_enable_prefetch(self, driver, monkeypatch, value):
"""Truthy literals still flip prefetch ON. Backwards-compatible
with the pre-flip ``_env_truthy`` contract — operators who set
the env var explicitly continue to get the expected behaviour
regardless of whether the default changed."""
monkeypatch.setenv("IMPLEMENTER_DISPATCHER_PREFETCH", value)
assert driver._is_prefetch_enabled() is True
@pytest.mark.parametrize("value", ["0", "false", "no", "off", ""])
@pytest.mark.parametrize("value", ["0", "false", "no", "off"])
def test_falsy_values_disable_prefetch(self, driver, monkeypatch, value):
"""Only the **explicit** falsy literals opt out of prefetch.
Empty string / unset fall through to the default-ON path — see
``test_prefetch_enabled_by_default_when_flag_unset`` above and
``test_empty_string_falls_through_to_default`` below for the
positive-control tests."""
monkeypatch.setenv("IMPLEMENTER_DISPATCHER_PREFETCH", value)
assert driver._is_prefetch_enabled() is False
def test_empty_string_falls_through_to_default(self, driver, monkeypatch):
"""An empty-string env var (e.g. ``IMPLEMENTER_DISPATCHER_PREFETCH=``
in a ``.env`` file) is NOT a falsy opt-out — it falls through
to the default-ON path. This matches Python's ``os.environ``
convention where ``""`` and unset are routinely conflated, and
avoids surprising an operator who exports an empty-string
value (the typical mistake is ``export FOO=`` rather than
``unset FOO``)."""
monkeypatch.setenv("IMPLEMENTER_DISPATCHER_PREFETCH", "")
assert driver._is_prefetch_enabled() is True
monkeypatch.setenv("IMPLEMENTER_DISPATCHER_PRECLONE", "")
assert driver._is_preclone_enabled() is True
class TestPrFixPrefetch:
def test_pr_fix_prompt_contains_all_sections(
@@ -449,8 +489,13 @@ class TestPreclone:
def test_clone_section_no_handle_when_flag_disabled(
self, driver, cfg, monkeypatch, fake_implementer_api
):
# Post-2026-05-10 the preclone default is ON; "disabled" here
# means the explicit ``=0`` opt-out path rather than the
# historical "unset → off" assumption. Without this explicit
# set, the test would silently invoke the real
# ``prepare_pr_worktree`` and break in CI.
monkeypatch.setenv("IMPLEMENTER_DISPATCHER_PREFETCH", "1")
monkeypatch.delenv("IMPLEMENTER_DISPATCHER_PRECLONE", raising=False)
monkeypatch.setenv("IMPLEMENTER_DISPATCHER_PRECLONE", "0")
stub_forgejo_pr_details(fake_implementer_api)
stub_forgejo_ci_status(fake_implementer_api, state="success")
stub_forgejo_pr_comments(fake_implementer_api)
@@ -73,11 +73,19 @@ def cfg(tmp_path):
@pytest.fixture(autouse=True)
def _disable_flags(monkeypatch):
"""Each test in this module asserts on the legacy / prefetch /
preclone branch its scenario describes; reset the flags between
tests so an unset env var doesn't leak prefetch state from a
sibling test."""
monkeypatch.delenv("IMPLEMENTER_DISPATCHER_PREFETCH", raising=False)
monkeypatch.delenv("IMPLEMENTER_DISPATCHER_PRECLONE", raising=False)
preclone branch its scenario describes; pin the flags to ``0``
(explicit opt-out) by default so legacy-snapshot tests don't
silently flip when the dispatcher's default behaviour changes.
Pre-flip (before 2026-05-10) this fixture used ``delenv`` because
"unset" meant "legacy off". Post-flip the default is ON, so we
have to be explicit: tests that want the rich prompt override
with ``setenv("...", "1")``, tests that want legacy inherit the
``"0"`` set here, and the dispatcher's default-flip is verified
by a dedicated test in ``test_dispatch_implementer.py``.
"""
monkeypatch.setenv("IMPLEMENTER_DISPATCHER_PREFETCH", "0")
monkeypatch.setenv("IMPLEMENTER_DISPATCHER_PRECLONE", "0")
yield
+50 -2
View File
@@ -77,12 +77,60 @@ def _env_truthy(name: str) -> bool:
return os.environ.get(name, "").strip().lower() in _TRUTHY_ENV_VALUES
_FALSY_ENV_VALUES = frozenset({"0", "false", "no", "off"})
def _env_falsy_explicit(name: str) -> bool:
"""``True`` when ``name`` is set to one of the falsy literals.
Used by :func:`_is_prefetch_enabled` and
:func:`_is_preclone_enabled` to support the default-ON / opt-out
model (set ``IMPLEMENTER_DISPATCHER_PREFETCH=0`` to explicitly
disable the rich prompt path for one cycle). An unset variable is
NOT falsy here it falls through to the default.
"""
return os.environ.get(name, "").strip().lower() in _FALSY_ENV_VALUES
def _is_prefetch_enabled() -> bool:
return _env_truthy(PREFETCH_ENV_VAR)
"""Return ``True`` when the dispatcher should use the rich
pre-fetched prompt (PR description, diff, CI status, comments,
linked issues, etc.).
**Default since 2026-05-10: ON.** The post-mortem of the
PR #30 implementer runs showed that without prefetch the
``task-implementor`` subagent burns ~3-4 minutes on
``curl`` + ``read`` calls just to discover the PR's head SHA,
base branch, and changed files all of which the dispatcher
already has from its own Forgejo calls. The rich prompt
eliminates those turns entirely. The legacy title-only prompt
remains available behind an explicit
``IMPLEMENTER_DISPATCHER_PREFETCH=0`` opt-out (e.g. for
bisecting a regression against the pre-Phase-4 behaviour).
"""
if _env_falsy_explicit(PREFETCH_ENV_VAR):
return False
return True
def _is_preclone_enabled() -> bool:
return _env_truthy(PRECLONE_ENV_VAR)
"""Return ``True`` when the dispatcher should pre-clone the PR's
head into ``/tmp/<repo>`` and pass the working-copy path to the
worker.
**Default since 2026-05-10: ON.** Symmetric to
:func:`_is_prefetch_enabled`: the pre-clone eliminates the
``git-isolator-util`` subagent's clone-then-checkout dance for
``pr_fix`` work (the typical ~60-100 s of network + git I/O).
The worker's prompt carries the ``## Pre-cloned working copy``
section with a working ``repo_dir`` path so the worker proceeds
directly to the patch step. Set
``IMPLEMENTER_DISPATCHER_PRECLONE=0`` to fall back to the
in-session clone via ``git-isolator-util``.
"""
if _env_falsy_explicit(PRECLONE_ENV_VAR):
return False
return True
# ─── Legacy prompt (preserved for ``IMPLEMENTER_DISPATCHER_PREFETCH=0``) ────