feat(auto-agents): sentinel-routed deterministic sections + masking hedge
Five rounds of fresh-eyes review against the b154d480 deterministic
worker-side improvements landed four substantive corrections:
- Sentinel handoff for compliance + preflight (workers read off
disk via implementer_pr_context.py --field; survives tier-agent
prompt summarisation)
- _GATE_STATUS_RE fixed to match real `## [unit_tests] PASS (12s)`
output from local_ci_gate.sh (previously zero gates parsed)
- Preflight timeout returns explicit sentinel (-9999) and the
orchestrator zeros counts so renderer can't show "Persistent
failures: N" alongside the timeout warning
- outcome_synthesised plumbed per-attempt through phase 4 telemetry
Plus a fresh-eyes catch: check_worktree_clean / check_commit_has_issues_closed
return True on git rc != 0 (deliberate "don't conflate masking with
real gap"), but a fully-masked worktree coincidentally with valid
CHANGELOG + CONTRIBUTORS would have produced a confident "PR resolved"
verdict. New check_compliance_gaps_with_masking + masked_checks plumb
through the sentinel; both renderers (module-level + dispatcher's
pointer) hedge the verdict; worker docs instruct inspection of
masked_checks before exiting resolved.
Two new env flags, both default-ON kill-switches:
- IMPLEMENTER_OUTCOME_SYNTHESIS (=0 reverts to UNKNOWN-bucket)
- IMPLEMENTER_COMPLIANCE_GAPS_ENABLED (=0 disables compliance scan;
AND-coupled with escalation since compliance is meaningless
outside the gap-filling flow)
Byte-equivalence holds when all new flags are unset. Test suite
1432 passed, 3 skipped (+~70 new tests across compliance, masking,
synthesis-across-loop, sentinel round-trip, regex fixes, fixtures).
ISSUES CLOSED: #30
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -330,7 +330,14 @@ The script emits ONE of three signals on stdout (always exit 0):
|
||||
|
||||
The exact authoritative-empty byte sequence depends on the field's native shape: `epic` and the plain-text fields (`description`, `title`, `diff`, `issue_body`) emit `null`; list fields (`comments`, `reviews`, `issues`) emit `[]`; `metadata` and `ci` always emit a JSON object when the sentinel exists at all (inspect the embedded `*_completed` / `data_complete` flags to decide whether the value is authoritative).
|
||||
|
||||
`<field>` is one of: `description`, `issue_body`, `metadata`, `diff`, `ci`, `comments`, `reviews`, `issues`, `epic`. See the `implementer-pr-context` skill's SKILL.md for the per-field output schema and worked examples.
|
||||
`<field>` is one of: `description`, `issue_body`, `metadata`, `diff`, `ci`, `comments`, `reviews`, `issues`, `epic`, `compliance_gaps`, `gate_preflight`. See the `implementer-pr-context` skill's SKILL.md for the per-field output schema and worked examples.
|
||||
|
||||
**Deterministic check sections (read these first).** When your prompt mentions a `## Compliance gap report` or `## Pre-flight gate summary` stanza, the AUTHORITATIVE data lives in two extra sentinel fields the dispatcher computes mechanically against the pre-cloned worktree:
|
||||
|
||||
- `--field compliance_gaps` returns the dict `{gaps: {worktree_clean, changelog_unreleased_nonempty, contributors_has_author, commit_has_issues_closed}, gaps_open_count, masked_checks, pr_number, git_user_email}`. **Inspect `masked_checks` BEFORE acting on `gaps`.** It's a list of check names where the underlying `git` call failed; for those keys, the dispatcher returned `true` to avoid conflating "couldn't check" with "real gap", but the value is unverified. If `masked_checks` is non-empty, do NOT emit `{"outcome": "resolved"}` even when every value in `gaps` is `true` — re-run `git status` and `git log -1` in-session to confirm the masked check(s) before deciding. When `masked_checks` is empty AND every value in `gaps` is `true`, the PR is complete — emit `{"outcome": "resolved", "files_touched": []}` and exit. When some `gaps` values are `false`, fill ONLY the missing items; do NOT re-touch the code fix in HEAD.
|
||||
- `--field gate_preflight` returns `{gate_statuses, failures_total, related, unrelated, runs, preflight_enabled, preflight_timeout?, flakes_filtered?}`. If `preflight_timeout` is `true`, treat every in-session gate failure as potentially real (the dispatcher's classification is unreliable). Otherwise `unrelated` failures are environmental — do NOT bail on the cycle for them; focus on `related` failures (if any) and compliance gaps.
|
||||
|
||||
Both fields fall back to empty stdout when the dispatcher did NOT compute them this cycle (flag off). In that case proceed with your normal in-session discovery — no special handling required.
|
||||
|
||||
**Why the three-case contract matters.** The naive "empty vs non-empty" reading would cause you to redundantly re-curl Forgejo every time the dispatcher confirmed a section was empty (e.g. PR has no Epic, no active REQUEST_CHANGES reviews). The middle case (authoritative-empty bytes) is the dispatcher's "I checked and there's nothing" — burn no wallclock fetching what is already known absent.
|
||||
|
||||
|
||||
@@ -67,6 +67,8 @@ python3 tools/implementer_pr_context.py read --pr {work_number} --field {field}
|
||||
| `epic` | JSON dict | when you need the parent Epic's body |
|
||||
| `diff` | plain text | redundant with `BEGIN_PR_DIFF` in your prompt, but useful if your prompt was clipped |
|
||||
| `issue_body` | plain text | step 1 of `issue_impl` (the linked issue's body) |
|
||||
| `compliance_gaps` | JSON dict | when your prompt mentions a "Compliance gap report" — pulls the deterministic dict of `{worktree_clean, changelog_unreleased_nonempty, contributors_has_author, commit_has_issues_closed}` (each strict `bool`) plus `gaps_open_count` and `masked_checks` (list of check names where `git` itself failed and the value was masked to `true`). **Always inspect `masked_checks` before acting on the `gaps` dict** — when non-empty, do NOT emit `{"outcome": "resolved"}` even if every gap value is `true`; re-verify the masked check(s) in-session first. |
|
||||
| `gate_preflight` | JSON dict | when your prompt mentions "Pre-flight gate summary" — pulls the persistent-failure classification (`{gate_statuses, failures_total, related, unrelated, runs, preflight_timeout?, flakes_filtered?}`) |
|
||||
| `all` | full JSON payload | debugging only — pull narrowly for normal use |
|
||||
|
||||
## How to consume the output (three-case contract)
|
||||
|
||||
@@ -70,6 +70,81 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
|
||||
dry-run / when no preclone worktree exists. Flag-off path is
|
||||
byte-equivalent to the pre-feature build.
|
||||
|
||||
- **Sentinel-routed deterministic sections + masking-aware hedge
|
||||
(2026-05-13, post-pilot follow-up to the above).** Five rounds
|
||||
of fresh-eyes review against the original deterministic
|
||||
improvements surfaced four substantive corrections plus polish:
|
||||
|
||||
- **Sentinel handoff for compliance + preflight.** The two
|
||||
deterministic sections are written into the existing PR-context
|
||||
sentinel under `compliance_gaps` / `gate_preflight` keys
|
||||
(`tools/_pr_context_sentinel.write` accepts new optional kwargs;
|
||||
`tools/implementer_pr_context.py` exposes matching
|
||||
`--field compliance_gaps` / `--field gate_preflight`). Workers
|
||||
read the authoritative data off disk so the intermediate
|
||||
tier-agent prompt summarisation can't strip it — the
|
||||
markdown stanza in the prompt body is now a short pointer +
|
||||
summary, not the load-bearing copy. Binary completion-flag
|
||||
contract (`{compliance_gaps,gate_preflight}_completed`)
|
||||
distinct from the prefetch fields' tri-state contract.
|
||||
|
||||
- **Gate-status regex fix.** `_diff_aware_gate._GATE_STATUS_RE`
|
||||
now matches the real `## [unit_tests] PASS (12s)` format that
|
||||
`tools/local_ci_gate.sh` emits. Previously expected
|
||||
`unit_tests: PASS` which no script ever produced — every
|
||||
gate parsed as missing and the worker saw an empty roll-up.
|
||||
Real-output fixture test pinned against
|
||||
`local_ci_gate.sh:323-340`.
|
||||
|
||||
- **Pre-flight timeout surfacing.** `_run_gate_once` returns a
|
||||
distinctive `_TIMEOUT_RETURNCODE = -9999` sentinel on
|
||||
`subprocess.TimeoutExpired` (well outside POSIX signal range).
|
||||
The orchestrator surfaces `preflight_timeout=True` and zeros
|
||||
the failure counts so the renderer cannot show contradictory
|
||||
"Persistent failures: N" alongside the timeout warning.
|
||||
Previously a wedged gate run silently rendered as "no
|
||||
failures persisted" — a green light on a broken signal.
|
||||
|
||||
- **Per-attempt `outcome_synthesised` telemetry.** When the
|
||||
worker exits without emitting JSON, the dispatcher's
|
||||
`_synthesize_outcome_if_missing` returns a synthesised verdict
|
||||
AND a flag now plumbed through `_record_attempt → _record_phase4_telemetry`
|
||||
onto each per-attempt JSONL row. Analysts distinguish
|
||||
worker-reported outcomes from dispatcher-inferred ones.
|
||||
|
||||
- **Compliance masking hedge.** `check_worktree_clean` and
|
||||
`check_commit_has_issues_closed` return `True` on
|
||||
`git rc != 0` (deliberate policy — don't conflate "couldn't
|
||||
check" with "real gap"), but a fully-masked worktree
|
||||
coincidentally with valid CHANGELOG + CONTRIBUTORS would have
|
||||
produced a confident "PR resolved, do not re-apply" directive
|
||||
to the worker. New `check_compliance_gaps_with_masking` returns
|
||||
`(gaps, masked_set)`; dispatcher stores `masked_checks` in the
|
||||
sentinel; both renderers (`tools/_implementer_compliance.render_prompt_stanza`
|
||||
and `dispatch_implementer._render_compliance_pointer_stanza`)
|
||||
hedge the "all passed" verdict when masked is non-empty.
|
||||
`task-implementor.md` + `implementer-pr-context` SKILL.md
|
||||
instruct the worker to inspect `masked_checks` before acting.
|
||||
|
||||
- **New env flags.** `IMPLEMENTER_OUTCOME_SYNTHESIS` (default-on
|
||||
kill-switch; set to 0 to revert to UNKNOWN-bucket behaviour
|
||||
at the cost of one wasted same-tier retry per cycle).
|
||||
`IMPLEMENTER_COMPLIANCE_GAPS_ENABLED` (default-on opt-out
|
||||
when the escalation flag is on; AND-coupled with escalation
|
||||
since compliance is meaningless outside the gap-filling flow).
|
||||
Both follow the falsy-explicit-opt-out pattern.
|
||||
|
||||
- **Misc.** Compliance regex tightened to exact `[Unreleased]`
|
||||
bracket alternation (rejects mismatched `## [Unreleased`).
|
||||
Hyphen/underscore feature-stem heuristic widened with negative
|
||||
tests guarding against substring over-matching. `gaps_open_count`
|
||||
accepts `masked_checks` and excludes masked keys from the count.
|
||||
`minimal_git_repo` conftest fixture hoists the 7-command
|
||||
init/commit/`gpgsign=false` block out of five inlined copies.
|
||||
|
||||
All new code paths are off-by-default and the byte-equivalence
|
||||
invariant holds when none of the new flags are set.
|
||||
|
||||
- **In-cycle tier escalation for the implementer dispatcher
|
||||
(2026-05-12).** Replaces the cross-cycle `auto/last-attempt-tier-N`
|
||||
scheme planned in `auto-agents-tier-2-3-plan.md § Phase 5c` with a
|
||||
|
||||
@@ -366,6 +366,54 @@ class GitRepoFixture:
|
||||
run: Callable[..., str]
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def minimal_git_repo(tmp_path):
|
||||
"""Minimal one-commit git repo for tests that only need HEAD to
|
||||
exist (compliance checks, deterministic-stanza integration tests).
|
||||
|
||||
Returns the repo path. Compared to :func:`real_git_repo` (which
|
||||
builds a feature-branch + master + tracking-ref scaffold for the
|
||||
review pipeline tests), this fixture is intentionally lean —
|
||||
seven git commands, all hermetic, and disables
|
||||
``commit.gpgsign`` so dev machines with global signing enabled
|
||||
don't hit an interactive prompt or commit failure.
|
||||
|
||||
The fixture was hoisted out of per-file scope after the
|
||||
2026-05-13 third-round review observed the same init/commit
|
||||
block copy-pasted across four integration tests plus one unit
|
||||
test fixture. Future tests that need a "valid git repo with one
|
||||
commit" should request this fixture rather than re-inlining the
|
||||
setup — keeps the `commit.gpgsign=false` invariant in one place.
|
||||
|
||||
Skipped if ``git`` is not on PATH (CI runners that strip it).
|
||||
"""
|
||||
import shutil
|
||||
if shutil.which("git") is None:
|
||||
pytest.skip("git not on PATH")
|
||||
subprocess.run(["git", "init", "-q", str(tmp_path)], check=True)
|
||||
subprocess.run(
|
||||
["git", "-C", str(tmp_path), "config", "user.email", "test@example.com"],
|
||||
check=True,
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "-C", str(tmp_path), "config", "user.name", "Test"],
|
||||
check=True,
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "-C", str(tmp_path), "config", "commit.gpgsign", "false"],
|
||||
check=True,
|
||||
)
|
||||
(tmp_path / "README.md").write_text("# test\n")
|
||||
subprocess.run(
|
||||
["git", "-C", str(tmp_path), "add", "."], check=True,
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "-C", str(tmp_path), "commit", "-q", "-m", "initial"],
|
||||
check=True,
|
||||
)
|
||||
return tmp_path
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def real_git_repo(tmp_path):
|
||||
"""A tiny ``git init`` repo with master + feature branch + one
|
||||
|
||||
@@ -66,12 +66,21 @@ class TestParseFailingScenarios:
|
||||
|
||||
|
||||
class TestParseGateStatuses:
|
||||
def test_extracts_gate_statuses(self, gate):
|
||||
def test_extracts_gate_statuses_from_real_local_ci_format(self, gate):
|
||||
# Real-shape fixture: ``local_ci_gate.sh`` emits
|
||||
# ``## [gate] {start,PASS,FAIL,SKIP} (Ns)`` lines. The
|
||||
# parser must accept this shape (the previous regex
|
||||
# expected ``gate: STATUS`` which no local script produces,
|
||||
# silently dropping every gate from the roll-up).
|
||||
text = """
|
||||
lint: PASS
|
||||
typecheck: PASS
|
||||
unit_tests: FAIL (6 scenarios)
|
||||
integration_tests: SKIP
|
||||
## [lint] start
|
||||
## [lint] PASS (3s)
|
||||
## [typecheck] start
|
||||
## [typecheck] PASS (5s)
|
||||
## [unit_tests] start
|
||||
## [unit_tests] FAIL (47s)
|
||||
## [integration_tests] start
|
||||
## [integration_tests] SKIP (0s)
|
||||
"""
|
||||
statuses = gate.parse_gate_statuses(text)
|
||||
assert statuses == {
|
||||
@@ -82,10 +91,31 @@ integration_tests: SKIP
|
||||
}
|
||||
|
||||
def test_handles_mixed_case(self, gate):
|
||||
text = "Lint: pass\nUnit_Tests: Fail"
|
||||
text = "## [Lint] pass (1s)\n## [Unit_Tests] Fail (5s)"
|
||||
statuses = gate.parse_gate_statuses(text)
|
||||
assert statuses == {"lint": "PASS", "unit_tests": "FAIL"}
|
||||
|
||||
def test_ignores_start_lines(self, gate):
|
||||
# ``## [unit_tests] start`` should NOT be matched as a
|
||||
# verdict — only PASS/FAIL/SKIP count. Otherwise the
|
||||
# parser would record "unit_tests: START" before the real
|
||||
# verdict arrives.
|
||||
text = "## [unit_tests] start\n## [unit_tests] PASS (12s)"
|
||||
statuses = gate.parse_gate_statuses(text)
|
||||
assert statuses == {"unit_tests": "PASS"}
|
||||
|
||||
def test_parses_skip_status(self, gate):
|
||||
# ``SKIP`` is in the regex alternation for future-proofing
|
||||
# against a gate that signals skipped runs explicitly.
|
||||
# ``local_ci_gate.sh`` doesn't emit it today but the
|
||||
# parser MUST handle it the moment one does — pinning it
|
||||
# here keeps the alternation honest. Without this test,
|
||||
# a future "drop SKIP for simplicity" refactor would
|
||||
# silently regress.
|
||||
text = "## [coverage] SKIP (0s)"
|
||||
statuses = gate.parse_gate_statuses(text)
|
||||
assert statuses == {"coverage": "SKIP"}
|
||||
|
||||
|
||||
# ─── Relatedness heuristic ─────────────────────────────────────────
|
||||
|
||||
@@ -116,7 +146,7 @@ class TestRelatedness:
|
||||
# code. Tier 0 gave up; this classifier would have told it
|
||||
# "all unrelated, ignore them".
|
||||
text = """
|
||||
unit_tests: FAIL
|
||||
## [unit_tests] FAIL (47s)
|
||||
Failing scenarios:
|
||||
features/automation_profile_cli.feature:74
|
||||
features/cli_extensions.feature:358
|
||||
@@ -135,12 +165,64 @@ Failing scenarios:
|
||||
|
||||
def test_no_failures_with_passing_gates(self, gate):
|
||||
out = gate.classify_failures(
|
||||
"lint: PASS\ntypecheck: PASS\nunit_tests: PASS",
|
||||
"## [lint] PASS (3s)\n## [typecheck] PASS (5s)\n"
|
||||
"## [unit_tests] PASS (12s)",
|
||||
changed_files=["any.py"],
|
||||
)
|
||||
assert out["failures_total"] == 0
|
||||
assert out["gate_statuses"]["lint"] == "PASS"
|
||||
|
||||
def test_hyphenated_source_path_matches_underscored_feature(self, gate):
|
||||
# Hyphenated convention in source paths
|
||||
# (``plan-cli/commands.py``) should match the underscored
|
||||
# feature name (``plan_cli_commands.feature``). The
|
||||
# heuristic generates prefix candidates in BOTH the
|
||||
# underscored and hyphenated forms so the segment match
|
||||
# finds ``plan-cli`` inside the source path.
|
||||
out = gate.classify_failures(
|
||||
"Failing scenarios:\n features/plan_cli_commands.feature:42",
|
||||
changed_files=["src/plan-cli/commands.py"],
|
||||
)
|
||||
assert out["failures_related_to_diff"] == 1
|
||||
assert out["failures_unrelated_to_diff"] == 0
|
||||
|
||||
def test_underscored_feature_matches_hyphenated_source_segment(self, gate):
|
||||
# Genuine cross-convention transformation: feature filename
|
||||
# stem uses ONLY underscored convention; source path segment
|
||||
# uses ONLY hyphens. The two MUST normalise to a match via
|
||||
# the explicit hyphen-join in ``prefix_candidates``.
|
||||
#
|
||||
# Feature stem ``plan_cli_handlers`` tokenises to
|
||||
# ``["plan", "cli", "handlers"]``. The two-token prefix
|
||||
# ``plan_cli`` (underscored) doesn't appear in source
|
||||
# segments; only the hyphen-joined ``plan-cli`` does. Without
|
||||
# the hyphen-join in ``prefix_candidates`` this would not
|
||||
# match — pinning the contract that BOTH join forms are
|
||||
# generated.
|
||||
out = gate.classify_failures(
|
||||
"Failing scenarios:\n features/plan_cli_handlers.feature:7",
|
||||
changed_files=["src/plan-cli/handlers.py"],
|
||||
)
|
||||
assert out["failures_related_to_diff"] == 1
|
||||
assert out["failures_unrelated_to_diff"] == 0
|
||||
|
||||
def test_hyphenated_source_does_not_match_unrelated_feature(self, gate):
|
||||
# Negative-case companion to the above: a diff in
|
||||
# ``src/plan-cli/commands.py`` must NOT match an unrelated
|
||||
# ``cli_extensions.feature`` just because the source path
|
||||
# segment ``plan-cli`` happens to contain the substring
|
||||
# ``cli``. An earlier iteration over-expanded each path
|
||||
# segment into its sub-tokens (``["plan-cli", "plan", "cli"]``)
|
||||
# and this case classified as related — pinning the negative
|
||||
# result keeps a future expansion from regressing the same
|
||||
# way without conscious thought.
|
||||
out = gate.classify_failures(
|
||||
"Failing scenarios:\n features/cli_extensions.feature:1",
|
||||
changed_files=["src/plan-cli/commands.py"],
|
||||
)
|
||||
assert out["failures_related_to_diff"] == 0
|
||||
assert out["failures_unrelated_to_diff"] == 1
|
||||
|
||||
|
||||
# ─── Prompt rendering ──────────────────────────────────────────────
|
||||
|
||||
@@ -154,7 +236,8 @@ class TestRenderPromptStanza:
|
||||
|
||||
def test_all_gates_pass_renders_no_failures_message(self, gate):
|
||||
classification = gate.classify_failures(
|
||||
"lint: PASS\ntypecheck: PASS\nunit_tests: PASS\nintegration_tests: PASS",
|
||||
"## [lint] PASS (3s)\n## [typecheck] PASS (5s)\n"
|
||||
"## [unit_tests] PASS (12s)\n## [integration_tests] PASS (30s)",
|
||||
changed_files=["any.py"],
|
||||
)
|
||||
stanza = gate.render_prompt_stanza(classification)
|
||||
@@ -164,7 +247,7 @@ class TestRenderPromptStanza:
|
||||
|
||||
def test_mixed_failures_renders_both_buckets(self, gate):
|
||||
text = """
|
||||
unit_tests: FAIL
|
||||
## [unit_tests] FAIL (47s)
|
||||
Failing scenarios:
|
||||
features/langgraph_subscription.feature:10
|
||||
features/automation_profile_cli.feature:74
|
||||
|
||||
@@ -32,26 +32,15 @@ def compliance():
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def git_repo(tmp_path):
|
||||
"""A minimal git repo with one commit. Returns the path."""
|
||||
subprocess.run(["git", "init", "-q", str(tmp_path)], check=True)
|
||||
subprocess.run(
|
||||
["git", "-C", str(tmp_path), "config", "user.email", "test@example.com"],
|
||||
check=True,
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "-C", str(tmp_path), "config", "user.name", "Test"],
|
||||
check=True,
|
||||
)
|
||||
(tmp_path / "README.md").write_text("# test\n")
|
||||
subprocess.run(
|
||||
["git", "-C", str(tmp_path), "add", "."], check=True
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "-C", str(tmp_path), "commit", "-q", "-m", "initial"],
|
||||
check=True,
|
||||
)
|
||||
return tmp_path
|
||||
def git_repo(minimal_git_repo):
|
||||
"""Alias for the shared ``minimal_git_repo`` conftest fixture so
|
||||
the existing tests below read with their original name. The
|
||||
underlying setup (init + email/name config + ``commit.gpgsign=false``
|
||||
+ one commit) lives in :func:`tests.auto_agents.conftest.minimal_git_repo`
|
||||
— see that fixture's docstring for rationale (hoisted out of
|
||||
per-file scope after the 2026-05-13 third-round review observed
|
||||
the same block copy-pasted across five test files)."""
|
||||
return minimal_git_repo
|
||||
|
||||
|
||||
# ─── worktree_clean ────────────────────────────────────────────────
|
||||
@@ -69,10 +58,24 @@ class TestWorktreeClean:
|
||||
(git_repo / "new.py").write_text("print()\n")
|
||||
assert compliance.check_worktree_clean(git_repo) is False
|
||||
|
||||
def test_non_git_dir_returns_false(self, compliance, tmp_path):
|
||||
def test_non_git_dir_returns_true(self, compliance, tmp_path):
|
||||
# When ``git status`` fails (non-repo / missing binary),
|
||||
# the check returns True. Returning False would conflate
|
||||
# "couldn't check" with "real gap" and push the worker to
|
||||
# re-apply code on top of HEAD chasing a phantom dirty
|
||||
# worktree. Structural git problems are the dispatcher's
|
||||
# pre-clone responsibility, not the gap detector's.
|
||||
not_a_repo = tmp_path / "nope"
|
||||
not_a_repo.mkdir()
|
||||
assert compliance.check_worktree_clean(not_a_repo) is False
|
||||
assert compliance.check_worktree_clean(not_a_repo) is True
|
||||
|
||||
def test_non_git_dir_commit_has_issues_closed_returns_true(
|
||||
self, compliance, tmp_path,
|
||||
):
|
||||
# Same conflation-avoidance rule for the commit footer check.
|
||||
not_a_repo = tmp_path / "nope"
|
||||
not_a_repo.mkdir()
|
||||
assert compliance.check_commit_has_issues_closed(not_a_repo) is True
|
||||
|
||||
|
||||
# ─── changelog_unreleased_nonempty ─────────────────────────────────
|
||||
@@ -100,6 +103,75 @@ class TestChangelogUnreleased:
|
||||
)
|
||||
assert compliance.check_changelog_unreleased_nonempty(tmp_path) is True
|
||||
|
||||
def test_unreleased_without_brackets_returns_true(
|
||||
self, compliance, tmp_path,
|
||||
):
|
||||
# Bare-header convention (``## Unreleased`` with no brackets)
|
||||
# is widely used and was silently classified as missing by
|
||||
# the bracket-required regex before 2026-05-13.
|
||||
(tmp_path / "CHANGELOG.md").write_text(
|
||||
"# Changelog\n\n## Unreleased\n- New feature\n\n## 1.0.0\n"
|
||||
)
|
||||
assert compliance.check_changelog_unreleased_nonempty(tmp_path) is True
|
||||
|
||||
def test_unreadable_changelog_returns_false(
|
||||
self, compliance, tmp_path, monkeypatch,
|
||||
):
|
||||
# A CHANGELOG.md that cannot be decoded as UTF-8 should
|
||||
# treated as "no Unreleased section" rather than crashing
|
||||
# the gap detector. The check exists to cover the
|
||||
# OSError / UnicodeDecodeError except branch.
|
||||
(tmp_path / "CHANGELOG.md").write_bytes(b"\xff\xfe\xfd not utf8")
|
||||
assert (
|
||||
compliance.check_changelog_unreleased_nonempty(tmp_path) is False
|
||||
)
|
||||
|
||||
def test_mismatched_open_bracket_returns_false(
|
||||
self, compliance, tmp_path,
|
||||
):
|
||||
# ``## [Unreleased`` (missing closing bracket) is malformed
|
||||
# — the regex's exact alternation MUST NOT silently accept
|
||||
# it. An earlier ``\[?Unreleased\]?`` form accepted both
|
||||
# bracket forms regardless of mismatch; the strict
|
||||
# alternation rejects the open-only variant.
|
||||
(tmp_path / "CHANGELOG.md").write_text(
|
||||
"# Changelog\n\n## [Unreleased\n- bug fix\n\n## [1.0.0]\n"
|
||||
)
|
||||
assert (
|
||||
compliance.check_changelog_unreleased_nonempty(tmp_path) is False
|
||||
)
|
||||
|
||||
def test_close_bracket_after_bare_header_is_accepted(
|
||||
self, compliance, tmp_path,
|
||||
):
|
||||
# The close-only variant ``## Unreleased]`` IS accepted via
|
||||
# the bare-header arm of the regex — the trailing ``[^\n]*``
|
||||
# slurps the ``]`` exactly as it would slurp the dash-date
|
||||
# suffix in ``## Unreleased - 2026-01-01``. This pins the
|
||||
# documented asymmetry so a future regex tightening that
|
||||
# rejects this form (and risks over-flagging real Keep-a-
|
||||
# Changelog files with trailing decorations) fails loudly
|
||||
# rather than silently changing semantics.
|
||||
(tmp_path / "CHANGELOG.md").write_text(
|
||||
"# Changelog\n\n## Unreleased]\n- bug fix\n\n## 1.0.0\n"
|
||||
)
|
||||
assert (
|
||||
compliance.check_changelog_unreleased_nonempty(tmp_path) is True
|
||||
)
|
||||
|
||||
def test_bare_header_with_date_suffix_is_accepted(
|
||||
self, compliance, tmp_path,
|
||||
):
|
||||
# Companion positive test: the bare-header arm tolerates
|
||||
# arbitrary trailing decoration on the same line. The most
|
||||
# common real-world case is a date suffix.
|
||||
(tmp_path / "CHANGELOG.md").write_text(
|
||||
"# Changelog\n\n## Unreleased - 2026-01-01\n- bug fix\n"
|
||||
)
|
||||
assert (
|
||||
compliance.check_changelog_unreleased_nonempty(tmp_path) is True
|
||||
)
|
||||
|
||||
def test_unreleased_with_only_subheader_returns_false(self, compliance, tmp_path):
|
||||
(tmp_path / "CHANGELOG.md").write_text(
|
||||
"# Changelog\n\n## [Unreleased]\n\n### Added\n\n## [1.0.0]\n"
|
||||
@@ -195,6 +267,21 @@ class TestCommitHasIssuesClosed:
|
||||
)
|
||||
assert compliance.check_commit_has_issues_closed(git_repo) is True
|
||||
|
||||
def test_empty_repo_no_commits_returns_true(self, compliance, tmp_path):
|
||||
# Init a valid git repo but never commit. ``git log -1`` fails
|
||||
# with rc != 0 ("does not have any commits yet"). The new
|
||||
# permissive policy treats this as having-footer rather than
|
||||
# missing-footer — deliberately, because asking the worker to
|
||||
# amend a non-existent commit would waste the cycle. The
|
||||
# masked-failure log line is the operator-visible breadcrumb.
|
||||
subprocess.run(["git", "init", "-q", str(tmp_path)], check=True)
|
||||
subprocess.run(
|
||||
["git", "-C", str(tmp_path), "config", "commit.gpgsign", "false"],
|
||||
check=True,
|
||||
)
|
||||
# No commit made — HEAD is unborn.
|
||||
assert compliance.check_commit_has_issues_closed(tmp_path) is True
|
||||
|
||||
|
||||
# ─── Aggregate check + rendering ───────────────────────────────────
|
||||
|
||||
@@ -240,6 +327,112 @@ class TestAllGapsClosed:
|
||||
assert compliance.all_gaps_closed({}) is False
|
||||
|
||||
|
||||
class TestCheckComplianceGapsWithMasking:
|
||||
def test_real_git_repo_returns_empty_masked_set(
|
||||
self, compliance, git_repo,
|
||||
):
|
||||
# Real git repo where every git call succeeds → masked set
|
||||
# is empty regardless of which gaps come back True/False.
|
||||
# The fixture has no CHANGELOG / CONTRIBUTORS / footer, so
|
||||
# three gaps return False — that's a REAL signal (not
|
||||
# masked) and the test does not assert on values, only on
|
||||
# the masked-set contract.
|
||||
gaps, masked = compliance.check_compliance_gaps_with_masking(
|
||||
git_repo, "a@b.com",
|
||||
)
|
||||
assert masked == set()
|
||||
# Sanity: dict shape matches the public API.
|
||||
assert set(gaps.keys()) == {
|
||||
"worktree_clean",
|
||||
"changelog_unreleased_nonempty",
|
||||
"contributors_has_author",
|
||||
"commit_has_issues_closed",
|
||||
}
|
||||
|
||||
def test_non_git_dir_returns_masked_for_git_checks(
|
||||
self, compliance, tmp_path,
|
||||
):
|
||||
# A non-git directory: both git-dependent checks
|
||||
# (``worktree_clean`` + ``commit_has_issues_closed``) mask.
|
||||
# The filesystem-only checks return False because the
|
||||
# files don't exist — those are NOT in the masked set.
|
||||
gaps, masked = compliance.check_compliance_gaps_with_masking(
|
||||
tmp_path, "x@y.com",
|
||||
)
|
||||
assert masked == {"worktree_clean", "commit_has_issues_closed"}
|
||||
# The masked values are True (don't conflate with real gap)
|
||||
assert gaps["worktree_clean"] is True
|
||||
assert gaps["commit_has_issues_closed"] is True
|
||||
# Filesystem-only checks return False because the files
|
||||
# don't exist — not masked, this is a real signal.
|
||||
assert gaps["changelog_unreleased_nonempty"] is False
|
||||
assert gaps["contributors_has_author"] is False
|
||||
|
||||
def test_unborn_head_masks_commit_check_only(
|
||||
self, compliance, tmp_path,
|
||||
):
|
||||
# Mixed-state masking: ``git status`` succeeds on a fresh
|
||||
# ``git init`` (no commits yet) but ``git log -1`` fails
|
||||
# because HEAD is unborn. This produces the asymmetric
|
||||
# masked set ``{commit_has_issues_closed}`` — the third
|
||||
# of four possible mask-state combinations. Without this
|
||||
# test, only the "both masked" and "neither masked"
|
||||
# corners are exercised, leaving the asymmetric cases
|
||||
# unverified.
|
||||
subprocess.run(["git", "init", "-q", str(tmp_path)], check=True)
|
||||
subprocess.run(
|
||||
["git", "-C", str(tmp_path),
|
||||
"config", "commit.gpgsign", "false"],
|
||||
check=True,
|
||||
)
|
||||
# No commit made — HEAD unborn. ``git status`` will work,
|
||||
# ``git log -1 HEAD`` will fail.
|
||||
gaps, masked = compliance.check_compliance_gaps_with_masking(
|
||||
tmp_path, "x@y.com",
|
||||
)
|
||||
# ONLY the commit check is masked — git status succeeded.
|
||||
assert masked == {"commit_has_issues_closed"}
|
||||
# worktree_clean: real signal (empty repo → True, not masked)
|
||||
assert gaps["worktree_clean"] is True
|
||||
# commit_has_issues_closed: masked True (couldn't read HEAD)
|
||||
assert gaps["commit_has_issues_closed"] is True
|
||||
# Filesystem checks: real False (no files)
|
||||
assert gaps["changelog_unreleased_nonempty"] is False
|
||||
assert gaps["contributors_has_author"] is False
|
||||
|
||||
def test_public_check_compliance_gaps_returns_dict_unchanged(
|
||||
self, compliance, git_repo,
|
||||
):
|
||||
# The public single-bool-dict API stays callable for unit
|
||||
# tests that don't care about masking. It must return the
|
||||
# same dict shape as before this refactor.
|
||||
gaps = compliance.check_compliance_gaps(git_repo, "a@b.com")
|
||||
assert isinstance(gaps, dict)
|
||||
assert set(gaps.keys()) == {
|
||||
"worktree_clean",
|
||||
"changelog_unreleased_nonempty",
|
||||
"contributors_has_author",
|
||||
"commit_has_issues_closed",
|
||||
}
|
||||
# All values are strict bool
|
||||
assert all(isinstance(v, bool) for v in gaps.values())
|
||||
|
||||
|
||||
class TestGapsOpenCount:
|
||||
def test_counts_false_values(self, compliance):
|
||||
assert compliance.gaps_open_count(
|
||||
{"a": True, "b": False, "c": False, "d": True}
|
||||
) == 2
|
||||
|
||||
def test_all_true_returns_zero(self, compliance):
|
||||
assert compliance.gaps_open_count(
|
||||
{"a": True, "b": True}
|
||||
) == 0
|
||||
|
||||
def test_empty_dict_returns_zero(self, compliance):
|
||||
assert compliance.gaps_open_count({}) == 0
|
||||
|
||||
|
||||
class TestRenderPromptStanza:
|
||||
def test_empty_dict_renders_empty(self, compliance):
|
||||
assert compliance.render_prompt_stanza({}) == ""
|
||||
@@ -268,3 +461,52 @@ class TestRenderPromptStanza:
|
||||
# Closed gaps shouldn't get a hint
|
||||
assert "amend the HEAD commit message" not in out
|
||||
assert "Gaps to fill (2)" in out
|
||||
|
||||
def test_all_passing_with_masking_hedges_verdict(self, compliance):
|
||||
# Module-level renderer must mirror the dispatcher's
|
||||
# short-form pointer: when every gap is True but some
|
||||
# checks were masked, do NOT emit the confident success
|
||||
# directive. The two renderers serve different audiences
|
||||
# (this one renders the long-form stanza for direct callers;
|
||||
# the dispatcher's renders the short-form pointer for the
|
||||
# prompt body) but they MUST agree on the verdict.
|
||||
out = compliance.render_prompt_stanza(
|
||||
{
|
||||
"worktree_clean": True,
|
||||
"changelog_unreleased_nonempty": True,
|
||||
"contributors_has_author": True,
|
||||
"commit_has_issues_closed": True,
|
||||
},
|
||||
masked_checks={"worktree_clean", "commit_has_issues_closed"},
|
||||
)
|
||||
assert "All OBSERVABLE checks passed" in out
|
||||
assert "MASKED" in out
|
||||
assert "`worktree_clean`" in out
|
||||
assert "`commit_has_issues_closed`" in out
|
||||
# Confident-success language must NOT leak into the hedge
|
||||
assert "All checks passed.**" not in out
|
||||
assert '"outcome": "resolved"' not in out
|
||||
# Per-line markers should annotate which entries were masked
|
||||
assert "_(masked — git failed)_" in out
|
||||
# Markdown balance: ``**`` markers paired
|
||||
assert out.count("**") % 2 == 0
|
||||
|
||||
def test_all_passing_no_masking_emits_success_directive(
|
||||
self, compliance,
|
||||
):
|
||||
# Mirror positive: with no masking, the confident success
|
||||
# directive IS emitted. Pins the hedge-introduction above
|
||||
# against accidentally suppressing the green-path verdict.
|
||||
out = compliance.render_prompt_stanza(
|
||||
{
|
||||
"worktree_clean": True,
|
||||
"changelog_unreleased_nonempty": True,
|
||||
"contributors_has_author": True,
|
||||
"commit_has_issues_closed": True,
|
||||
},
|
||||
masked_checks=set(),
|
||||
)
|
||||
assert "All checks passed" in out
|
||||
assert '"outcome": "resolved"' in out
|
||||
assert "MASKED" not in out
|
||||
assert "_(masked — git failed)_" not in out
|
||||
|
||||
@@ -440,6 +440,106 @@ class TestEscalationLoop:
|
||||
"escalation loop completes"
|
||||
)
|
||||
|
||||
def test_outcome_synthesised_recorded_per_attempt_across_loop(
|
||||
self, driver, cfg, monkeypatch, patch_network,
|
||||
scripted_worker, reset_calls, claim_refresh_calls,
|
||||
):
|
||||
# Walk the loop through 2 attempts and assert the per-attempt
|
||||
# ``outcome_synthesised`` flag is plumbed correctly:
|
||||
#
|
||||
# Tier 0: worker dies without JSON (parsed_json=None,
|
||||
# terminal_state="completed") → synthesised
|
||||
# "rebase-failed" → outcome_synthesised=True on
|
||||
# attempts[0].
|
||||
# Tier 1: worker emits real JSON {"outcome": "resolved"}
|
||||
# → no synthesis → outcome_synthesised=False on
|
||||
# attempts[1].
|
||||
#
|
||||
# Without this end-to-end test a refactor that reassigned
|
||||
# ``last_outcome_synthesised`` to a stale value inside the
|
||||
# while-loop would slip past every unit test (synthesiser
|
||||
# tests don't exercise the loop; telemetry tests don't
|
||||
# exercise the dispatcher's plumbing).
|
||||
monkeypatch.setenv("IMPLEMENTER_ESCALATION_ENABLED", "1")
|
||||
item = _pr_item()
|
||||
item["_dispatcher_implementer_context"] = {
|
||||
"result": None,
|
||||
"clone_handle": type(
|
||||
"FakeHandle", (),
|
||||
{"path": "/tmp/pretend", "head_sha": "deadbeefcafe"},
|
||||
)(),
|
||||
}
|
||||
# Tier 0 GET: head_sha unchanged → competence-class failure
|
||||
# → ESCALATE. Tier 1 GET: head_sha advances → SUCCESS.
|
||||
get_counter = {"n": 0}
|
||||
|
||||
def _gets(_path, _cfg):
|
||||
get_counter["n"] += 1
|
||||
if get_counter["n"] <= 2:
|
||||
return {
|
||||
"status": 200,
|
||||
"body": {
|
||||
"state": "open",
|
||||
"head": {"sha": "deadbeefcafe"},
|
||||
},
|
||||
}
|
||||
return {
|
||||
"status": 200,
|
||||
"body": {
|
||||
"state": "open",
|
||||
"head": {"sha": "tier1pushedsha"},
|
||||
},
|
||||
}
|
||||
|
||||
monkeypatch.setattr(driver._claim_runtime, "get", _gets)
|
||||
|
||||
# Tier 1 worker session emits a real JSON outcome — NOT
|
||||
# synthesised.
|
||||
script, _calls = scripted_worker
|
||||
script.append(_FakeSession(
|
||||
status="completed",
|
||||
parsed_json={"outcome": "resolved"},
|
||||
wallclock_seconds=2.0,
|
||||
))
|
||||
|
||||
runtime = load_tool_module("_dispatch_runtime")
|
||||
ctx = runtime.SessionContext(
|
||||
work_group_name="failing_ci_pr",
|
||||
session_started_at="t0",
|
||||
session_completed_at="t1",
|
||||
session_wallclock_seconds=1.0,
|
||||
subagent_max_depth=0,
|
||||
)
|
||||
|
||||
# Tier 0: parsed_json=None — the synthesiser MUST fire on
|
||||
# this attempt and produce a "rebase-failed" outcome.
|
||||
result = driver._dispatch_post_session_action(
|
||||
cfg, item,
|
||||
None, # no JSON from Tier 0
|
||||
"",
|
||||
"completed",
|
||||
session_context=ctx,
|
||||
)
|
||||
|
||||
assert result["final_action"] == "success"
|
||||
assert len(result["attempts"]) == 2
|
||||
|
||||
attempt0_row = result["attempts"][0]["phase4_telemetry"]["row"]
|
||||
attempt1_row = result["attempts"][1]["phase4_telemetry"]["row"]
|
||||
# Tier 0 was synthesised: dispatcher saw parsed_json=None
|
||||
# and filled in the outcome.
|
||||
assert attempt0_row.get("outcome_synthesised") is True, (
|
||||
f"expected Tier 0 row to record outcome_synthesised=True; "
|
||||
f"got {attempt0_row.get('outcome_synthesised')!r}. Full row: "
|
||||
f"{attempt0_row}"
|
||||
)
|
||||
# Tier 1 was NOT synthesised: worker emitted real JSON.
|
||||
assert attempt1_row.get("outcome_synthesised") is False, (
|
||||
f"expected Tier 1 row to record outcome_synthesised=False; "
|
||||
f"got {attempt1_row.get('outcome_synthesised')!r}. Full row: "
|
||||
f"{attempt1_row}"
|
||||
)
|
||||
|
||||
|
||||
class TestTransportRetry:
|
||||
"""Transport-class failures retry at the SAME tier (2 budget,
|
||||
@@ -522,6 +622,97 @@ class TestTransportRetry:
|
||||
"same-tier retry must NOT reset the worktree"
|
||||
)
|
||||
|
||||
def test_outcome_synthesised_recorded_across_retry_same_tier(
|
||||
self, driver, cfg, monkeypatch, patch_network,
|
||||
scripted_worker, reset_calls, claim_refresh_calls,
|
||||
):
|
||||
# Companion to TestEscalationLoop's
|
||||
# ``test_outcome_synthesised_recorded_per_attempt_across_loop``
|
||||
# — that test pins the ESCALATE branch's reassignment of
|
||||
# ``last_outcome_synthesised``. This test pins the
|
||||
# RETRY_SAME_TIER branch (a separate reassignment site). A
|
||||
# refactor that diverges the two (e.g., one passes the
|
||||
# synthesised flag and the other forgets) would slip past
|
||||
# the ESCALATE-only test.
|
||||
#
|
||||
# Scenario: Tier 0 transport-error (no JSON, synth=True),
|
||||
# retry at same tier succeeds with real JSON (synth=False).
|
||||
monkeypatch.setenv("IMPLEMENTER_ESCALATION_ENABLED", "1")
|
||||
item = _pr_item()
|
||||
item["_dispatcher_implementer_context"] = {
|
||||
"result": None, "clone_handle": None,
|
||||
}
|
||||
get_counter = {"n": 0}
|
||||
|
||||
def _gets(_path, _cfg):
|
||||
get_counter["n"] += 1
|
||||
if get_counter["n"] <= 2:
|
||||
return {
|
||||
"status": 200,
|
||||
"body": {
|
||||
"state": "open",
|
||||
"head": {"sha": "deadbeefcafe"},
|
||||
},
|
||||
}
|
||||
return {
|
||||
"status": 200,
|
||||
"body": {
|
||||
"state": "open",
|
||||
"head": {"sha": "retryadvanced"},
|
||||
},
|
||||
}
|
||||
|
||||
monkeypatch.setattr(driver._claim_runtime, "get", _gets)
|
||||
monkeypatch.setattr(
|
||||
driver._implementer_escalation, "sleep_for_retry",
|
||||
lambda *a, **k: None,
|
||||
)
|
||||
# Retry worker emits real JSON — synthesis must NOT fire.
|
||||
script, _calls = scripted_worker
|
||||
script.append(_FakeSession(
|
||||
status="completed",
|
||||
parsed_json={"outcome": "resolved"},
|
||||
))
|
||||
|
||||
runtime = load_tool_module("_dispatch_runtime")
|
||||
ctx = runtime.SessionContext(
|
||||
work_group_name="failing_ci_pr",
|
||||
session_started_at="t0",
|
||||
session_completed_at="t1",
|
||||
session_wallclock_seconds=1.0,
|
||||
subagent_max_depth=0,
|
||||
)
|
||||
|
||||
# Tier 0 dies without JSON. Synthesiser fills in
|
||||
# "transport-error" outcome → outcome_synthesised=True.
|
||||
result = driver._dispatch_post_session_action(
|
||||
cfg, item,
|
||||
None, # no JSON → synth fires
|
||||
"",
|
||||
"transport-error",
|
||||
session_context=ctx,
|
||||
)
|
||||
|
||||
assert result["final_action"] == "success"
|
||||
assert result["final_tier"] == 0 # same-tier retry, no escalation
|
||||
assert len(result["attempts"]) == 2
|
||||
|
||||
attempt0_row = result["attempts"][0]["phase4_telemetry"]["row"]
|
||||
attempt1_row = result["attempts"][1]["phase4_telemetry"]["row"]
|
||||
# Tier 0 was synthesised
|
||||
assert attempt0_row.get("outcome_synthesised") is True, (
|
||||
f"expected Tier 0 row to record outcome_synthesised=True; "
|
||||
f"got {attempt0_row.get('outcome_synthesised')!r}"
|
||||
)
|
||||
# Same-tier retry was NOT synthesised — distinct reassignment
|
||||
# site in the RETRY_SAME_TIER branch
|
||||
assert attempt1_row.get("outcome_synthesised") is False, (
|
||||
f"expected retry row to record outcome_synthesised=False; "
|
||||
f"got {attempt1_row.get('outcome_synthesised')!r}. "
|
||||
f"This indicates the RETRY_SAME_TIER branch failed to "
|
||||
f"reassign last_outcome_synthesised."
|
||||
)
|
||||
|
||||
def test_transport_retry_exhausts_then_escalates(
|
||||
self, driver, cfg, monkeypatch, patch_network,
|
||||
scripted_worker, reset_calls, claim_refresh_calls,
|
||||
@@ -882,27 +1073,12 @@ class TestDeterministicStanzas:
|
||||
assert out == "BASE PROMPT"
|
||||
|
||||
def test_escalation_flag_on_appends_compliance_stanza(
|
||||
self, driver, cfg, monkeypatch, tmp_path
|
||||
self, driver, cfg, monkeypatch, minimal_git_repo,
|
||||
):
|
||||
import subprocess
|
||||
# Build a real git repo so the compliance detector can run
|
||||
subprocess.run(["git", "init", "-q", str(tmp_path)], check=True)
|
||||
subprocess.run(
|
||||
["git", "-C", str(tmp_path), "config", "user.email", "x@y.com"],
|
||||
check=True,
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "-C", str(tmp_path), "config", "user.name", "Test"],
|
||||
check=True,
|
||||
)
|
||||
(tmp_path / "README.md").write_text("init\n")
|
||||
subprocess.run(
|
||||
["git", "-C", str(tmp_path), "add", "."], check=True,
|
||||
)
|
||||
subprocess.run(
|
||||
["git", "-C", str(tmp_path), "commit", "-q", "-m", "init"],
|
||||
check=True,
|
||||
)
|
||||
# Use the shared ``minimal_git_repo`` conftest fixture rather
|
||||
# than re-inlining the 7-command init/commit boilerplate
|
||||
# (hoisted in the 2026-05-13 third-round review).
|
||||
tmp_path = minimal_git_repo
|
||||
|
||||
monkeypatch.setenv("IMPLEMENTER_ESCALATION_ENABLED", "1")
|
||||
monkeypatch.delenv(
|
||||
@@ -917,12 +1093,281 @@ class TestDeterministicStanzas:
|
||||
out = driver._append_deterministic_stanzas(
|
||||
"BASE PROMPT", cfg, item, _pr_group(driver), result, handle,
|
||||
)
|
||||
# Compliance stanza header should be present
|
||||
# Compliance stanza header should be present (short-pointer
|
||||
# form post-2026-05-13 — the full hints live in the sentinel,
|
||||
# the prompt-side gets a summary + read-command).
|
||||
assert "Compliance gap report" in out
|
||||
# The git_repo fixture has no CHANGELOG → gap reported
|
||||
# The fixture has no CHANGELOG → gap reported in the bullet
|
||||
assert "changelog_unreleased_nonempty" in out
|
||||
# Read-command pointer must reference the sentinel script
|
||||
assert "implementer_pr_context.py" in out
|
||||
# Gate-preflight stanza should NOT be present (flag off)
|
||||
assert "Pre-flight quality gates" not in out
|
||||
assert "Pre-flight gate summary" not in out
|
||||
|
||||
def test_compliance_all_passed_with_masking_hedges_verdict(
|
||||
self, driver,
|
||||
):
|
||||
# Compliance footgun guard: when ALL four checks return True
|
||||
# but one or more were MASKED (git itself failed), the
|
||||
# renderer MUST hedge — do NOT direct the worker to exit
|
||||
# with ``{"outcome": "resolved"}``. The hedged stanza tells
|
||||
# the worker to re-verify the masked checks in-session
|
||||
# before deciding. Without the hedge a broken-git worktree
|
||||
# with a coincidentally-valid CHANGELOG + CONTRIBUTORS would
|
||||
# produce a confident "PR resolved" directive on a tree the
|
||||
# dispatcher couldn't actually inspect.
|
||||
gaps = {
|
||||
"worktree_clean": True,
|
||||
"changelog_unreleased_nonempty": True,
|
||||
"contributors_has_author": True,
|
||||
"commit_has_issues_closed": True,
|
||||
}
|
||||
out = driver._render_compliance_pointer_stanza(
|
||||
gaps, pr_number=30,
|
||||
masked_checks=["worktree_clean", "commit_has_issues_closed"],
|
||||
)
|
||||
# Hedged verdict, not the confident success directive
|
||||
assert "All OBSERVABLE compliance checks passed" in out
|
||||
assert "MASKED" in out
|
||||
assert "`worktree_clean`" in out
|
||||
assert "`commit_has_issues_closed`" in out
|
||||
assert "Do NOT exit with `resolved`" in out
|
||||
# Crucially: the confident-success language MUST NOT appear
|
||||
assert "do NOT re-apply or re-edit the code fix" not in out
|
||||
assert "{\"outcome\": \"resolved\"" not in out
|
||||
# Markdown balance: every ``**`` opens a bold span and must
|
||||
# be paired. An earlier iteration left the hedge's leading
|
||||
# ``**`` unclosed, so the bold ran through the rest of the
|
||||
# stanza (substring asserts didn't catch it). Parity of
|
||||
# ``**`` occurrences is the cheap structural check.
|
||||
assert out.count("**") % 2 == 0, (
|
||||
f"unbalanced ** markdown bold markers in stanza: "
|
||||
f"{out.count('**')} occurrences; stanza follows:\n{out}"
|
||||
)
|
||||
|
||||
def test_compliance_all_passed_no_masking_emits_resolved_directive(
|
||||
self, driver,
|
||||
):
|
||||
# Mirror positive: when nothing was masked, the renderer
|
||||
# emits the confident success directive as before. Pinning
|
||||
# this so the hedge change above doesn't accidentally
|
||||
# suppress the green-path verdict.
|
||||
gaps = {
|
||||
"worktree_clean": True,
|
||||
"changelog_unreleased_nonempty": True,
|
||||
"contributors_has_author": True,
|
||||
"commit_has_issues_closed": True,
|
||||
}
|
||||
out = driver._render_compliance_pointer_stanza(
|
||||
gaps, pr_number=30, masked_checks=[],
|
||||
)
|
||||
assert "All compliance checks passed" in out
|
||||
assert "do NOT re-apply" in out
|
||||
assert '"outcome": "resolved"' in out
|
||||
assert "MASKED" not in out
|
||||
|
||||
def test_compliance_some_missing_no_mask_emits_fill_directive(
|
||||
self, driver,
|
||||
):
|
||||
# Most-common production case: some gaps are open AND
|
||||
# nothing was masked. The renderer emits the "fill ONLY
|
||||
# the missing items" directive and does NOT add the
|
||||
# "values may be stale" italic note. This is the fourth
|
||||
# quadrant of the (all-passed × masking) truth table —
|
||||
# previously covered only indirectly via the real-git-repo
|
||||
# integration test.
|
||||
gaps = {
|
||||
"worktree_clean": True,
|
||||
"changelog_unreleased_nonempty": False,
|
||||
"contributors_has_author": False,
|
||||
"commit_has_issues_closed": True,
|
||||
}
|
||||
out = driver._render_compliance_pointer_stanza(
|
||||
gaps, pr_number=30, masked_checks=[],
|
||||
)
|
||||
# The fill directive must appear with the correct count
|
||||
assert "Gaps to fill (2)" in out
|
||||
assert "`changelog_unreleased_nonempty`" in out
|
||||
assert "`contributors_has_author`" in out
|
||||
# Fill-only language pinned (worker contract)
|
||||
assert "Fill ONLY the missing items" in out
|
||||
# The masking-only note must NOT appear (nothing was masked)
|
||||
assert "may be stale" not in out
|
||||
# Confident-success directive MUST NOT leak into the
|
||||
# some-missing branch
|
||||
assert '"outcome": "resolved"' not in out
|
||||
# Markdown balance
|
||||
assert out.count("**") % 2 == 0
|
||||
|
||||
def test_compliance_some_missing_with_masking_notes_masked(
|
||||
self, driver,
|
||||
):
|
||||
# When some gaps are open AND some checks were masked, the
|
||||
# renderer emits the normal "fill these gaps" directive
|
||||
# PLUS an italic note that some values may be stale. The
|
||||
# worker still has actionable work; the note nudges them
|
||||
# to re-verify the masked keys before exiting.
|
||||
gaps = {
|
||||
"worktree_clean": True, # masked (git broken)
|
||||
"changelog_unreleased_nonempty": False, # real gap
|
||||
"contributors_has_author": True,
|
||||
"commit_has_issues_closed": True, # masked (git broken)
|
||||
}
|
||||
out = driver._render_compliance_pointer_stanza(
|
||||
gaps, pr_number=30,
|
||||
masked_checks=["worktree_clean", "commit_has_issues_closed"],
|
||||
)
|
||||
assert "Gaps to fill (1)" in out
|
||||
assert "changelog_unreleased_nonempty" in out
|
||||
assert "masked" in out # note about masking
|
||||
assert "may be stale" in out
|
||||
|
||||
def test_preflight_disabled_shape_renders_empty(self, driver):
|
||||
# Defensive: ``preflight_enabled=False`` is the documented
|
||||
# disabled-shape returned by ``run_preflight`` when its flag
|
||||
# is off. Production never feeds this to the renderer
|
||||
# (``_compute_deterministic_sections`` gates upstream) but
|
||||
# if a future caller did, the renderer must return empty
|
||||
# — not the misleading "no persistent failures" green-light
|
||||
# line.
|
||||
preflight = {
|
||||
"preflight_enabled": False,
|
||||
"failures_total": 0,
|
||||
"related": [],
|
||||
"unrelated": [],
|
||||
"gate_statuses": {},
|
||||
"runs": [],
|
||||
}
|
||||
out = driver._render_preflight_pointer_stanza(
|
||||
preflight, pr_number=30,
|
||||
)
|
||||
assert out == ""
|
||||
|
||||
def test_preflight_timeout_renders_suppressed_classification(
|
||||
self, driver,
|
||||
):
|
||||
# When the orchestrator returns a timeout payload (zeroed
|
||||
# counts, ``preflight_timeout=True``), the rendered stanza
|
||||
# must:
|
||||
# 1. surface the "Pre-flight timed out" warning
|
||||
# 2. include the "(Classification suppressed)" line so the
|
||||
# worker knows why no counts appear
|
||||
# 3. NOT include "Persistent failures: N" — that would
|
||||
# contradict the timeout warning
|
||||
# Use the preflight module's actual sentinel so the fixture
|
||||
# stays self-consistent if the constant ever changes
|
||||
# (post-2026-05-13 it's ``-9999``, not the old ``-1``).
|
||||
preflight = load_tool_module("_implementer_gate_preflight")
|
||||
preflight_payload = {
|
||||
"preflight_enabled": True,
|
||||
"preflight_timeout": True,
|
||||
"gate_statuses": {},
|
||||
"failures_total": 0,
|
||||
"related": [],
|
||||
"unrelated": [],
|
||||
"runs": [{
|
||||
"run": 1,
|
||||
"returncode": preflight._TIMEOUT_RETURNCODE,
|
||||
"failures": 0,
|
||||
}],
|
||||
}
|
||||
out = driver._render_preflight_pointer_stanza(
|
||||
preflight_payload, pr_number=30,
|
||||
)
|
||||
assert "Pre-flight timed out" in out
|
||||
assert "Classification suppressed" in out
|
||||
assert "Persistent failures" not in out
|
||||
# The "no persistent failures" green-light line is also
|
||||
# suppressed — that would be just as misleading.
|
||||
assert "No persistent failures across two pre-flight runs" not in out
|
||||
|
||||
def test_compliance_gaps_kill_switch_disables_compliance_section(
|
||||
self, driver, cfg, monkeypatch, minimal_git_repo,
|
||||
):
|
||||
# ``IMPLEMENTER_COMPLIANCE_GAPS_ENABLED=0`` opts out of the
|
||||
# compliance scan while leaving the escalation feature on.
|
||||
# The det_sections dict therefore omits ``compliance_gaps``
|
||||
# and the rendered prompt has no compliance stanza.
|
||||
tmp_path = minimal_git_repo
|
||||
monkeypatch.setenv("IMPLEMENTER_ESCALATION_ENABLED", "1")
|
||||
monkeypatch.setenv("IMPLEMENTER_COMPLIANCE_GAPS_ENABLED", "0")
|
||||
item = _pr_item()
|
||||
result = type("FakeResult", (), {"diff": ""})()
|
||||
handle = type("FakeHandle", (), {"path": str(tmp_path)})()
|
||||
det = driver._compute_deterministic_sections(
|
||||
cfg, item, _pr_group(driver), result, handle,
|
||||
)
|
||||
assert "compliance_gaps" not in det
|
||||
|
||||
def test_default_on_when_compliance_flag_unset(
|
||||
self, driver, cfg, monkeypatch, minimal_git_repo,
|
||||
):
|
||||
# When the per-feature opt-out is unset and escalation is on,
|
||||
# the compliance section is computed (post-2026-05-13
|
||||
# default-ON behaviour). Mirrors the outcome-synthesis
|
||||
# default-on test.
|
||||
tmp_path = minimal_git_repo
|
||||
monkeypatch.setenv("IMPLEMENTER_ESCALATION_ENABLED", "1")
|
||||
monkeypatch.delenv("IMPLEMENTER_COMPLIANCE_GAPS_ENABLED", raising=False)
|
||||
item = _pr_item()
|
||||
result = type("FakeResult", (), {"diff": ""})()
|
||||
handle = type("FakeHandle", (), {"path": str(tmp_path)})()
|
||||
det = driver._compute_deterministic_sections(
|
||||
cfg, item, _pr_group(driver), result, handle,
|
||||
)
|
||||
assert "compliance_gaps" in det
|
||||
|
||||
def test_both_flags_on_preflight_appears_before_compliance(
|
||||
self, driver, cfg, monkeypatch, minimal_git_repo,
|
||||
):
|
||||
# Stanza-ordering invariant: gate-preflight (which tells the
|
||||
# agent which test failures matter) must precede compliance
|
||||
# (which tells the agent what to fill in). Otherwise an
|
||||
# agent that reads sequentially would react to compliance
|
||||
# gaps first and miss the gate context.
|
||||
tmp_path = minimal_git_repo
|
||||
monkeypatch.setenv("IMPLEMENTER_ESCALATION_ENABLED", "1")
|
||||
monkeypatch.setenv("IMPLEMENTER_DISPATCHER_GATE_PREFLIGHT", "1")
|
||||
item = _pr_item()
|
||||
# Hand-built det_sections so we don't shell out to the real
|
||||
# ``local_ci_gate.sh``. Production wires this in
|
||||
# ``_prefetch_prompt``; the test exercises the same renderer.
|
||||
det_sections = {
|
||||
"gate_preflight": {
|
||||
"preflight_enabled": True,
|
||||
"gate_statuses": {"unit_tests": "FAIL"},
|
||||
"failures_total": 1,
|
||||
"related": [],
|
||||
"unrelated": [{"path": "features/x.feature", "line": "1"}],
|
||||
"runs": [{"run": 1, "returncode": 1, "failures": 1}],
|
||||
},
|
||||
"compliance_gaps": {
|
||||
"gaps": {
|
||||
"worktree_clean": True,
|
||||
"changelog_unreleased_nonempty": False,
|
||||
"contributors_has_author": False,
|
||||
"commit_has_issues_closed": False,
|
||||
},
|
||||
"pr_number": 30,
|
||||
"git_user_email": "x@y.com",
|
||||
"gaps_open_count": 3,
|
||||
},
|
||||
}
|
||||
result = type("FakeResult", (), {"diff": ""})()
|
||||
handle = type(
|
||||
"FakeHandle", (), {"path": str(tmp_path)},
|
||||
)()
|
||||
out = driver._append_deterministic_stanzas(
|
||||
"BASE", cfg, item, _pr_group(driver), result, handle,
|
||||
det_sections=det_sections,
|
||||
)
|
||||
preflight_idx = out.find("Pre-flight gate summary")
|
||||
compliance_idx = out.find("Compliance gap report")
|
||||
assert preflight_idx >= 0 and compliance_idx >= 0
|
||||
assert preflight_idx < compliance_idx, (
|
||||
f"gate-preflight (idx={preflight_idx}) must precede "
|
||||
f"compliance (idx={compliance_idx})"
|
||||
)
|
||||
|
||||
|
||||
def _pr_group(driver):
|
||||
@@ -994,6 +1439,70 @@ class TestOutcomeSynthesis:
|
||||
assert synth is True
|
||||
assert parsed["outcome"] == "rebase-failed"
|
||||
|
||||
def test_empty_string_outcome_synthesises(self, driver):
|
||||
# outcome present but empty string — the parsed-JSON
|
||||
# passthrough requires a TRUTHY string, so this still
|
||||
# synthesises rather than passing through.
|
||||
parsed, synth = driver._synthesize_outcome_if_missing(
|
||||
{"outcome": ""}, "completed",
|
||||
)
|
||||
assert synth is True
|
||||
assert parsed["outcome"] == "rebase-failed"
|
||||
|
||||
def test_passthrough_omits_synthesised_marker(self, driver):
|
||||
# When the worker emitted a real outcome, the returned dict
|
||||
# MUST NOT carry the ``_synthesized`` flag. Otherwise the
|
||||
# phase4 telemetry can't distinguish synthesised from
|
||||
# genuine outcomes.
|
||||
parsed, synth = driver._synthesize_outcome_if_missing(
|
||||
{"outcome": "resolved", "files_touched": []},
|
||||
"completed",
|
||||
)
|
||||
assert synth is False
|
||||
assert "_synthesized" not in parsed
|
||||
|
||||
def test_kill_switch_disables_synthesis(self, driver, monkeypatch):
|
||||
# Operator panic-button: ``IMPLEMENTER_OUTCOME_SYNTHESIS=0``
|
||||
# reverts to the legacy "UNKNOWN-bucket wastes one retry"
|
||||
# behaviour. parsed_json passes through (None stays None);
|
||||
# synth is False so the telemetry row will not carry the
|
||||
# ``outcome_synthesised`` field.
|
||||
monkeypatch.setenv("IMPLEMENTER_OUTCOME_SYNTHESIS", "0")
|
||||
parsed, synth = driver._synthesize_outcome_if_missing(
|
||||
None, "completed",
|
||||
)
|
||||
assert synth is False
|
||||
assert parsed is None
|
||||
|
||||
def test_kill_switch_passes_through_empty_outcome_dict(
|
||||
self, driver, monkeypatch,
|
||||
):
|
||||
# Kill-switch + non-None-but-falsy outcome: parsed_json
|
||||
# carries an empty-string outcome (worker emitted JSON but
|
||||
# botched the field). With the kill-switch ON the
|
||||
# synthesiser passes it through unchanged so the downstream
|
||||
# escalation classifier sees the original dict (and routes
|
||||
# via the legacy UNKNOWN bucket). Without this test, a
|
||||
# future refactor that synthesises whenever ``outcome``
|
||||
# falls into the empty-string branch could regress the
|
||||
# kill-switch contract silently.
|
||||
monkeypatch.setenv("IMPLEMENTER_OUTCOME_SYNTHESIS", "0")
|
||||
parsed, synth = driver._synthesize_outcome_if_missing(
|
||||
{"outcome": "", "files_touched": []}, "completed",
|
||||
)
|
||||
assert synth is False
|
||||
assert parsed == {"outcome": "", "files_touched": []}
|
||||
|
||||
def test_default_on_when_env_unset(self, driver, monkeypatch):
|
||||
# Default behaviour (env unset) is to synthesise — this is
|
||||
# the post-2026-05-13 production default.
|
||||
monkeypatch.delenv("IMPLEMENTER_OUTCOME_SYNTHESIS", raising=False)
|
||||
parsed, synth = driver._synthesize_outcome_if_missing(
|
||||
None, "completed",
|
||||
)
|
||||
assert synth is True
|
||||
assert parsed["outcome"] == "rebase-failed"
|
||||
|
||||
|
||||
class TestStatusCommentFingerprintTier:
|
||||
"""Direct unit-ish tests for ``_review_post.post_implementer_status_comment``'s
|
||||
|
||||
@@ -28,6 +28,10 @@ from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
# Timeout-test imports — kept top-level so they don't get duplicated
|
||||
# across two test methods that need them.
|
||||
from unittest.mock import patch
|
||||
|
||||
from .conftest import load_tool_module
|
||||
|
||||
|
||||
@@ -81,7 +85,7 @@ class TestPersistentFailures:
|
||||
def test_run1_passing_skips_run2(self, preflight, monkeypatch):
|
||||
monkeypatch.setenv(preflight.PREFLIGHT_ENABLED_ENV_VAR, "1")
|
||||
runner = _fake_runner(
|
||||
stdouts=["lint: PASS\nunit_tests: PASS\n"],
|
||||
stdouts=["## [lint] PASS (3s)\n## [unit_tests] PASS (12s)\n"],
|
||||
returncodes=[0],
|
||||
)
|
||||
result = preflight.run_preflight(
|
||||
@@ -99,13 +103,13 @@ class TestPersistentFailures:
|
||||
# Persistent failures = empty.
|
||||
monkeypatch.setenv(preflight.PREFLIGHT_ENABLED_ENV_VAR, "1")
|
||||
run1_out = """
|
||||
unit_tests: FAIL
|
||||
## [unit_tests] FAIL (47s)
|
||||
Failing scenarios:
|
||||
features/automation_profile_cli.feature:74
|
||||
features/cli_extensions.feature:358
|
||||
features/cli_output_formats.feature:46
|
||||
"""
|
||||
run2_out = "lint: PASS\nunit_tests: PASS\n"
|
||||
run2_out = "## [lint] PASS (3s)\n## [unit_tests] PASS (12s)\n"
|
||||
runner = _fake_runner(
|
||||
stdouts=[run1_out, run2_out], returncodes=[1, 0],
|
||||
)
|
||||
@@ -124,7 +128,7 @@ Failing scenarios:
|
||||
# Two runs with identical failures → all persistent
|
||||
monkeypatch.setenv(preflight.PREFLIGHT_ENABLED_ENV_VAR, "1")
|
||||
same = """
|
||||
unit_tests: FAIL
|
||||
## [unit_tests] FAIL (47s)
|
||||
Failing scenarios:
|
||||
features/real_bug.feature:10
|
||||
features/related.feature:20
|
||||
@@ -147,14 +151,14 @@ Failing scenarios:
|
||||
# Run 1 fails A,B,C; run 2 fails B,C,D. Persistent = B,C.
|
||||
monkeypatch.setenv(preflight.PREFLIGHT_ENABLED_ENV_VAR, "1")
|
||||
run1 = """
|
||||
unit_tests: FAIL
|
||||
## [unit_tests] FAIL (47s)
|
||||
Failing scenarios:
|
||||
features/a.feature:1
|
||||
features/b.feature:2
|
||||
features/c.feature:3
|
||||
"""
|
||||
run2 = """
|
||||
unit_tests: FAIL
|
||||
## [unit_tests] FAIL (47s)
|
||||
Failing scenarios:
|
||||
features/b.feature:2
|
||||
features/c.feature:3
|
||||
@@ -175,7 +179,7 @@ Failing scenarios:
|
||||
# → agent gets a clean prompt and doesn't bail.
|
||||
monkeypatch.setenv(preflight.PREFLIGHT_ENABLED_ENV_VAR, "1")
|
||||
run1 = """
|
||||
unit_tests: FAIL
|
||||
## [unit_tests] FAIL (47s)
|
||||
Failing scenarios:
|
||||
features/automation_profile_cli.feature:74
|
||||
features/cli_extensions.feature:358
|
||||
@@ -184,7 +188,10 @@ Failing scenarios:
|
||||
features/plan_cli_coverage_boost.feature:90
|
||||
features/plan_prompt_command.feature:30
|
||||
"""
|
||||
run2 = "lint: PASS\ntypecheck: PASS\nunit_tests: PASS\nintegration_tests: PASS"
|
||||
run2 = (
|
||||
"## [lint] PASS (3s)\n## [typecheck] PASS (5s)\n"
|
||||
"## [unit_tests] PASS (12s)\n## [integration_tests] PASS (30s)"
|
||||
)
|
||||
runner = _fake_runner(stdouts=[run1, run2], returncodes=[1, 0])
|
||||
result = preflight.run_preflight(
|
||||
Path("/tmp"),
|
||||
@@ -195,3 +202,112 @@ Failing scenarios:
|
||||
assert result["failures_total"] == 0
|
||||
assert result["runs"][0]["failures"] == 6
|
||||
assert result["runs"][1]["failures"] == 0
|
||||
# ``flakes_filtered`` records how many failures cleared on
|
||||
# the retry — six in this case.
|
||||
assert result["flakes_filtered"] == 6
|
||||
|
||||
|
||||
# ─── Timeout handling ──────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestTimeoutHandling:
|
||||
def test_run1_timeout_surfaces_preflight_timeout_flag(
|
||||
self, preflight, monkeypatch
|
||||
):
|
||||
# When run 1 times out, the classification MUST surface
|
||||
# ``preflight_timeout=True`` rather than silently rendering
|
||||
# as "no persistent failures" (the previous bug). The
|
||||
# marker line ``<TIMEOUT after Xs>`` parses to zero
|
||||
# behave-failures, so without the explicit flag the
|
||||
# classifier would emit a clean green roll-up.
|
||||
monkeypatch.setenv(preflight.PREFLIGHT_ENABLED_ENV_VAR, "1")
|
||||
# Use a runner that returns the timeout sentinel directly.
|
||||
def timeout_runner(cmd, env, timeout):
|
||||
return subprocess.CompletedProcess(
|
||||
args=cmd,
|
||||
returncode=preflight._TIMEOUT_RETURNCODE,
|
||||
stdout="<TIMEOUT after 1200.0s>",
|
||||
stderr="",
|
||||
)
|
||||
|
||||
result = preflight.run_preflight(
|
||||
Path("/tmp"), changed_files=["x.py"], runner=timeout_runner,
|
||||
)
|
||||
assert result.get("preflight_timeout") is True
|
||||
assert result["preflight_enabled"] is True
|
||||
# Only one run recorded — we don't double-up after timeout.
|
||||
assert len(result["runs"]) == 1
|
||||
# Counts are explicitly suppressed on timeout — the renderer
|
||||
# cannot show a contradictory "Persistent failures: N" line
|
||||
# next to the timeout warning.
|
||||
assert result["failures_total"] == 0
|
||||
assert result["related"] == []
|
||||
assert result["unrelated"] == []
|
||||
assert result["gate_statuses"] == {}
|
||||
|
||||
def test_run2_timeout_suppresses_run1_failure_counts(
|
||||
self, preflight, monkeypatch
|
||||
):
|
||||
# Run 1 produces parseable failures; run 2 times out. The
|
||||
# orchestrator cannot intersect to find PERSISTENT failures,
|
||||
# so it suppresses the counts and surfaces preflight_timeout
|
||||
# instead. Run-1's gate_statuses ARE preserved (they're real
|
||||
# data) but the failure lists are emptied. Without this
|
||||
# suppression the worker would see run-1's count next to the
|
||||
# timeout banner and read contradictory advice.
|
||||
monkeypatch.setenv(preflight.PREFLIGHT_ENABLED_ENV_VAR, "1")
|
||||
|
||||
run1_out = """
|
||||
## [unit_tests] FAIL (47s)
|
||||
Failing scenarios:
|
||||
features/a.feature:1
|
||||
features/b.feature:2
|
||||
"""
|
||||
|
||||
def two_call_runner(cmd, env, timeout):
|
||||
# First call: real failure output. Second call: timeout.
|
||||
if not hasattr(two_call_runner, "calls"):
|
||||
two_call_runner.calls = 0
|
||||
two_call_runner.calls += 1
|
||||
if two_call_runner.calls == 1:
|
||||
return subprocess.CompletedProcess(
|
||||
args=cmd, returncode=1, stdout=run1_out, stderr=""
|
||||
)
|
||||
return subprocess.CompletedProcess(
|
||||
args=cmd,
|
||||
returncode=preflight._TIMEOUT_RETURNCODE,
|
||||
stdout="<TIMEOUT after 1200.0s>",
|
||||
stderr="",
|
||||
)
|
||||
|
||||
result = preflight.run_preflight(
|
||||
Path("/tmp"), changed_files=["x.py"], runner=two_call_runner,
|
||||
)
|
||||
assert result.get("preflight_timeout") is True
|
||||
assert len(result["runs"]) == 2
|
||||
# run-1 gate_statuses survive — they're real data from the
|
||||
# run that DID complete.
|
||||
assert result["gate_statuses"].get("unit_tests") == "FAIL"
|
||||
# Failure counts/lists are suppressed.
|
||||
assert result["failures_total"] == 0
|
||||
assert result["related"] == []
|
||||
assert result["unrelated"] == []
|
||||
|
||||
def test_run_gate_once_raises_subprocess_timeout(
|
||||
self, preflight, monkeypatch, tmp_path,
|
||||
):
|
||||
# ``subprocess.run`` raising :class:`subprocess.TimeoutExpired`
|
||||
# must collapse to ``(_TIMEOUT_RETURNCODE, "<TIMEOUT ...>")``
|
||||
# rather than propagating. The default-runner branch is the
|
||||
# one that hits the real subprocess and the one we couldn't
|
||||
# cover via the ``runner`` indirection.
|
||||
def raise_timeout(*args, **kwargs):
|
||||
raise subprocess.TimeoutExpired(
|
||||
cmd=kwargs.get("args") or args[0],
|
||||
timeout=kwargs.get("timeout", 1200),
|
||||
)
|
||||
|
||||
with patch.object(subprocess, "run", side_effect=raise_timeout):
|
||||
rc, out = preflight._run_gate_once(tmp_path)
|
||||
assert rc == preflight._TIMEOUT_RETURNCODE
|
||||
assert out.startswith("<TIMEOUT after")
|
||||
|
||||
@@ -232,6 +232,83 @@ def test_read_epic_returns_epic_object(cli, handoff_dir, capsys):
|
||||
assert parsed["number"] == 42
|
||||
|
||||
|
||||
def test_read_compliance_gaps_when_present(cli, handoff_dir, capsys):
|
||||
"""The dispatcher writes ``compliance_gaps`` + the matching
|
||||
completion flag; ``--field compliance_gaps`` returns the dict.
|
||||
Worker uses this instead of the prompt-side markdown stanza
|
||||
(which gets summarised away by intermediate tier agents)."""
|
||||
gaps_section = {
|
||||
"gaps": {
|
||||
"worktree_clean": True,
|
||||
"changelog_unreleased_nonempty": False,
|
||||
"contributors_has_author": False,
|
||||
"commit_has_issues_closed": False,
|
||||
},
|
||||
"pr_number": 30,
|
||||
"git_user_email": "x@y.com",
|
||||
"gaps_open_count": 3,
|
||||
}
|
||||
_write_sentinel(
|
||||
handoff_dir, 30,
|
||||
compliance_gaps=gaps_section,
|
||||
compliance_gaps_completed=True,
|
||||
)
|
||||
rc = cli.main(["read", "--pr", "30", "--field", "compliance_gaps"])
|
||||
captured = capsys.readouterr()
|
||||
assert rc == 0
|
||||
parsed = json.loads(captured.out)
|
||||
assert parsed["gaps_open_count"] == 3
|
||||
assert parsed["gaps"]["changelog_unreleased_nonempty"] is False
|
||||
|
||||
|
||||
def test_read_compliance_gaps_not_computed_prints_nothing(
|
||||
cli, handoff_dir, capsys,
|
||||
):
|
||||
"""When the dispatcher's flag was off, no ``compliance_gaps``
|
||||
section is written to the sentinel. The reader should print
|
||||
NOTHING (empty stdout) — the worker's contract treats this as
|
||||
"fall through to in-session discovery", same shape as a
|
||||
missing prefetch field."""
|
||||
_write_sentinel(handoff_dir, 30)
|
||||
rc = cli.main(["read", "--pr", "30", "--field", "compliance_gaps"])
|
||||
captured = capsys.readouterr()
|
||||
assert rc == 0
|
||||
assert captured.out == ""
|
||||
|
||||
|
||||
def test_read_gate_preflight_when_present(cli, handoff_dir, capsys):
|
||||
preflight_section = {
|
||||
"preflight_enabled": True,
|
||||
"gate_statuses": {"unit_tests": "FAIL"},
|
||||
"failures_total": 2,
|
||||
"related": [],
|
||||
"unrelated": [{"path": "features/x.feature", "line": "1"}],
|
||||
"runs": [{"run": 1, "returncode": 1, "failures": 2}],
|
||||
"flakes_filtered": 0,
|
||||
}
|
||||
_write_sentinel(
|
||||
handoff_dir, 31,
|
||||
gate_preflight=preflight_section,
|
||||
gate_preflight_completed=True,
|
||||
)
|
||||
rc = cli.main(["read", "--pr", "31", "--field", "gate_preflight"])
|
||||
captured = capsys.readouterr()
|
||||
assert rc == 0
|
||||
parsed = json.loads(captured.out)
|
||||
assert parsed["failures_total"] == 2
|
||||
assert parsed["gate_statuses"]["unit_tests"] == "FAIL"
|
||||
|
||||
|
||||
def test_read_gate_preflight_not_computed_prints_nothing(
|
||||
cli, handoff_dir, capsys,
|
||||
):
|
||||
_write_sentinel(handoff_dir, 31)
|
||||
rc = cli.main(["read", "--pr", "31", "--field", "gate_preflight"])
|
||||
captured = capsys.readouterr()
|
||||
assert rc == 0
|
||||
assert captured.out == ""
|
||||
|
||||
|
||||
def test_read_all_returns_full_payload(cli, handoff_dir, capsys):
|
||||
"""``all`` returns the entire sentinel — debugging affordance
|
||||
for an operator inspecting from the shell."""
|
||||
|
||||
@@ -335,6 +335,63 @@ class TestEscalationExtras:
|
||||
assert row["tier_attempt_index"] == 0
|
||||
assert row["escalation_tier_hint"] == 0
|
||||
|
||||
def test_outcome_synthesised_omitted_when_not_passed(self, telemetry):
|
||||
# ``outcome_synthesised`` follows the same opt-in contract as
|
||||
# the other escalation extras: legacy callers don't pass it
|
||||
# and the row must stay byte-equivalent to the pre-feature
|
||||
# build.
|
||||
row = telemetry.extract_phase4_telemetry(
|
||||
cycle_id="cycle-1",
|
||||
pr_number=42,
|
||||
work_group="failing_ci_pr",
|
||||
start_ts="t0",
|
||||
end_ts="t1",
|
||||
wall_clock_seconds=60.0,
|
||||
parsed_json={"outcome": "resolved"},
|
||||
raw_response="",
|
||||
terminal_state="completed",
|
||||
pre_session_head_sha=None,
|
||||
)
|
||||
assert "outcome_synthesised" not in row
|
||||
|
||||
def test_outcome_synthesised_true_recorded(self, telemetry):
|
||||
row = telemetry.extract_phase4_telemetry(
|
||||
cycle_id="cycle-1",
|
||||
pr_number=42,
|
||||
work_group="failing_ci_pr",
|
||||
start_ts="t0",
|
||||
end_ts="t1",
|
||||
wall_clock_seconds=60.0,
|
||||
parsed_json={"outcome": "rebase-failed", "_synthesized": True},
|
||||
raw_response="",
|
||||
terminal_state="completed",
|
||||
pre_session_head_sha=None,
|
||||
tier_attempt_index=0,
|
||||
outcome_synthesised=True,
|
||||
)
|
||||
assert row["outcome_synthesised"] is True
|
||||
|
||||
def test_outcome_synthesised_false_recorded(self, telemetry):
|
||||
# ``False`` is distinct from "not passed" — when the
|
||||
# escalation runner observed a genuine worker outcome it
|
||||
# records False so an analyst can see the legacy success
|
||||
# vs. synthesised verdict explicitly.
|
||||
row = telemetry.extract_phase4_telemetry(
|
||||
cycle_id="cycle-1",
|
||||
pr_number=42,
|
||||
work_group="failing_ci_pr",
|
||||
start_ts="t0",
|
||||
end_ts="t1",
|
||||
wall_clock_seconds=60.0,
|
||||
parsed_json={"outcome": "resolved"},
|
||||
raw_response="",
|
||||
terminal_state="completed",
|
||||
pre_session_head_sha=None,
|
||||
tier_attempt_index=0,
|
||||
outcome_synthesised=False,
|
||||
)
|
||||
assert row["outcome_synthesised"] is False
|
||||
|
||||
|
||||
class TestSubagentMaxDepth:
|
||||
"""``subagent_max_depth`` (added in Tier-1 R2, 2026-05-12) is
|
||||
|
||||
@@ -820,6 +820,95 @@ def test_write_atomic_cleans_tmp_on_write_failure(
|
||||
assert not (handoff_dir / "pr-30.json.tmp").exists()
|
||||
|
||||
|
||||
def test_write_includes_compliance_gaps_when_passed(
|
||||
sentinel, handoff_dir,
|
||||
):
|
||||
"""Deterministic compliance-gap section (added 2026-05-13)
|
||||
must be projected into the sentinel under ``compliance_gaps``
|
||||
plus a matching ``compliance_gaps_completed=True`` flag. The
|
||||
flag is what the reader's gating helper checks to distinguish
|
||||
"dispatcher computed nothing" (worker falls through) from
|
||||
"dispatcher computed and produced this dict" (worker reads)."""
|
||||
result = _FakeResult(
|
||||
head_sha="aaa",
|
||||
pr_details={
|
||||
"title": "T", "body": "B",
|
||||
"head": {"ref": "feat"}, "base": {"ref": "master"},
|
||||
},
|
||||
diff_text="diff",
|
||||
)
|
||||
gaps_payload = {
|
||||
"gaps": {
|
||||
"worktree_clean": True,
|
||||
"changelog_unreleased_nonempty": False,
|
||||
},
|
||||
"pr_number": 30,
|
||||
"git_user_email": "x@y.com",
|
||||
"gaps_open_count": 1,
|
||||
}
|
||||
target = sentinel.write(
|
||||
pr_number=30, work_type="pr_fix",
|
||||
work_group="failing_ci_pr", result=result,
|
||||
compliance_gaps=gaps_payload,
|
||||
)
|
||||
payload = json.loads(target.read_text())
|
||||
assert payload["compliance_gaps"] == gaps_payload
|
||||
assert payload["compliance_gaps_completed"] is True
|
||||
|
||||
|
||||
def test_write_includes_gate_preflight_when_passed(
|
||||
sentinel, handoff_dir,
|
||||
):
|
||||
result = _FakeResult(
|
||||
head_sha="bbb",
|
||||
pr_details={"title": "T", "body": "B",
|
||||
"head": {"ref": "h"}, "base": {"ref": "b"}},
|
||||
diff_text="diff",
|
||||
)
|
||||
preflight_payload = {
|
||||
"preflight_enabled": True,
|
||||
"gate_statuses": {"unit_tests": "FAIL"},
|
||||
"failures_total": 1,
|
||||
"related": [],
|
||||
"unrelated": [{"path": "features/x.feature", "line": "1"}],
|
||||
"runs": [{"run": 1, "returncode": 1, "failures": 1}],
|
||||
"flakes_filtered": 0,
|
||||
}
|
||||
target = sentinel.write(
|
||||
pr_number=31, work_type="pr_fix",
|
||||
work_group="failing_ci_pr", result=result,
|
||||
gate_preflight=preflight_payload,
|
||||
)
|
||||
payload = json.loads(target.read_text())
|
||||
assert payload["gate_preflight"] == preflight_payload
|
||||
assert payload["gate_preflight_completed"] is True
|
||||
|
||||
|
||||
def test_write_omits_deterministic_sections_when_not_passed(
|
||||
sentinel, handoff_dir,
|
||||
):
|
||||
"""Backwards compatibility: legacy callers that don't pass
|
||||
``compliance_gaps`` or ``gate_preflight`` get a sentinel
|
||||
without those keys (not a sentinel with ``None`` placeholders).
|
||||
The reader's gating logic keys off ``*_completed`` flag absence
|
||||
to fall through to in-session discovery."""
|
||||
result = _FakeResult(
|
||||
head_sha="ccc",
|
||||
pr_details={"title": "T", "body": "B",
|
||||
"head": {"ref": "h"}, "base": {"ref": "b"}},
|
||||
diff_text="diff",
|
||||
)
|
||||
target = sentinel.write(
|
||||
pr_number=32, work_type="pr_fix",
|
||||
work_group="failing_ci_pr", result=result,
|
||||
)
|
||||
payload = json.loads(target.read_text())
|
||||
assert "compliance_gaps" not in payload
|
||||
assert "compliance_gaps_completed" not in payload
|
||||
assert "gate_preflight" not in payload
|
||||
assert "gate_preflight_completed" not in payload
|
||||
|
||||
|
||||
def test_delete_removes_existing_sentinel(sentinel, handoff_dir):
|
||||
handoff_dir.mkdir(parents=True, exist_ok=True)
|
||||
target = handoff_dir / "pr-30.json"
|
||||
|
||||
+44
-10
@@ -43,12 +43,26 @@ _BEHAVE_FAILURE_RE = re.compile(
|
||||
re.MULTILINE,
|
||||
)
|
||||
|
||||
# When a gate reports "FAILED" via the local_ci_gate.sh wrapper the
|
||||
# header line looks like ``CI / unit_tests* FAIL ...``. Used as a
|
||||
# coarse status indicator for gates the parser doesn't deep-parse.
|
||||
# Gate status lines from ``tools/local_ci_gate.sh``. The script
|
||||
# prints one of two shapes per gate (see ``local_ci_gate.sh:323+``):
|
||||
#
|
||||
# ## [unit_tests] start
|
||||
# ## [unit_tests] PASS (12s)
|
||||
# ## [unit_tests] FAIL (47s)
|
||||
#
|
||||
# We match the second-line shape because that's the one with the
|
||||
# verdict. The leading ``##`` is the script's section marker; the
|
||||
# ``(Ns)`` elapsed-time tail is informational and ignored here.
|
||||
#
|
||||
# Historically the regex expected ``unit_tests: PASS`` colon-style
|
||||
# output that no local script actually produces — every gate parsed
|
||||
# as missing and the worker saw an empty roll-up. The fix is to
|
||||
# match the real format. We keep ``SKIP`` in the alternation even
|
||||
# though the current script doesn't emit it; future gate additions
|
||||
# may.
|
||||
_GATE_STATUS_RE = re.compile(
|
||||
r"^(?P<gate>lint|typecheck|unit_tests|integration_tests|"
|
||||
r"e2e_tests|coverage)\s*:\s*(?P<status>PASS|FAIL|SKIP)",
|
||||
r"^##\s*\[(?P<gate>lint|typecheck|unit_tests|integration_tests|"
|
||||
r"e2e_tests|coverage)\]\s+(?P<status>PASS|FAIL|SKIP)\b",
|
||||
re.MULTILINE | re.IGNORECASE,
|
||||
)
|
||||
|
||||
@@ -131,22 +145,42 @@ def _feature_likely_related(
|
||||
# filenames are often verbose ("plan_cli_commands_r2_boost") and
|
||||
# matching the WHOLE stem against a path would miss legitimate
|
||||
# matches like ``plan_cli/commands.py``.
|
||||
tokens = stem.split("_")
|
||||
#
|
||||
# Split on BOTH ``_`` and ``-`` so source files that use the
|
||||
# hyphenated convention (``plan-cli-commands.py``) match a
|
||||
# feature file using the underscored convention
|
||||
# (``plan_cli_commands.feature``). Without the hyphen split,
|
||||
# a feature ``auto-debug-cli`` (rare but legal in features/)
|
||||
# would tokenise as a single string and miss every legitimate
|
||||
# source-path hit.
|
||||
tokens = re.split(r"[_\-]", stem)
|
||||
# Take the first 2 tokens as the "salient prefix" — the bit
|
||||
# that's most likely to map to a source-tree path component.
|
||||
prefix_candidates = []
|
||||
# Each two-token prefix is generated in BOTH the underscored
|
||||
# and hyphenated forms so a feature ``plan_cli_commands.feature``
|
||||
# matches a source path ``src/plan-cli/commands.py``.
|
||||
prefix_candidates: list[str] = []
|
||||
if tokens:
|
||||
prefix_candidates.append(tokens[0])
|
||||
if len(tokens) >= 2:
|
||||
prefix_candidates.append("_".join(tokens[:2]))
|
||||
prefix_candidates.append("-".join(tokens[:2]))
|
||||
|
||||
for prefix in prefix_candidates:
|
||||
if not prefix:
|
||||
continue
|
||||
for changed in changed_norm:
|
||||
# Match against PATH SEGMENTS — avoid matching substrings
|
||||
# of unrelated names (``plan`` matches ``plans/`` but
|
||||
# not ``misplanned``).
|
||||
# Match against PATH SEGMENTS only — do NOT further
|
||||
# split each segment on ``[_\-]``. Earlier iterations
|
||||
# exploded each segment into its sub-tokens, which made
|
||||
# a diff in ``src/plan-cli/commands.py`` (segments
|
||||
# ``["src", "plan-cli", "commands", "py"]``) match
|
||||
# every feature whose stem started with ``plan`` OR
|
||||
# ``cli`` OR ``commands`` — including unrelated
|
||||
# ``cli_extensions.feature``. The fix: keep the segment
|
||||
# set literal, and rely on ``prefix_candidates`` already
|
||||
# spanning both underscored and hyphenated forms to
|
||||
# bridge the naming-convention gap.
|
||||
segments = changed.replace(".", "/").split("/")
|
||||
if prefix in segments:
|
||||
return True
|
||||
|
||||
@@ -33,11 +33,14 @@ work — easy to unit-test against fixture trees.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import re
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
_logger = logging.getLogger("implementer_compliance")
|
||||
|
||||
|
||||
def _git(args: list[str], cwd: Path, timeout: int = 10) -> tuple[int, str]:
|
||||
"""Run a ``git`` subprocess inside ``cwd`` and return
|
||||
@@ -64,19 +67,38 @@ def check_worktree_clean(worktree: Path) -> bool:
|
||||
would get pushed if the agent ran ``git push``. The agent should
|
||||
NOT re-apply code changes when this is True; the diff is already
|
||||
present in the committed history.
|
||||
|
||||
Git-unavailable / non-repo handling: a non-zero return code or a
|
||||
missing ``git`` binary returns ``True`` (treat as "no observable
|
||||
gap"), not ``False``. Returning ``False`` would conflate "we
|
||||
couldn't check" with "there IS a gap" — the worker would then
|
||||
chase a phantom dirty-worktree and re-apply the code fix on top
|
||||
of HEAD. Callers that need to distinguish the masked-True from a
|
||||
real-True must use :func:`check_compliance_gaps_with_masking`.
|
||||
"""
|
||||
rc, out = _git(["status", "--porcelain"], worktree)
|
||||
if rc != 0:
|
||||
return False
|
||||
return out.strip() == ""
|
||||
value, _was_masked = _check_worktree_clean_with_masking(worktree)
|
||||
return value
|
||||
|
||||
|
||||
# Regex to find the ``[Unreleased]`` section and the next ``## `` or
|
||||
# end-of-file. Multi-line, ungreedy. The section is "non-empty" if
|
||||
# anything but whitespace + horizontal rules appears between the
|
||||
# header and the next section.
|
||||
#
|
||||
# Exact alternation across the two accepted forms:
|
||||
#
|
||||
# - Keep-a-Changelog form: ``## [Unreleased]``
|
||||
# - Bare-header form: ``## Unreleased``
|
||||
#
|
||||
# The previous regex used ``\[?Unreleased\]?`` which silently
|
||||
# accepted mismatched brackets (``## [Unreleased`` or
|
||||
# ``## Unreleased]``) — a malformed header that should be flagged
|
||||
# as missing the section, not silently matched. The non-capturing
|
||||
# group makes the alternation explicit so a future contributor
|
||||
# adding a third form (e.g. ``## (Unreleased)``) sees the pattern
|
||||
# and knows to extend it deliberately.
|
||||
_UNRELEASED_SECTION_RE = re.compile(
|
||||
r"^##\s*\[Unreleased\][^\n]*\n(?P<body>.*?)(?=^##\s|\Z)",
|
||||
r"^##\s*(?:\[Unreleased\]|Unreleased)[^\n]*\n(?P<body>.*?)(?=^##\s|\Z)",
|
||||
re.MULTILINE | re.DOTALL | re.IGNORECASE,
|
||||
)
|
||||
|
||||
@@ -147,11 +169,14 @@ def check_commit_has_issues_closed(worktree: Path) -> bool:
|
||||
insensitive. The actual project convention uses uppercase
|
||||
``ISSUES CLOSED:`` but we accept variants like
|
||||
``Issues closed: 42`` for robustness.
|
||||
|
||||
Git-unavailable / non-repo handling: same rule as
|
||||
:func:`check_worktree_clean` — a non-zero git return code returns
|
||||
``True``. Callers that need to distinguish the masked-True from
|
||||
a real-True must use :func:`check_compliance_gaps_with_masking`.
|
||||
"""
|
||||
rc, body = _git(["log", "-1", "--pretty=%B", "HEAD"], worktree)
|
||||
if rc != 0:
|
||||
return False
|
||||
return _ISSUES_CLOSED_RE.search(body) is not None
|
||||
value, _was_masked = _check_commit_has_issues_closed_with_masking(worktree)
|
||||
return value
|
||||
|
||||
|
||||
def check_compliance_gaps(
|
||||
@@ -161,17 +186,124 @@ def check_compliance_gaps(
|
||||
|
||||
Each value is a strict ``bool``. The agent's prompt instruction
|
||||
is to inspect this dict and fill in the missing items only.
|
||||
|
||||
See :func:`check_compliance_gaps_with_masking` for the
|
||||
masking-aware variant the dispatcher uses to detect when a
|
||||
``True`` value was synthesised because git failed (vs. a real
|
||||
passing check). Callers that only need the simple-bool dict
|
||||
(most unit tests) can keep using this thin wrapper.
|
||||
"""
|
||||
return {
|
||||
"worktree_clean": check_worktree_clean(worktree),
|
||||
gaps, _ = check_compliance_gaps_with_masking(worktree, git_user_email)
|
||||
return gaps
|
||||
|
||||
|
||||
def check_compliance_gaps_with_masking(
|
||||
worktree: Path, git_user_email: str = ""
|
||||
) -> tuple[dict[str, bool], set[str]]:
|
||||
"""Run all four checks and report which ones were MASKED.
|
||||
|
||||
Two checks ( :func:`check_worktree_clean` and
|
||||
:func:`check_commit_has_issues_closed`) return ``True`` when git
|
||||
itself fails — the deliberate "don't conflate masking with a real
|
||||
gap" policy documented on those functions. The cost of that
|
||||
policy: if BOTH masked checks coincide with a real-content
|
||||
CHANGELOG + CONTRIBUTORS the worker sees ``all_gaps_closed=True``
|
||||
and would receive a confident "PR resolved, do NOT re-apply"
|
||||
directive from the renderer.
|
||||
|
||||
This function returns both the gap dict AND the set of check
|
||||
names that were masked this run. Callers that surface the verdict
|
||||
to the worker (the dispatcher's ``_compute_deterministic_sections``
|
||||
and the pointer-stanza renderer) consult ``masked`` and hedge
|
||||
their language when it is non-empty. Pure unit-test callers can
|
||||
ignore the second element.
|
||||
"""
|
||||
masked: set[str] = set()
|
||||
worktree_clean, was_masked = _check_worktree_clean_with_masking(worktree)
|
||||
if was_masked:
|
||||
masked.add("worktree_clean")
|
||||
issues_closed, was_masked = _check_commit_has_issues_closed_with_masking(
|
||||
worktree
|
||||
)
|
||||
if was_masked:
|
||||
masked.add("commit_has_issues_closed")
|
||||
gaps = {
|
||||
"worktree_clean": worktree_clean,
|
||||
"changelog_unreleased_nonempty": check_changelog_unreleased_nonempty(
|
||||
worktree
|
||||
),
|
||||
"contributors_has_author": check_contributors_has_author(
|
||||
worktree, git_user_email
|
||||
),
|
||||
"commit_has_issues_closed": check_commit_has_issues_closed(worktree),
|
||||
"commit_has_issues_closed": issues_closed,
|
||||
}
|
||||
return gaps, masked
|
||||
|
||||
|
||||
def _check_worktree_clean_with_masking(worktree: Path) -> tuple[bool, bool]:
|
||||
"""Internal helper: returns ``(value, was_masked)``.
|
||||
|
||||
``was_masked`` is True when the underlying ``git status`` call
|
||||
failed and the function returned ``True`` because of the
|
||||
conflation-avoidance policy. Used by
|
||||
:func:`check_compliance_gaps_with_masking` to surface the
|
||||
masking state to the renderer. The public
|
||||
:func:`check_worktree_clean` wraps this and discards the second
|
||||
element for backward compat.
|
||||
"""
|
||||
rc, out = _git(["status", "--porcelain"], worktree)
|
||||
if rc != 0:
|
||||
_logger.info(
|
||||
"check_worktree_clean: git status returned rc=%d in %s; "
|
||||
"masking failure (returning True to avoid conflation "
|
||||
"with a real gap)",
|
||||
rc, worktree,
|
||||
)
|
||||
return True, True
|
||||
return out.strip() == "", False
|
||||
|
||||
|
||||
def _check_commit_has_issues_closed_with_masking(
|
||||
worktree: Path,
|
||||
) -> tuple[bool, bool]:
|
||||
"""Internal helper mirror of
|
||||
:func:`_check_worktree_clean_with_masking` for the commit-footer
|
||||
check. Same masking contract."""
|
||||
rc, body = _git(["log", "-1", "--pretty=%B", "HEAD"], worktree)
|
||||
if rc != 0:
|
||||
_logger.info(
|
||||
"check_commit_has_issues_closed: git log returned rc=%d "
|
||||
"in %s; masking failure (returning True to avoid "
|
||||
"conflation with a real gap)",
|
||||
rc, worktree,
|
||||
)
|
||||
return True, True
|
||||
return _ISSUES_CLOSED_RE.search(body) is not None, False
|
||||
|
||||
|
||||
def gaps_open_count(
|
||||
gaps: dict[str, bool], masked_checks: set[str] | None = None,
|
||||
) -> int:
|
||||
"""Number of compliance checks that returned ``False`` — i.e.
|
||||
real observable gaps the worker needs to fill.
|
||||
|
||||
When ``masked_checks`` is provided, MASKED checks are excluded
|
||||
from the count entirely (neither "open" nor "closed" — their
|
||||
value is unverified, not a real signal). Without the
|
||||
``masked_checks`` argument the function returns the raw count
|
||||
of False values, which over-counts the truly-open set when
|
||||
masking happened.
|
||||
|
||||
Convenience for telemetry / status comments: lets analysts see
|
||||
"how much real compliance debt was discovered this cycle"
|
||||
without re-parsing the dict. ``0`` when all checks pass; up to
|
||||
``len(gaps)`` when none do.
|
||||
"""
|
||||
masked = masked_checks or set()
|
||||
return sum(
|
||||
1 for k, v in gaps.items()
|
||||
if not v and k not in masked
|
||||
)
|
||||
|
||||
|
||||
def all_gaps_closed(gaps: dict[str, bool]) -> bool:
|
||||
@@ -181,22 +313,36 @@ def all_gaps_closed(gaps: dict[str, bool]) -> bool:
|
||||
|
||||
|
||||
def render_prompt_stanza(
|
||||
gaps: dict[str, bool], pr_number: int | None = None
|
||||
gaps: dict[str, bool],
|
||||
pr_number: int | None = None,
|
||||
*,
|
||||
masked_checks: list[str] | set[str] | None = None,
|
||||
) -> str:
|
||||
"""Render the compliance-gap report as a markdown stanza for
|
||||
the worker prompt.
|
||||
|
||||
The instruction text tells the agent how to react:
|
||||
|
||||
- All True → emit the success-JSON and exit (no code changes
|
||||
needed).
|
||||
- All True (no masking) → emit the success-JSON and exit (no
|
||||
code changes needed).
|
||||
- All True (with masking) → hedge: re-verify the masked check(s)
|
||||
before exiting; the dispatcher returned True for them because
|
||||
git itself failed, not because they really passed.
|
||||
- Some False → fill the missing items only.
|
||||
|
||||
Keeps the rendering co-located with the data shape so a future
|
||||
schema change lands in one place.
|
||||
schema change lands in one place. Note: this module-level
|
||||
renderer is paralleled by
|
||||
:func:`dispatch_implementer._render_compliance_pointer_stanza`
|
||||
which produces the short, prompt-survival-optimised pointer that
|
||||
production actually sends. Both must stay in sync on the
|
||||
all-passed-with-masking hedge — otherwise a worker reading the
|
||||
sentinel JSON sees one verdict and a worker reading the prompt
|
||||
sees another.
|
||||
"""
|
||||
if not gaps:
|
||||
return ""
|
||||
masked = set(masked_checks or [])
|
||||
|
||||
lines = ["## Compliance gap report (deterministic)\n"]
|
||||
lines.append(
|
||||
@@ -207,10 +353,11 @@ def render_prompt_stanza(
|
||||
lines.append("")
|
||||
for key, value in sorted(gaps.items()):
|
||||
marker = "✅" if value else "❌"
|
||||
lines.append(f"- {marker} `{key}`: **{value}**")
|
||||
masked_note = " _(masked — git failed)_" if key in masked else ""
|
||||
lines.append(f"- {marker} `{key}`: **{value}**{masked_note}")
|
||||
lines.append("")
|
||||
|
||||
if all_gaps_closed(gaps):
|
||||
if all_gaps_closed(gaps) and not masked:
|
||||
lines.append(
|
||||
"**All checks passed.** The PR is already complete — "
|
||||
"the code fix is in HEAD and every Compliance Checklist "
|
||||
@@ -221,6 +368,15 @@ def render_prompt_stanza(
|
||||
lines.append(
|
||||
" {\"outcome\": \"resolved\", \"files_touched\": []}"
|
||||
)
|
||||
elif all_gaps_closed(gaps) and masked:
|
||||
joined_masked = ", ".join(f"`{m}`" for m in sorted(masked))
|
||||
lines.append(
|
||||
f"**All OBSERVABLE checks passed, but {len(masked)} "
|
||||
f"check(s) were MASKED** because `git` itself failed: "
|
||||
f"{joined_masked}. Do NOT exit with `resolved` — "
|
||||
"re-verify the masked check(s) in-session (run `git "
|
||||
"status` / `git log -1`) before deciding."
|
||||
)
|
||||
else:
|
||||
missing = [k for k, v in gaps.items() if not v]
|
||||
lines.append(
|
||||
|
||||
@@ -64,6 +64,27 @@ _logger = logging.getLogger("implementer_gate_preflight")
|
||||
# Env var controlling whether the pre-flight runs at all. Default
|
||||
# OFF so the dispatcher's pre-feature behaviour is byte-equivalent.
|
||||
# Set ``IMPLEMENTER_DISPATCHER_GATE_PREFLIGHT=1`` to activate.
|
||||
#
|
||||
# **Independent of ``IMPLEMENTER_ESCALATION_ENABLED``.** The
|
||||
# preflight runs whenever this flag is on, regardless of escalation
|
||||
# state — the result lands in the PR-context sentinel for the
|
||||
# worker to read on its first turn, which is useful even outside
|
||||
# the escalation loop. Operational consequence: enabling preflight
|
||||
# alone (without escalation) adds 2× ``local_ci_gate.sh --fast``
|
||||
# wallclock (~6 min cold-cache, ~1 min warm) to every dispatched
|
||||
# cycle. Enable both together unless you have a specific reason to
|
||||
# decouple them.
|
||||
#
|
||||
# **Heartbeat / watchdog implication:** the preflight runs inside
|
||||
# the dispatcher's ``prompt_factory`` before
|
||||
# :func:`_dispatch_runtime._refresh_heartbeat` is established for
|
||||
# the cycle. The cold-cache ~6-min window is silent from the
|
||||
# systemd watchdog's perspective and stacks with the existing
|
||||
# prefetch + preclone pre-heartbeat wallclock. Operators enabling
|
||||
# this flag should ensure the dispatcher's systemd watchdog
|
||||
# interval comfortably exceeds the worst-case pre-heartbeat
|
||||
# budget — bumping ``WatchdogSec`` to 15-20 min when this flag is
|
||||
# on is the conservative move.
|
||||
PREFLIGHT_ENABLED_ENV_VAR = "IMPLEMENTER_DISPATCHER_GATE_PREFLIGHT"
|
||||
|
||||
# Per-invocation timeout for ``local_ci_gate.sh --fast``. 20 minutes
|
||||
@@ -88,6 +109,19 @@ def is_preflight_enabled() -> bool:
|
||||
|
||||
_TRUTHY = frozenset({"1", "true", "yes", "on"})
|
||||
|
||||
# Magic returncode sentinel for the timeout case. Distinct from
|
||||
# the generic non-zero codes the gate script itself emits on test
|
||||
# failure (it uses 1 / 2) AND from the negative signal-handling
|
||||
# codes Python's ``subprocess`` returns when the child dies from a
|
||||
# signal (e.g. ``-15`` for SIGTERM, ``-9`` for SIGKILL — values in
|
||||
# roughly ``-1..-64``). ``-9999`` is well outside that range so a
|
||||
# real subprocess can never produce it, eliminating the collision
|
||||
# risk of the earlier ``-1`` sentinel. The orchestrator
|
||||
# (:func:`run_preflight`) checks ``rc == _TIMEOUT_RETURNCODE`` to
|
||||
# surface ``preflight_timeout=True`` in the classification rather
|
||||
# than treating timeout-with-no-parsed-failures as "all green".
|
||||
_TIMEOUT_RETURNCODE = -9999
|
||||
|
||||
|
||||
def _run_gate_once(
|
||||
worktree: Path,
|
||||
@@ -102,6 +136,14 @@ def _run_gate_once(
|
||||
non-zero exit codes (the gate exits 1 on test failure, which is
|
||||
the case we WANT to capture and parse).
|
||||
|
||||
On :class:`subprocess.TimeoutExpired` the function returns
|
||||
``(_TIMEOUT_RETURNCODE, "<TIMEOUT after Xs>")``. The marker line
|
||||
is what :func:`run_preflight` keys off to set
|
||||
``preflight_timeout=True`` in the classification — without it,
|
||||
the classifier saw no parseable failures in the timeout text
|
||||
and (incorrectly) reported "no failures persisted", giving the
|
||||
worker a silent green light on a wedged gate run.
|
||||
|
||||
The ``runner`` indirection exists so unit tests can substitute
|
||||
a function that returns scripted output without invoking the
|
||||
real script.
|
||||
@@ -131,7 +173,7 @@ def _run_gate_once(
|
||||
"gate pre-flight timed out after %.1fs running %s",
|
||||
elapsed, " ".join(cmd),
|
||||
)
|
||||
return -1, f"<TIMEOUT after {elapsed:.1f}s>"
|
||||
return _TIMEOUT_RETURNCODE, f"<TIMEOUT after {elapsed:.1f}s>"
|
||||
return result.returncode, (result.stdout or "") + (result.stderr or "")
|
||||
|
||||
|
||||
@@ -197,6 +239,31 @@ def run_preflight(
|
||||
run_summaries: list[dict[str, object]] = [
|
||||
{"run": 1, "returncode": rc1, "failures": len(failures1)}
|
||||
]
|
||||
timed_out_run1 = rc1 == _TIMEOUT_RETURNCODE
|
||||
|
||||
# ─── Timeout short-circuit ─────────────────────────────────
|
||||
# When the first run timed out we cannot deterministically
|
||||
# classify failures (the output is a marker line, not gate
|
||||
# text). Returning the empty classification would render as
|
||||
# "no persistent failures" to the worker — a silent green
|
||||
# light. Instead, surface ``preflight_timeout=True`` so the
|
||||
# renderer warns the worker that classification is unreliable.
|
||||
# Counts/lists are explicitly zeroed so the renderer cannot
|
||||
# show a contradictory "Persistent failures: N" line next to
|
||||
# the timeout warning — the classification IS suppressed in
|
||||
# this branch, and the payload should say so unambiguously.
|
||||
if timed_out_run1:
|
||||
return {
|
||||
"runs": run_summaries,
|
||||
"preflight_enabled": True,
|
||||
"preflight_timeout": True,
|
||||
"gate_statuses": {},
|
||||
"failures_total": 0,
|
||||
"failures_related_to_diff": 0,
|
||||
"failures_unrelated_to_diff": 0,
|
||||
"related": [],
|
||||
"unrelated": [],
|
||||
}
|
||||
|
||||
if rc1 == 0 and not failures1:
|
||||
# All gates passed on the first attempt — no need to re-run.
|
||||
@@ -211,22 +278,48 @@ def run_preflight(
|
||||
run_summaries.append(
|
||||
{"run": 2, "returncode": rc2, "failures": len(failures2)}
|
||||
)
|
||||
timed_out_run2 = rc2 == _TIMEOUT_RETURNCODE
|
||||
|
||||
# Persistent = present in BOTH runs. Synthesize a synthetic
|
||||
# "gate output" containing only the persistent failures so the
|
||||
# classifier doesn't accidentally see flaky ones too. The
|
||||
# synthetic output uses the same shape parse_failing_scenarios
|
||||
# consumes (``features/path.feature:N``).
|
||||
if timed_out_run2:
|
||||
# Run 1 produced parseable output but run 2 timed out — we
|
||||
# can't intersect to find persistent failures, so we treat
|
||||
# the whole pre-flight as inconclusive (same reasoning as
|
||||
# the run-1 timeout branch). Surface run-1's gate_statuses
|
||||
# roll-up (it IS real data) but suppress the failure
|
||||
# counts: without a second run we cannot distinguish flakes
|
||||
# from persistent failures, and emitting the run-1 list as
|
||||
# if it were persistent would mislead the worker. The
|
||||
# ``preflight_timeout`` flag + zeroed counts give the
|
||||
# renderer one unambiguous story.
|
||||
run1_classification = _diff_aware_gate.classify_failures(
|
||||
out1, changed_files
|
||||
)
|
||||
return {
|
||||
"runs": run_summaries,
|
||||
"preflight_enabled": True,
|
||||
"preflight_timeout": True,
|
||||
"gate_statuses": run1_classification.get("gate_statuses") or {},
|
||||
"failures_total": 0,
|
||||
"failures_related_to_diff": 0,
|
||||
"failures_unrelated_to_diff": 0,
|
||||
"related": [],
|
||||
"unrelated": [],
|
||||
}
|
||||
|
||||
# Persistent = present in BOTH runs. Track flake count
|
||||
# (failed run 1, passed run 2) so the worker / telemetry can
|
||||
# see how much the dispatcher filtered out.
|
||||
persistent = failures1 & failures2
|
||||
flakes_filtered = len(failures1 - failures2)
|
||||
synthetic_lines = [f" {p}:{n}" for (p, n) in sorted(persistent)]
|
||||
# Preserve the gate-status header from the second run's output
|
||||
# so the classifier still emits a gate_statuses summary.
|
||||
# so the classifier still emits a gate_statuses summary. The
|
||||
# gate emits ``## [unit_tests] PASS (12s)`` lines (see
|
||||
# _GATE_STATUS_RE in _diff_aware_gate.py); we filter out
|
||||
# non-status lines while keeping ``## [gate]`` rows.
|
||||
statuses_text = "\n".join(
|
||||
line for line in (out2 or "").splitlines()
|
||||
if any(g in line.lower() for g in (
|
||||
"lint:", "typecheck:", "unit_tests:",
|
||||
"integration_tests:", "e2e_tests:", "coverage:",
|
||||
))
|
||||
if line.lstrip().startswith("## [")
|
||||
)
|
||||
synthetic_output = (
|
||||
statuses_text
|
||||
@@ -236,7 +329,9 @@ def run_preflight(
|
||||
classification = _diff_aware_gate.classify_failures(
|
||||
synthetic_output, changed_files
|
||||
)
|
||||
classification.update(
|
||||
{"runs": run_summaries, "preflight_enabled": True}
|
||||
)
|
||||
classification.update({
|
||||
"runs": run_summaries,
|
||||
"preflight_enabled": True,
|
||||
"flakes_filtered": flakes_filtered,
|
||||
})
|
||||
return classification
|
||||
|
||||
@@ -162,6 +162,7 @@ def extract_phase4_telemetry(
|
||||
tier_attempt_index: int | None = None,
|
||||
escalation_action: str | None = None,
|
||||
escalation_tier_hint: int | None = None,
|
||||
outcome_synthesised: bool | None = None,
|
||||
) -> dict[str, Any]:
|
||||
"""Build a Phase 4 telemetry row from a completed cycle.
|
||||
|
||||
@@ -238,6 +239,15 @@ def extract_phase4_telemetry(
|
||||
row["escalation_action"] = str(escalation_action)
|
||||
if escalation_tier_hint is not None:
|
||||
row["escalation_tier_hint"] = int(escalation_tier_hint)
|
||||
if outcome_synthesised is not None:
|
||||
# Explicit ``bool(...)`` coercion: the kwarg is typed
|
||||
# ``bool | None`` and the ``is not None`` guard above
|
||||
# narrows it to ``bool``, but a future caller passing a
|
||||
# truthy non-bool (e.g. ``1`` from a JSON deserialiser)
|
||||
# would still land here. The coercion normalises the row
|
||||
# to a real ``bool`` for downstream JSONL consumers that
|
||||
# filter on ``row["outcome_synthesised"] is True``.
|
||||
row["outcome_synthesised"] = bool(outcome_synthesised)
|
||||
return row
|
||||
|
||||
|
||||
|
||||
@@ -272,6 +272,8 @@ def write(
|
||||
work_group: str,
|
||||
result: Any,
|
||||
item: dict[str, Any] | None = None,
|
||||
compliance_gaps: dict[str, Any] | None = None,
|
||||
gate_preflight: dict[str, Any] | None = None,
|
||||
) -> Path | None:
|
||||
"""Serialise the prefetch result for ``pr_number`` and write it
|
||||
atomically to disk.
|
||||
@@ -283,6 +285,16 @@ def write(
|
||||
``item`` is forwarded only for the optional metadata block
|
||||
(PR title, listing-time labels) — never reach back to Forgejo.
|
||||
|
||||
``compliance_gaps`` and ``gate_preflight`` are the deterministic
|
||||
pre-check artefacts the dispatcher computes after prefetch (see
|
||||
:mod:`_implementer_compliance` and :mod:`_implementer_gate_preflight`).
|
||||
Passing them here serialises them into the sentinel under the
|
||||
same JSON schema the worker reads via
|
||||
``implementer_pr_context.py --field {compliance_gaps,gate_preflight}``.
|
||||
Markdown stanzas in the prompt body get summarised away by the
|
||||
intermediate tier agents (see :file:`task-implementor.md`); the
|
||||
sentinel survives because the worker reads it directly off disk.
|
||||
|
||||
Returns the resolved path on success, or ``None`` on any I/O
|
||||
or serialisation failure (and logs WARNING). The dispatcher's
|
||||
caller treats a ``None`` return as best-effort and proceeds
|
||||
@@ -295,7 +307,7 @@ def write(
|
||||
pr_number = int(pr_number)
|
||||
target = handoff_path(pr_number)
|
||||
item = item or {}
|
||||
payload = {
|
||||
payload: dict[str, Any] = {
|
||||
"schema_version": SCHEMA_VERSION,
|
||||
"pr_number": pr_number,
|
||||
"work_type": work_type,
|
||||
@@ -305,6 +317,26 @@ def write(
|
||||
"listing_title": str(item.get("title") or ""),
|
||||
**_to_dict(result),
|
||||
}
|
||||
# Deterministic-check sections (plan 2026-05-13). Binary
|
||||
# contract: either the dispatcher computed the section this
|
||||
# cycle (both the value AND the ``*_completed`` flag are
|
||||
# written) or it did not (both keys are absent — worker's
|
||||
# reader treats absence as _MISSING and falls through to its
|
||||
# in-session discovery flow).
|
||||
#
|
||||
# No tri-state. An earlier iteration documented a third
|
||||
# "tried-but-got-nothing" state (``completed=True`` with a
|
||||
# ``null`` payload) but the compliance + preflight detectors
|
||||
# always produce a structured dict when they run, so that
|
||||
# state is unreachable. Removing the language keeps the
|
||||
# reader's three-case generic contract (used by the prefetch
|
||||
# fields) from leaking into these binary fields.
|
||||
if compliance_gaps is not None:
|
||||
payload["compliance_gaps"] = compliance_gaps
|
||||
payload["compliance_gaps_completed"] = True
|
||||
if gate_preflight is not None:
|
||||
payload["gate_preflight"] = gate_preflight
|
||||
payload["gate_preflight_completed"] = True
|
||||
tmp = target.with_suffix(target.suffix + ".tmp")
|
||||
try:
|
||||
target.parent.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
+411
-61
@@ -76,9 +76,6 @@ _implementer_compliance = _load_sibling(
|
||||
_implementer_gate_preflight = _load_sibling(
|
||||
"_implementer_gate_preflight", "_implementer_gate_preflight.py"
|
||||
)
|
||||
_diff_aware_gate = _load_sibling(
|
||||
"_diff_aware_gate", "_diff_aware_gate.py"
|
||||
)
|
||||
_opencode_worker = _load_sibling("_opencode_worker", "_opencode_worker.py")
|
||||
|
||||
|
||||
@@ -122,6 +119,45 @@ ESCALATION_ENABLED_ENV_VAR = "IMPLEMENTER_ESCALATION_ENABLED"
|
||||
# reclaiming the PR mid-cycle.
|
||||
ESCALATION_TIER2_ENABLED_ENV_VAR = "IMPLEMENTER_ESCALATION_TIER2_ENABLED"
|
||||
|
||||
# Outcome-synthesis kill-switch. The escalation loop synthesises a
|
||||
# concrete outcome (rebase-failed / timeout / transport-error) when
|
||||
# the worker dies before emitting JSON, so the escalation predicate
|
||||
# sees a competence-class signal instead of falling into the UNKNOWN
|
||||
# bucket.
|
||||
#
|
||||
# **Default since 2026-05-13: ON when escalation is enabled.** Split
|
||||
# from ``IMPLEMENTER_ESCALATION_ENABLED`` so an operator who suspects
|
||||
# the synthesis is mis-routing a particular failure class can disable
|
||||
# it without turning off the whole escalation feature. Set
|
||||
# ``IMPLEMENTER_OUTCOME_SYNTHESIS=0`` to revert to the legacy
|
||||
# "UNKNOWN-bucket wastes one retry" behaviour.
|
||||
#
|
||||
# **Operational cost when set to 0:** the escalation predicate sees
|
||||
# UNKNOWN, which costs one wasted same-tier retry per cycle (the
|
||||
# worker is re-spawned with the same tier hint and almost always
|
||||
# fails the same way). Worst-case cycle wallclock rises by one
|
||||
# worker_timeout (typically ~7200 s). Operators flipping this off
|
||||
# should expect the cost and treat the flag as a panic-button, not
|
||||
# a default-state.
|
||||
OUTCOME_SYNTHESIS_ENV_VAR = "IMPLEMENTER_OUTCOME_SYNTHESIS"
|
||||
|
||||
# Compliance-gap detection kill-switch. The compliance section is
|
||||
# computed by :func:`_compute_deterministic_sections` and serialised
|
||||
# into the sentinel for the worker to read via
|
||||
# ``implementer_pr_context.py --field compliance_gaps``.
|
||||
#
|
||||
# **Default since 2026-05-13: ON whenever escalation is enabled.**
|
||||
# Set ``IMPLEMENTER_COMPLIANCE_GAPS_ENABLED=0`` to opt out.
|
||||
#
|
||||
# **Gating semantics: AND-only.** The compliance scan only runs when
|
||||
# BOTH ``IMPLEMENTER_ESCALATION_ENABLED`` is on AND this flag is
|
||||
# non-falsy. Setting this flag ON with escalation OFF does NOT
|
||||
# enable the scan in isolation — compliance is meaningless outside
|
||||
# the escalation loop's gap-filling flow (the worker has no retry
|
||||
# path to fix discovered gaps). This flag is a kill-switch for the
|
||||
# escalation-on case, not an independent feature toggle.
|
||||
COMPLIANCE_GAPS_ENABLED_ENV_VAR = "IMPLEMENTER_COMPLIANCE_GAPS_ENABLED"
|
||||
|
||||
|
||||
def _work_type_for_group(group_name: str) -> str:
|
||||
"""Map dispatcher work-group names to the ``work_type`` constant
|
||||
@@ -199,6 +235,39 @@ def _is_escalation_enabled() -> bool:
|
||||
return _env_truthy(ESCALATION_ENABLED_ENV_VAR)
|
||||
|
||||
|
||||
def _is_outcome_synthesis_enabled() -> bool:
|
||||
"""Return ``True`` when the dispatcher should synthesise an
|
||||
outcome JSON for workers that exited without emitting one.
|
||||
|
||||
Default-ON when ``IMPLEMENTER_OUTCOME_SYNTHESIS`` is unset; set
|
||||
to a falsy literal (``0`` / ``false`` / ``no`` / ``off``) to
|
||||
revert to legacy behaviour. Independent of
|
||||
:func:`_is_escalation_enabled` so the kill-switch can be flipped
|
||||
without disabling the whole escalation feature. The legacy path
|
||||
(``_post_session_action`` for non-PR work / when escalation is
|
||||
off) never calls the synthesiser regardless of this flag.
|
||||
"""
|
||||
if _env_falsy_explicit(OUTCOME_SYNTHESIS_ENV_VAR):
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
def _is_compliance_gaps_enabled() -> bool:
|
||||
"""Return ``True`` when the dispatcher should run the compliance
|
||||
gap detector and serialise the result into the sentinel.
|
||||
|
||||
Default-ON when ``IMPLEMENTER_COMPLIANCE_GAPS_ENABLED`` is unset
|
||||
(and escalation is enabled — the caller's responsibility); set
|
||||
to a falsy literal to opt out. The split from
|
||||
:func:`_is_escalation_enabled` lets an operator disable just
|
||||
the compliance scan if e.g. ``git`` is unreliable on the
|
||||
worktree, without losing tier escalation.
|
||||
"""
|
||||
if _env_falsy_explicit(COMPLIANCE_GAPS_ENABLED_ENV_VAR):
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
def _is_tier2_enabled() -> bool:
|
||||
"""Return ``True`` when Tier 2 (``tier-kimi``) is part of the
|
||||
escalation ladder.
|
||||
@@ -440,6 +509,17 @@ def _prefetch_prompt(
|
||||
# cycle would have to overwrite. The workspace handoff is
|
||||
# gated upstream in ``_build_clone_section``; this is the matching
|
||||
# gate for the PR-context handoff.
|
||||
# Compute deterministic sections (gate-preflight + compliance
|
||||
# gaps) BEFORE the sentinel write so they land in the same
|
||||
# payload the worker reads via ``implementer_pr_context.py``.
|
||||
# ``_compute_deterministic_sections`` returns a dict keyed by
|
||||
# ``compliance_gaps`` / ``gate_preflight`` (each value either a
|
||||
# plain dict or ``None``). Skipped in dry-run / when no worktree
|
||||
# was materialised.
|
||||
det_sections = _compute_deterministic_sections(
|
||||
cfg, item, group, result, clone_handle,
|
||||
)
|
||||
|
||||
if not cfg.dry_run:
|
||||
try:
|
||||
_pr_context_sentinel.write(
|
||||
@@ -448,6 +528,8 @@ def _prefetch_prompt(
|
||||
work_group=group.name,
|
||||
result=result,
|
||||
item=item,
|
||||
compliance_gaps=det_sections.get("compliance_gaps"),
|
||||
gate_preflight=det_sections.get("gate_preflight"),
|
||||
)
|
||||
except Exception as e: # noqa: BLE001 — best-effort
|
||||
_logger.warning(
|
||||
@@ -455,16 +537,117 @@ def _prefetch_prompt(
|
||||
pr_number, e,
|
||||
)
|
||||
|
||||
# Append deterministic-improvement stanzas (plan 2026-05-13).
|
||||
# Both are gated and skipped in dry-run / when no worktree was
|
||||
# materialised. Each appends a self-contained markdown section
|
||||
# the worker can read without changing the existing prompt body.
|
||||
# Append a brief sentinel-pointer stanza + (when the flags are
|
||||
# on) a short summary the agent can react to even if upstream
|
||||
# tier agents summarise the prompt body. The authoritative copy
|
||||
# lives in the sentinel; the prompt-side stanza is a fallback +
|
||||
# nudge to read the sentinel.
|
||||
text = _append_deterministic_stanzas(
|
||||
text, cfg, item, group, result, clone_handle,
|
||||
det_sections=det_sections,
|
||||
)
|
||||
return text
|
||||
|
||||
|
||||
def _compute_deterministic_sections(
|
||||
cfg: Any,
|
||||
item: dict[str, Any],
|
||||
group: Any,
|
||||
result: Any,
|
||||
clone_handle: Any,
|
||||
) -> dict[str, Any]:
|
||||
"""Run gate-preflight + compliance-gap detection and return a
|
||||
dict with the two keys ``gate_preflight`` and ``compliance_gaps``
|
||||
(each value is the section dict or absent).
|
||||
|
||||
Run-order rationale: gate-preflight first because the dispatcher
|
||||
pays its wallclock cost (two ``--fast`` runs) regardless of what
|
||||
compliance reports. Compliance is sub-100-ms and never blocks.
|
||||
|
||||
Skipped in dry-run / when no worktree was materialised — both
|
||||
sections need the pre-cloned worktree. Returns an empty dict in
|
||||
those cases (caller does not write either key into the sentinel).
|
||||
"""
|
||||
out: dict[str, Any] = {}
|
||||
if cfg.dry_run:
|
||||
return out
|
||||
worktree = getattr(clone_handle, "path", None)
|
||||
if not worktree:
|
||||
return out
|
||||
from pathlib import Path
|
||||
worktree_path = Path(str(worktree))
|
||||
if not worktree_path.exists():
|
||||
return out
|
||||
|
||||
pr_number = int(item.get("number") or 0)
|
||||
|
||||
# ─── Gate pre-flight (off-by-default; runs --fast twice) ────
|
||||
if _implementer_gate_preflight.is_preflight_enabled():
|
||||
try:
|
||||
changed_files = _collect_changed_files_from_result(result)
|
||||
classification = _implementer_gate_preflight.run_preflight(
|
||||
worktree_path, changed_files,
|
||||
)
|
||||
if isinstance(classification, dict):
|
||||
out["gate_preflight"] = classification
|
||||
except Exception as exc:
|
||||
_logger.warning(
|
||||
"gate pre-flight failed for #%s "
|
||||
"(continuing without the section): %s",
|
||||
pr_number, exc,
|
||||
)
|
||||
|
||||
# ─── Compliance gap detection ──────────────────────────────
|
||||
# Two-flag gate: requires the escalation feature to be on
|
||||
# (compliance is meaningless outside the escalation loop's
|
||||
# gap-filling flow) AND the per-feature opt-out
|
||||
# ``IMPLEMENTER_COMPLIANCE_GAPS_ENABLED`` to be non-falsy
|
||||
# (default-ON). The split lets an operator disable just the
|
||||
# compliance scan without losing tier escalation — useful
|
||||
# when ``git`` is misbehaving on the pre-cloned worktree.
|
||||
if _is_escalation_enabled() and _is_compliance_gaps_enabled():
|
||||
try:
|
||||
git_user_email = (
|
||||
os.environ.get("GIT_USER_EMAIL")
|
||||
or getattr(cfg, "git_user_email", "")
|
||||
or ""
|
||||
)
|
||||
gaps, masked_checks = (
|
||||
_implementer_compliance.check_compliance_gaps_with_masking(
|
||||
worktree_path, git_user_email,
|
||||
)
|
||||
)
|
||||
if isinstance(gaps, dict):
|
||||
out["compliance_gaps"] = {
|
||||
"gaps": gaps,
|
||||
"pr_number": pr_number or None,
|
||||
"git_user_email": git_user_email,
|
||||
# Masking-aware count: a downstream consumer
|
||||
# reading just ``gaps_open_count`` on a
|
||||
# fully-masked worktree would otherwise see 0
|
||||
# (because masked checks return True) and read
|
||||
# "all clean" on a tree the dispatcher couldn't
|
||||
# actually inspect. Passing the masked set
|
||||
# excludes those keys from the count entirely.
|
||||
"gaps_open_count": _implementer_compliance.gaps_open_count(
|
||||
gaps, masked_checks=masked_checks,
|
||||
),
|
||||
# The masked-checks list lets the renderer hedge
|
||||
# the "all passed" verdict when git itself failed
|
||||
# on one or more checks. Sorted for deterministic
|
||||
# serialisation in the sentinel.
|
||||
"masked_checks": sorted(masked_checks),
|
||||
}
|
||||
except Exception as exc:
|
||||
_logger.warning(
|
||||
"compliance gap detection failed for #%s "
|
||||
"(continuing without the section): %s",
|
||||
pr_number, exc,
|
||||
)
|
||||
|
||||
return out
|
||||
|
||||
|
||||
def _append_deterministic_stanzas(
|
||||
prompt: str,
|
||||
cfg: Any,
|
||||
@@ -472,21 +655,32 @@ def _append_deterministic_stanzas(
|
||||
group: Any,
|
||||
result: Any,
|
||||
clone_handle: Any,
|
||||
*,
|
||||
det_sections: dict[str, Any] | None = None,
|
||||
) -> str:
|
||||
"""Append the gate-preflight + compliance-gap stanzas to the
|
||||
prefetch prompt when their respective flags are on.
|
||||
"""Append a brief sentinel-pointer stanza (plus, when the
|
||||
matching flag is on, a condensed inline summary) for each
|
||||
deterministic section the dispatcher computed.
|
||||
|
||||
Both stanzas need the pre-cloned worktree's path — when the
|
||||
preclone is disabled or failed, we skip silently (the worker
|
||||
falls back to its in-session discovery flow). Both are off by
|
||||
default so the legacy prompt is byte-equivalent for operators
|
||||
who haven't opted in.
|
||||
Why a short stanza vs. the full markdown
|
||||
----------------------------------------
|
||||
The previous design embedded the full classification + per-gap
|
||||
hints in the prompt body. ``task-implementor.md:305`` is explicit
|
||||
that intermediate tier agents routinely summarise non-diff
|
||||
sections away before the worker sees them. The sentinel survives
|
||||
because the worker reads it directly off disk.
|
||||
|
||||
This stanza is intentionally short (one paragraph + one bullet
|
||||
list per section) so its survival probability through tier-agent
|
||||
summarisation is high. It tells the agent: (a) the read command,
|
||||
(b) the headline counts, (c) the per-gap hints. The full
|
||||
classification stays in the sentinel where the agent can drill in.
|
||||
|
||||
Order: gate-preflight stanza first (it tells the agent which
|
||||
test failures matter), then compliance stanza (it tells the
|
||||
agent what to fill in). When the compliance stanza reports
|
||||
"all gaps closed" the agent's exit path is clean even if the
|
||||
gate-preflight surfaced unrelated flakes.
|
||||
agent what to fill in). When compliance reports "all gaps
|
||||
closed" the agent's exit path is clean even if gate-preflight
|
||||
surfaced unrelated flakes.
|
||||
"""
|
||||
if cfg.dry_run:
|
||||
return prompt
|
||||
@@ -498,54 +692,193 @@ def _append_deterministic_stanzas(
|
||||
if not worktree_path.exists():
|
||||
return prompt
|
||||
|
||||
# When the caller did not pre-compute, do it here so legacy
|
||||
# callers (and test fixtures that hand-roll a clone_handle)
|
||||
# still produce a fully-populated stanza. Production callers
|
||||
# in :func:`_prefetch_prompt` pass ``det_sections`` so the same
|
||||
# computation runs once per cycle and lands in BOTH the sentinel
|
||||
# and the prompt — no double work. An earlier iteration emitted
|
||||
# a WARNING on this path to catch production-callsite regression;
|
||||
# removed because (a) the sole production caller is one line of
|
||||
# code, easily audited, and (b) the noise leaked into tests and
|
||||
# broke ``pytest -W error`` setups.
|
||||
if det_sections is None:
|
||||
det_sections = _compute_deterministic_sections(
|
||||
cfg, item, group, result, clone_handle,
|
||||
)
|
||||
|
||||
pr_number = int(item.get("number") or 0)
|
||||
extras: list[str] = []
|
||||
|
||||
# ─── Gate pre-flight (off-by-default; runs --fast twice) ────
|
||||
if _implementer_gate_preflight.is_preflight_enabled():
|
||||
try:
|
||||
changed_files = _collect_changed_files_from_result(result)
|
||||
classification = _implementer_gate_preflight.run_preflight(
|
||||
worktree_path, changed_files,
|
||||
)
|
||||
stanza = _diff_aware_gate.render_prompt_stanza(classification)
|
||||
if stanza:
|
||||
extras.append(stanza)
|
||||
except Exception as exc:
|
||||
_logger.warning(
|
||||
"gate pre-flight failed for #%s "
|
||||
"(continuing without the stanza): %s",
|
||||
pr_number, exc,
|
||||
)
|
||||
# ─── Gate pre-flight summary (always before compliance) ─────
|
||||
# ``_compute_deterministic_sections`` only sets ``gate_preflight``
|
||||
# when ``is_preflight_enabled()`` is True; ``run_preflight`` then
|
||||
# always returns at least ``{"preflight_enabled": True, ...}``
|
||||
# so the dict-shape check alone is sufficient. (An earlier
|
||||
# iteration OR'd over three truthy fields as defensive coverage
|
||||
# against an unreachable caller — removed for clarity.)
|
||||
preflight = det_sections.get("gate_preflight")
|
||||
if isinstance(preflight, dict):
|
||||
extras.append(_render_preflight_pointer_stanza(preflight, pr_number))
|
||||
|
||||
# ─── Compliance gap detection (gated on escalation flag) ───
|
||||
if _is_escalation_enabled():
|
||||
try:
|
||||
git_user_email = (
|
||||
os.environ.get("GIT_USER_EMAIL")
|
||||
or getattr(cfg, "git_user_email", "")
|
||||
or ""
|
||||
)
|
||||
gaps = _implementer_compliance.check_compliance_gaps(
|
||||
worktree_path, git_user_email,
|
||||
)
|
||||
stanza = _implementer_compliance.render_prompt_stanza(
|
||||
gaps, pr_number=pr_number or None,
|
||||
)
|
||||
if stanza:
|
||||
extras.append(stanza)
|
||||
except Exception as exc:
|
||||
_logger.warning(
|
||||
"compliance gap detection failed for #%s "
|
||||
"(continuing without the stanza): %s",
|
||||
pr_number, exc,
|
||||
)
|
||||
# ─── Compliance summary (always after gate-preflight) ───────
|
||||
compliance = det_sections.get("compliance_gaps")
|
||||
if isinstance(compliance, dict) and isinstance(
|
||||
compliance.get("gaps"), dict
|
||||
):
|
||||
masked_checks = compliance.get("masked_checks") or []
|
||||
extras.append(_render_compliance_pointer_stanza(
|
||||
compliance["gaps"], pr_number,
|
||||
masked_checks=list(masked_checks),
|
||||
))
|
||||
|
||||
if not extras:
|
||||
return prompt
|
||||
return prompt + "\n\n" + "\n\n".join(extras) + "\n"
|
||||
|
||||
|
||||
def _render_preflight_pointer_stanza(
|
||||
preflight: dict[str, Any], pr_number: int,
|
||||
) -> str:
|
||||
"""Brief gate-preflight summary + sentinel-read hint.
|
||||
|
||||
Surfaces preflight-timeout explicitly: the underlying
|
||||
:func:`_implementer_gate_preflight._run_gate_once` returns
|
||||
``preflight_timeout=True`` on subprocess.TimeoutExpired. Without
|
||||
surfacing it here, the renderer would emit a clean "no failures"
|
||||
summary on a silent timeout (the previous bug).
|
||||
|
||||
Defensive short-circuit: ``preflight_enabled=False`` is the
|
||||
documented disabled-shape from :func:`run_preflight`. Production
|
||||
never lands here (``_compute_deterministic_sections`` already
|
||||
gates on ``is_preflight_enabled()`` before computing), but a
|
||||
future caller writing the disabled-shape into the sentinel
|
||||
would otherwise trigger the misleading "no persistent failures"
|
||||
green-light line. Treat disabled-shape as "no stanza."
|
||||
"""
|
||||
if not preflight.get("preflight_enabled"):
|
||||
return ""
|
||||
statuses = preflight.get("gate_statuses") or {}
|
||||
failures_total = int(preflight.get("failures_total") or 0)
|
||||
related = preflight.get("related") or []
|
||||
unrelated = preflight.get("unrelated") or []
|
||||
timeout = bool(preflight.get("preflight_timeout"))
|
||||
lines = ["## Pre-flight gate summary (read sentinel for detail)"]
|
||||
lines.append("")
|
||||
if timeout:
|
||||
# Timeout payloads zero the counts in the orchestrator so
|
||||
# the renderer can't double-message; this stanza tells the
|
||||
# worker the classification is suppressed and why.
|
||||
lines.append(
|
||||
"**Pre-flight timed out** — the dispatcher could not "
|
||||
"deterministically classify failures this cycle. Treat "
|
||||
"every gate failure you see in-session as potentially real."
|
||||
)
|
||||
lines.append(
|
||||
"_Classification suppressed: persistent-failure counts "
|
||||
"are not reported on timeout._"
|
||||
)
|
||||
if statuses:
|
||||
roll = ", ".join(
|
||||
f"`{g}`={s}" for g, s in sorted(statuses.items())
|
||||
)
|
||||
lines.append(f"Gate roll-up: {roll}")
|
||||
if failures_total and not timeout:
|
||||
# Only emit the count outside the timeout path. The
|
||||
# timeout branch suppresses counts on purpose; this guard
|
||||
# is a belt-and-braces defence against a future caller
|
||||
# bypassing the orchestrator's zeroing.
|
||||
lines.append(
|
||||
f"Persistent failures: **{failures_total}** "
|
||||
f"(related-to-diff={len(related)}, unrelated={len(unrelated)})."
|
||||
)
|
||||
elif not timeout:
|
||||
lines.append(
|
||||
"No persistent failures across two pre-flight runs — you "
|
||||
"do NOT need to re-run `local_ci_gate.sh --fast` unless "
|
||||
"you change files."
|
||||
)
|
||||
lines.append(
|
||||
"Full classification (per-scenario paths + run metadata) is "
|
||||
"in the PR-context sentinel: "
|
||||
f"`python3 tools/implementer_pr_context.py read --pr {pr_number} "
|
||||
"--field gate_preflight`"
|
||||
)
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
def _render_compliance_pointer_stanza(
|
||||
gaps: dict[str, bool], pr_number: int,
|
||||
*,
|
||||
masked_checks: list[str] | None = None,
|
||||
) -> str:
|
||||
"""Brief compliance summary + sentinel-read hint.
|
||||
|
||||
Mirrors :func:`_implementer_compliance.render_prompt_stanza` but
|
||||
condensed: this is the SHORT version that survives tier-agent
|
||||
summarisation. The full per-gap hints are still rendered by the
|
||||
compliance module's own helper if the worker chooses to fetch
|
||||
them.
|
||||
|
||||
``masked_checks`` carries the list of check names where the
|
||||
underlying git call failed and the function fell back to the
|
||||
"treat as passing" policy. When non-empty the renderer
|
||||
explicitly downgrades the "all checks passed" verdict to "all
|
||||
OBSERVABLE checks passed; N were masked because git failed"
|
||||
and refuses to direct the worker to exit. Without this hedge
|
||||
the worker would receive a confident `resolved` directive on
|
||||
a worktree the dispatcher couldn't actually inspect — see
|
||||
plan v4 §"Compliance masking footgun".
|
||||
"""
|
||||
masked_checks = masked_checks or []
|
||||
all_closed = _implementer_compliance.all_gaps_closed(gaps)
|
||||
missing = [k for k, v in gaps.items() if not v]
|
||||
lines = ["## Compliance gap report (read sentinel for hints)"]
|
||||
lines.append("")
|
||||
if all_closed and not masked_checks:
|
||||
lines.append(
|
||||
"**All compliance checks passed.** The PR is complete — "
|
||||
"do NOT re-apply or re-edit the code fix. Verify quality "
|
||||
"gates and emit `{\"outcome\": \"resolved\", \"files_touched\": []}`."
|
||||
)
|
||||
elif all_closed and masked_checks:
|
||||
# Hedge: every check that COULD be evaluated passed, but
|
||||
# one or more underlying ``git`` calls failed so the
|
||||
# dispatcher could not actually inspect those signals.
|
||||
# Do NOT direct the worker to exit — the masked check
|
||||
# could be hiding a real gap.
|
||||
joined_masked = ", ".join(f"`{m}`" for m in masked_checks)
|
||||
lines.append(
|
||||
"**All OBSERVABLE compliance checks passed, but "
|
||||
f"{len(masked_checks)} check(s) were MASKED** because "
|
||||
f"`git` itself failed: {joined_masked}. Do NOT exit "
|
||||
"with `resolved` — re-verify the masked check(s) "
|
||||
"in-session (run `git status` / `git log -1`) before "
|
||||
"deciding whether the PR is actually complete."
|
||||
)
|
||||
else:
|
||||
joined = ", ".join(f"`{m}`" for m in missing)
|
||||
lines.append(f"Gaps to fill ({len(missing)}): {joined}.")
|
||||
lines.append(
|
||||
"Fill ONLY the missing items — the existing code fix in "
|
||||
"HEAD is correct and should not be re-touched unless a "
|
||||
"related-to-diff gate failure tells you otherwise."
|
||||
)
|
||||
if masked_checks:
|
||||
joined_masked = ", ".join(f"`{m}`" for m in masked_checks)
|
||||
lines.append(
|
||||
f"_Note: {len(masked_checks)} check(s) were masked "
|
||||
f"because git failed: {joined_masked}. The values "
|
||||
"shown for those keys may be stale — re-verify in-session._"
|
||||
)
|
||||
lines.append(
|
||||
"Per-gap hints + ground-truth dict are in the PR-context "
|
||||
f"sentinel: `python3 tools/implementer_pr_context.py read "
|
||||
f"--pr {pr_number} --field compliance_gaps`"
|
||||
)
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
def _collect_changed_files_from_result(result: Any) -> list[str]:
|
||||
"""Best-effort extraction of the PR's changed-file list from the
|
||||
prefetch result. Used by the gate pre-flight to classify gate
|
||||
@@ -791,6 +1124,7 @@ def _record_phase4_telemetry(
|
||||
tier_attempt_index: int | None = None,
|
||||
escalation_action: str | None = None,
|
||||
escalation_tier_hint: int | None = None,
|
||||
outcome_synthesised: bool | None = None,
|
||||
) -> dict[str, Any] | None:
|
||||
"""Extract a Phase 4 telemetry row + write it to the JSONL sink.
|
||||
|
||||
@@ -832,6 +1166,7 @@ def _record_phase4_telemetry(
|
||||
tier_attempt_index=tier_attempt_index,
|
||||
escalation_action=escalation_action,
|
||||
escalation_tier_hint=escalation_tier_hint,
|
||||
outcome_synthesised=outcome_synthesised,
|
||||
)
|
||||
except Exception as exc:
|
||||
_logger.warning(
|
||||
@@ -1357,14 +1692,24 @@ def _synthesize_outcome_if_missing(
|
||||
one wasted same-tier retry per cycle).
|
||||
|
||||
The ``_synthesized`` flag is captured in the Phase 4 telemetry
|
||||
row so an operator analysing the JSONL sink can distinguish
|
||||
cycles where the worker explicitly reported failure from cycles
|
||||
where the dispatcher synthesised a verdict.
|
||||
row (as ``outcome_synthesised``) so an operator analysing the
|
||||
JSONL sink can distinguish cycles where the worker explicitly
|
||||
reported failure from cycles where the dispatcher synthesised a
|
||||
verdict.
|
||||
|
||||
Gated by :func:`_is_outcome_synthesis_enabled`. When that returns
|
||||
False, the function passes through whatever ``parsed_json`` was
|
||||
(including ``None``) with ``was_synthesized=False``; the
|
||||
downstream escalation predicate will then see the legacy
|
||||
UNKNOWN bucket and behave byte-equivalent to the pre-feature
|
||||
build.
|
||||
"""
|
||||
if isinstance(parsed_json, dict):
|
||||
outcome = parsed_json.get("outcome")
|
||||
if isinstance(outcome, str) and outcome:
|
||||
return parsed_json, False
|
||||
if not _is_outcome_synthesis_enabled():
|
||||
return parsed_json, False
|
||||
synthesized_outcome = _SYNTHESIZED_OUTCOME_FROM_TERMINAL_STATE.get(
|
||||
terminal_state, "unknown"
|
||||
)
|
||||
@@ -1486,6 +1831,7 @@ def _post_session_action_with_escalation(
|
||||
wallclock: float | None,
|
||||
depth: int | None,
|
||||
action_str: str | None = None,
|
||||
outcome_synthesised: bool | None = None,
|
||||
) -> dict[str, Any]:
|
||||
"""Per-attempt side effects: telemetry write + per-tier
|
||||
status comment. Returns a dict for the attempts log."""
|
||||
@@ -1503,6 +1849,7 @@ def _post_session_action_with_escalation(
|
||||
tier_attempt_index=tier_idx,
|
||||
escalation_action=action_str,
|
||||
escalation_tier_hint=tier_idx,
|
||||
outcome_synthesised=outcome_synthesised,
|
||||
)
|
||||
status_comment = _maybe_post_status_comment(
|
||||
cfg, item,
|
||||
@@ -1523,7 +1870,7 @@ def _post_session_action_with_escalation(
|
||||
# turns the dispatcher's UNKNOWN-bucket waste into a clean
|
||||
# competence-class signal. Recorded on the per-attempt
|
||||
# telemetry row as ``outcome_synthesised=true``.
|
||||
parsed_json, _t0_outcome_synthesised = _synthesize_outcome_if_missing(
|
||||
parsed_json, t0_outcome_synthesised = _synthesize_outcome_if_missing(
|
||||
parsed_json, terminal_state,
|
||||
)
|
||||
|
||||
@@ -1572,6 +1919,7 @@ def _post_session_action_with_escalation(
|
||||
start_tier, parsed_json, raw_response, terminal_state,
|
||||
session_wallclock_seconds, subagent_max_depth,
|
||||
action_str=action.value,
|
||||
outcome_synthesised=t0_outcome_synthesised,
|
||||
))
|
||||
|
||||
EscAction = _implementer_escalation.EscalationAction
|
||||
@@ -1584,6 +1932,7 @@ def _post_session_action_with_escalation(
|
||||
last_terminal_state = terminal_state
|
||||
last_wallclock = session_wallclock_seconds
|
||||
last_depth = subagent_max_depth
|
||||
last_outcome_synthesised = t0_outcome_synthesised
|
||||
final_action = action
|
||||
|
||||
# The terminal actions (SUCCESS, END_CYCLE, EXHAUSTED) skip the
|
||||
@@ -1622,7 +1971,7 @@ def _post_session_action_with_escalation(
|
||||
)
|
||||
last_parsed = new_session.parsed_json
|
||||
last_terminal_state = _terminal_state_from_session(new_session)
|
||||
last_parsed, _ = _synthesize_outcome_if_missing(
|
||||
last_parsed, last_outcome_synthesised = _synthesize_outcome_if_missing(
|
||||
last_parsed, last_terminal_state,
|
||||
)
|
||||
last_raw = new_session.raw_response
|
||||
@@ -1674,7 +2023,7 @@ def _post_session_action_with_escalation(
|
||||
)
|
||||
last_parsed = new_session.parsed_json
|
||||
last_terminal_state = _terminal_state_from_session(new_session)
|
||||
last_parsed, _ = _synthesize_outcome_if_missing(
|
||||
last_parsed, last_outcome_synthesised = _synthesize_outcome_if_missing(
|
||||
last_parsed, last_terminal_state,
|
||||
)
|
||||
last_raw = new_session.raw_response
|
||||
@@ -1696,6 +2045,7 @@ def _post_session_action_with_escalation(
|
||||
current_tier, last_parsed, last_raw, last_terminal_state,
|
||||
last_wallclock, last_depth,
|
||||
action_str=action.value,
|
||||
outcome_synthesised=last_outcome_synthesised,
|
||||
))
|
||||
final_action = action
|
||||
|
||||
|
||||
@@ -47,6 +47,14 @@ Subcommand ``read``:
|
||||
- ``diff`` — the unified diff as plain text (between
|
||||
``BEGIN_PR_DIFF`` / ``END_PR_DIFF`` markers in the original
|
||||
prompt)
|
||||
- ``compliance_gaps`` — JSON dict of the dispatcher's deterministic
|
||||
PR Compliance Checklist scan (worktree_clean, changelog,
|
||||
contributors, issues-closed footer). Empty stdout when the
|
||||
dispatcher did not run the scan this cycle.
|
||||
- ``gate_preflight`` — JSON dict of the heavy flaky-test gate
|
||||
pre-flight (two ``local_ci_gate.sh --fast`` runs, classification
|
||||
of persistent vs. flaky failures). Empty stdout when the
|
||||
dispatcher did not run the pre-flight this cycle.
|
||||
- ``all`` — the entire sentinel payload as JSON (useful for
|
||||
debugging; ``task-implementor`` should pull narrowly)
|
||||
|
||||
@@ -344,6 +352,19 @@ def _project_field(payload: dict[str, Any], field: str) -> Any:
|
||||
if not payload.get("epic_completed", True):
|
||||
return _MISSING
|
||||
return payload.get("epic")
|
||||
if field in ("compliance_gaps", "gate_preflight"):
|
||||
# Deterministic-check sections — BINARY contract (not the
|
||||
# generic three-case prefetch contract). The dispatcher
|
||||
# either computed this cycle (``*_completed=True`` AND a
|
||||
# populated dict, emitted as JSON) or it did not (both keys
|
||||
# absent, emitted as empty stdout / worker falls through).
|
||||
# The compliance + preflight detectors always produce a
|
||||
# structured dict when they run, so there is no
|
||||
# "tried-but-empty" middle state to surface.
|
||||
completed_key = f"{field}_completed"
|
||||
if not payload.get(completed_key):
|
||||
return _MISSING
|
||||
return payload.get(field)
|
||||
raise ValueError(f"unknown --field: {field}")
|
||||
|
||||
|
||||
@@ -359,6 +380,8 @@ _ALLOWED_FIELDS = (
|
||||
"issues",
|
||||
"epic",
|
||||
"issue_body",
|
||||
"compliance_gaps",
|
||||
"gate_preflight",
|
||||
)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user