Skip to content

fix(sweep): replace the reply-reaction shortcut with the four-condition trigger gate - #2486

Merged
carlos-alm merged 4 commits into
mainfrom
fix/sweep-four-condition-gate
Aug 15, 2026
Merged

fix(sweep): replace the reply-reaction shortcut with the four-condition trigger gate#2486
carlos-alm merged 4 commits into
mainfrom
fix/sweep-four-condition-gate

Conversation

@carlos-alm

Copy link
Copy Markdown
Contributor

The bug

Step 2g decided "is Greptile satisfied?" by checking for a positive reaction on the most recent non-Greptile comment — that is, on one of your own replies. A 👍 on a reply is not a re-review of your code. Under that rule the re-trigger is skipped while the fix sits un-reviewed — which is how a sweep reports a PR as converged when Greptile has never seen the change.

The fix

Transplant the four-condition gate the fleet standardised on. Satisfied now requires all of:

  1. an @greptileai trigger comment exists — and really mentions Greptile. A literal inside a code span, fenced block, indented block or HTML comment notifies nobody and is not a trigger.
  2. Greptile reacted positively to that trigger (never to a reply).
  3. Greptile posted no issue-style or inline comment since.
  4. Greptile has reviewed the current head, established from its own Last reviewed commit marker — not a commit timestamp. Greptile re-reviews by editing its summary in place, so timestamps cannot observe a completed re-review; a timestamp proxy re-triggers commits Greptile has already approved. Timestamps survive only as the fallback when no marker parses.

Every fetch failure fails safe by posting rather than assuming satisfaction.

Fitting it to this repo

  • Step 2i (final mandatory re-trigger) added; Return result renumbered to 2j. The gate is designed to run twice, and its own header names Step 2i. That header doubles as the sentinel the shared test suite locates the gate by, so aligning this repo's numbering keeps one identical suite proving every fleet copy. I first tried the reverse — rewording the sentinel to say "Step 2h.1" to match the existing layout — and it immediately broke the suite's locator, which is the right outcome: that line is a fleet contract, not prose.
  • The 50-trigger hard cap is preserved, and Step 2g's prose now says explicitly that the gate is not exempt from it — if the cap is spent, follow the cap's instructions instead of running the gate.
  • The Step 2g prose was corrected. It still told the reader to "skip the actual trigger only if Greptile already reacted to your most recent reply" — the precise rule the gate exists to remove, and a direct contradiction of the code beneath it.

Verification

test_sweep_greptile_gate.py ported alongside. It extracts the real fenced block from SKILL.md — no copy — and runs it against fixture PR state behind a stub gh, under both bash and zsh (zsh because it is the maintainers' interactive shell, and a /sweep guard in a sibling repo once silently passed under zsh while failing its job, since zsh does not word-split an unquoted parameter).

Three of its six cases are mutation tests: they revert the gate to the pre-fix behaviour — remove the marker lookup, break the SHA regex, ignore a marker mismatch — and require the regression assertion to fail. Without them that assertion could pass with and against the bug it names.

6/6 on this branch. Wired into CI as sweep-gate.yml, path-filtered to the skill + suite — without it nothing ever executes this gate: it lives in Markdown, so it is shipped, load-bearing and otherwise entirely unproven.

Part of the fleet-wide sweep tracked by optave/data-retrieval-storage-svc#1021.

…on trigger gate

Step 2g decided "is Greptile satisfied?" by checking for a positive reaction on
THE MOST RECENT NON-GREPTILE COMMENT - i.e. on one of your own replies. A 👍 on a
reply is not a re-review of your code. Under that rule the re-trigger is skipped
while the fix sits un-reviewed, which is how a sweep reports a PR as converged
when Greptile has never seen the change.

Transplant the four-condition gate the fleet standardised on. Satisfied now
requires ALL of: an @greptileai TRIGGER comment exists and really mentions
Greptile (a literal inside a code span, fenced block, indented block or HTML
comment notifies nobody and is not a trigger); Greptile reacted positively TO THAT
TRIGGER; Greptile posted nothing since; and Greptile has reviewed the CURRENT head,
established from its own `Last reviewed commit` marker rather than a commit
timestamp - Greptile re-reviews by EDITING its summary IN PLACE, so timestamps
cannot observe a completed re-review. Every fetch failure fails safe by posting.

Add Step 2i (final mandatory re-trigger) and renumber Return result to 2j, matching
the fleet layout. The gate is meant to run twice and its own header names Step 2i;
that header doubles as the sentinel the shared test suite locates it by, so aligning
this repo's numbering keeps one identical suite proving every copy - the alternative,
rewording the sentinel per repo, would fork the contract.

The 50-trigger cap is preserved and now stated where it matters: the gate is not
exempt from it, and Step 2g's prose says so. That prose previously taught the exact
rule the gate replaces ("skip the actual trigger only if Greptile already reacted to
your most recent reply"), contradicting the code beneath it.

Verified with test_sweep_greptile_gate.py, ported alongside: it extracts the real
fenced block from SKILL.md and runs it behind a stub gh under both bash and zsh.
Three of its six cases are mutation tests that re-introduce the pre-fix behaviour
and require the regression assertion to fail. 6/6. Wired via sweep-gate.yml, since
a gate that lives in Markdown is otherwise never executed by anything.

Refs: optave/data-retrieval-storage-svc#1021, optave/data-retrieval-storage-svc#930
@greptile-apps

greptile-apps Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR replaces the reply-reaction shortcut with a four-condition gate that verifies Greptile reviewed the current head and adds fail-safe handling for API and trigger-post failures.

  • Adds a mandatory final gate execution before reporting sweep results.
  • Tests the actual Markdown-embedded shell gate under bash and zsh using fixture-backed GitHub CLI behavior.
  • Adds path-filtered CI coverage for the sweep skill and its regression suite.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
.claude/skills/sweep/SKILL.md Replaces reply-reaction satisfaction with the four-condition current-head gate and routes all trigger paths through a checked posting helper.
.github/scripts/test_sweep_greptile_gate.py Exercises the real embedded gate under bash and zsh, including marker handling, fetch failures, POST failures, and mutation regressions.
.github/workflows/sweep-gate.yml Adds path-filtered CI execution for the sweep-gate regression suite.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Run re-trigger gate] --> B{Real trigger exists?}
  B -- No --> P[Post @greptileai]
  B -- Yes --> C{Positive reaction on trigger?}
  C -- No --> P
  C -- Yes --> D{No later Greptile comments?}
  D -- No --> P
  D -- Yes --> E{Current head reviewed?}
  E -- Marker matches --> S[Skip re-trigger]
  E -- Marker differs --> P
  E -- No marker --> F{Timestamp fallback satisfied?}
  F -- Yes --> S
  F -- No --> P
  P --> G{POST succeeds?}
  G -- Yes --> H[Exit successfully]
  G -- No --> I[Fail loudly]
Loading

Reviews (3): Last reviewed commit: "fix(sweep): the fail-safe recovery post ..." | Re-trigger Greptile

Comment thread .claude/skills/sweep/SKILL.md Outdated
Comment on lines +382 to +384
trigger_candidates=$(gh api repos/<repo>/issues/<number>/comments --paginate \
--jq '.[] | select(.user.login != "greptile-apps[bot]") | "\(.id)\t\(.created_at)\t\((.body // "") | @json)"') \
|| { echo "FATAL: could not fetch trigger comments — aborting gate"; exit 1; }

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Trigger fetch aborts fail-safe gate

When the issue-comments request fails during Step 2g or mandatory Step 2i, this branch exits before attempting to post @greptileai, causing the sweep to stop without completing the required final review trigger.

Fix in Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — the trigger-comments fetch now fails safe by posting @greptileai and exiting 0 instead of aborting the whole gate with exit 1. This matches every other guard in this block (head-SHA fetch failure, after-trigger-comment fetch failure, etc.), so a transient API error during Step 2g or the mandatory Step 2i can no longer silently stop the sweep short of the required final trigger. Added a new scenario (trigger_candidates_fetch_fails) plus a mutation test proving the fix is load-bearing to test_sweep_greptile_gate.py.

Comment thread .claude/skills/sweep/SKILL.md Outdated
Comment on lines +562 to +565
reviewed_shas=$(gh api repos/<repo>/issues/<number>/comments --paginate \
--jq '.[] | select(.user.login == "greptile-apps[bot]") | .body' 2>/dev/null \
| grep -o 'Last reviewed commit:.*/commit/[0-9a-fA-F]\{40\}' \
| grep -o '[0-9a-fA-F]\{40\}$' | tr 'A-Z' 'a-z')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Marker fetch failure permits false skip

When this issue-comments request fails after the initial fetch succeeded, the pipeline silently produces an empty reviewed_shas and enters the timestamp fallback. If the commit timestamp is no later than the trigger, the gate treats the head as reviewed and skips the mandatory trigger even though Greptile's current-head marker was never retrieved.

Fix in Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — split the reviewed-commit marker fetch into its own gh api call whose exit status is checked directly with ||, before piping into grep. Previously the fetch was piped straight into grep -o, so a failed API call was indistinguishable from a successful call that legitimately found no marker yet (both left reviewed_shas empty and landed in the timestamp-proxy fallback) — meaning a fetch failure could get silently treated as 'satisfied' whenever the commit timestamp happened to predate the trigger. Now a fetch failure posts @greptileai immediately instead of falling through to the proxy. Added a new scenario (reviewed_shas_fetch_fails_must_not_fall_through) plus a mutation test proving the fix is load-bearing to test_sweep_greptile_gate.py.

@carlos-alm

Copy link
Copy Markdown
Contributor Author

Addressed Greptile's review feedback (Confidence Score 3/5):

  • Trigger-comments fetch (Step 2g gate): now fails safe by posting @greptileai and exiting 0 instead of aborting the whole gate with exit 1 on a failed fetch — matches every other guard's fail-safe pattern in this block, so a transient API error can no longer silently stop the sweep short of the required Step 2i final trigger.
  • Reviewed-commit marker fetch (condition 4): split into its own gh api call whose exit status is checked directly, before piping into grep — a failed fetch used to be indistinguishable from a legitimate "no marker yet" and could fall through to the timestamp-proxy fallback, which could wrongly read "satisfied" and skip the trigger.
  • Added two new scenarios (trigger_candidates_fetch_fails, reviewed_shas_fetch_fails_must_not_fall_through) plus two mutation tests to test_sweep_greptile_gate.py proving both fixes are load-bearing (now 8 checks total, all passing under both bash and zsh). Updated the module docstring and the sweep-gate.yml header comment that referenced "three mutation tests" to reflect the new total.

…etch failures (#2486)

The trigger-comments fetch aborted the whole gate with exit 1 instead of
posting @greptileai, so a transient API error during Step 2g or the
mandatory Step 2i could stop the sweep short of the required final
trigger. The reviewed-commit marker fetch piped gh api straight into
grep, so a failed fetch was indistinguishable from a legitimate 'no
marker yet' and could fall through to the timestamp-proxy fallback,
silently permitting a skip of a head Greptile never actually reviewed.

Both now check their own fetch's exit status directly and fail safe by
posting rather than aborting or falling through. Adds two scenarios and
two mutation tests to test_sweep_greptile_gate.py proving both fixes are
load-bearing, and updates the doc comments that referenced the old
mutation-test count.
@carlos-alm

Copy link
Copy Markdown
Contributor Author

@greptileai

Comment thread .claude/skills/sweep/SKILL.md Outdated
Comment on lines +389 to +390
echo "Could not fetch trigger comments — posting @greptileai (fail safe)."
gh api repos/<repo>/issues/<number>/comments -f body="@greptileai"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Recovery POST failure is ignored

When the trigger-comment fetch fails and the subsequent @greptileai POST also fails, this branch unconditionally exits successfully, causing the sweep to proceed without sending the mandatory final review trigger. The marker-fetch recovery branch has the same behavior.

Fix in Claude Code

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — and generalized beyond the two branches named here. Introduced a shared post_trigger_or_die helper that every posting branch in the gate now routes through (all 9: the two you flagged, the five other pre-existing fail-safe branches with the identical gap — trigger_ts empty, after-trigger-comment fetch failure, non-numeric page count, head_sha fetch failure, head_ts fetch failure — plus the 'no trigger exists' branch and the ordinary 'not satisfied' verdict, which had the same unchecked-POST issue). The helper exits 0 only if the @greptileai POST actually succeeds; otherwise it prints a loud FATAL message to stderr and exits 1, so a double failure (original fetch + recovery post both failing) can no longer silently report success. Added a scenario (trigger_fetch+trigger_post both failing) plus a mutation test proving this is load-bearing to test_sweep_greptile_gate.py (now 10 checks total, all passing under bash and zsh).

… own failure (#2486)

Every branch of the Greptile trigger gate that needs to post @greptileai
did so inline and then unconditionally exited 0 (or fell through to an
implicit success), without checking whether that POST itself succeeded.
A double failure — the original fetch AND the recovery post both
failing — would still report success, reproducing the exact bug this
PR fixes at one remove: the sweep believing the mandatory trigger
reached Greptile when it never left the machine.

Introduces a shared post_trigger_or_die helper used by all nine posting
branches (including the ordinary 'not satisfied' verdict): it exits 0
only if the post actually succeeds, and exits 1 with a loud message
otherwise. Adds a scenario and a mutation test proving the double-
failure case now fails loudly instead of silently.
@carlos-alm

Copy link
Copy Markdown
Contributor Author

Addressed the round-2 finding (Confidence Score 4/5 — "Recovery POST failure is ignored"):

  • Introduced a shared post_trigger_or_die helper and routed all nine posting branches in the gate through it (the two you flagged, five other pre-existing fail-safe branches with the identical gap, the "no trigger exists" branch, and the ordinary "not satisfied" verdict). It exits 0 only if the @greptileai POST itself succeeds, otherwise it prints a loud FATAL message and exits 1 — a double failure (original fetch + recovery post both failing) can no longer silently report success.
  • Added a scenario and a mutation test proving this is load-bearing to test_sweep_greptile_gate.py (now 10 checks total, all passing under bash and zsh).

@carlos-alm

Copy link
Copy Markdown
Contributor Author

@greptileai

@carlos-alm
carlos-alm merged commit ef206ff into main Aug 15, 2026
13 checks passed
@carlos-alm
carlos-alm deleted the fix/sweep-four-condition-gate branch August 15, 2026 02:21
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant