Skip to content

fix(#5536): use is_bot for bot-author detection in fix eligibility - #6250

Open
ggallen wants to merge 1 commit into
fullsend-ai:mainfrom
ggallen:worktree-issue-5536
Open

fix(#5536): use is_bot for bot-author detection in fix eligibility#6250
ggallen wants to merge 1 commit into
fullsend-ai:mainfrom
ggallen:worktree-issue-5536

Conversation

@ggallen

@ggallen ggallen commented Aug 15, 2026

Copy link
Copy Markdown
Member

Summary

  • Fix agent's eligibility check used gh pr view --json author (GraphQL), which returns app/fullsend-ai-coder — not the REST fullsend-ai-coder[bot] format. The [bot]$ regex never matched, so bot-authored PRs were misidentified as human-authored and skipped.
  • Switch to checking .author.is_bot (bool) and .author.login to precisely identify coder-bot PRs. This uses the fields actually exposed by gh pr view --json, following the same identity-check logic as the existing-PR guard (see Code dispatch existing-PR guard uses bare substring search, causing false-positive blocks #5974 and docs/contributing/bot-identities.md) adapted for the gh pr view API surface.
  • Extract eligibility logic into shared script (.github/scripts/check-fix-eligibility.sh) with 8 fixture tests, called by both reusable-fix.yml and reusable-dispatch.yml.
  • The third location named in Fix agent eligibility check misidentifies bot-authored PRs as human-authored #5536 (org-level scaffold dispatch.yml) does not have a fix eligibility check — it only has the existing-PR guard which already uses __typename correctly via raw GraphQL. No change needed there.
  • Scope: this fix covers the default hosted app-set (fullsend-ai-coder). Self-hosted --app-set deployments with custom app names remain in the same state as before (requiring the fullsend-fix label) — not a regression since the old [bot]$ regex never matched any bot.

Closes #5536

Test plan

  • Verify gh pr view <bot-pr> --json author --jq '{is_bot: .author.is_bot, login: .author.login}' returns {"is_bot":true,"login":"app/fullsend-ai-coder"} for a coder-bot-authored PR (verified via PR fix(#6246)!: preserve SHA pinning only for already-pinned repos during upgrade #6252)
  • Verify the same query returns {"is_bot":false,...} for a human-authored PR
  • Confirm fix agent auto-executes on a bot-authored PR after review posts CHANGES_REQUESTED (no manual /fs-fix needed)
  • Confirm fix agent still requires fullsend-fix label for human-authored PRs when bot-triggered

🤖 Generated with Claude Code

@ggallen
ggallen requested a review from a team as a code owner August 15, 2026 13:46
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:48 PM UTC · Completed 2:00 PM UTC

Commit: 83a98f2 · View workflow run →

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Use GraphQL __typename to detect bot-authored PRs in fix eligibility

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Fix bot-authored PR detection by checking GraphQL author.__typename instead of login suffix.
• Ensure bot-triggered fix runs without requiring fullsend-fix label on bot-authored PRs.
• Apply the same eligibility logic to both reusable-dispatch and reusable-fix workflows.
Diagram

graph TD
  A["Workflow: reusable-dispatch"] --> B["Workflow: reusable-fix"] --> C["gh pr view (GraphQL)"] --> D["PR_INFO: labels + authorType"] --> E{"authorType == Bot?"} --> F["Skip fullsend-fix label gate"]
  E --> G["Require fullsend-fix label"]
  C --> H["GitHub API"]
  subgraph Legend
    direction LR
    _wf[Workflow job] ~~~ _dec{"Decision"} ~~~ _ext[[External API]]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Use REST PR endpoint for author.type
  • ➕ REST user objects explicitly provide bot/user type in a stable field
  • ➕ Avoids GraphQL/CLI representation differences entirely
  • ➖ Requires switching from gh pr view JSON query to gh api (or equivalent) and updating parsing
  • ➖ Potentially more code churn across workflows
2. Standardize on trigger source only (no PR refetch)
  • ➕ Avoids extra API call during workflow execution
  • ➕ Simpler logic and fewer failure points
  • ➖ Does not answer the actual question (PR author type) when trigger and author differ
  • ➖ Would keep the fullsend-fix gating behavior incorrect for bot-authored PRs

Recommendation: Prefer the PR’s approach: using author.__typename == &quot;Bot&quot; is a minimal, robust fix that aligns with existing workflow conventions and avoids brittle string parsing of logins that can differ between REST and GraphQL representations.

Files changed (2) +12 / -6

Bug fix (2) +12 / -6
reusable-dispatch.ymlSwitch fix eligibility from login regex to author.__typename +6/-3

Switch fix eligibility from login regex to author.__typename

• Updates PR info fetching to capture 'author.__typename' and uses it to determine bot authorship. This fixes bot-authored PRs being treated as human-authored due to GraphQL login formatting differences.

.github/workflows/reusable-dispatch.yml

reusable-fix.ymlAlign bot detection with GraphQL author.__typename in reusable-fix +6/-3

Align bot detection with GraphQL author.__typename in reusable-fix

• Mirrors the dispatch workflow fix by storing 'authorType' from GraphQL and gating the fullsend-fix label requirement only for non-bot PR authors. Adds comments clarifying why login suffix matching is unreliable.

.github/workflows/reusable-fix.yml

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can turn on the rule miner and Qodo learns your standards from review history

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@codecov

codecov Bot commented Aug 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 15, 2026

Copy link
Copy Markdown

Review

Findings

Medium

  • [protected-path] .github/scripts/check-fix-eligibility.sh, .github/scripts/check-fix-eligibility-test.sh, .github/workflows/reusable-dispatch.yml, .github/workflows/reusable-fix.yml — This PR adds and modifies files under the protected .github/ path. The change is authorized by issue Fix agent eligibility check misidentifies bot-authored PRs as human-authored #5536 and the rationale is clearly explained in the PR description. Human approval is always required for protected-path changes, regardless of context.
Previous run

Review

Findings

Medium

  • [protected-path] .github/scripts/check-fix-eligibility.sh, .github/scripts/check-fix-eligibility-test.sh, .github/workflows/reusable-dispatch.yml, .github/workflows/reusable-fix.yml — This PR adds and modifies files under the protected .github/ path. The change is authorized by issue Fix agent eligibility check misidentifies bot-authored PRs as human-authored #5536 and the rationale is clearly explained in the PR description. Human approval is always required for protected-path changes, regardless of context.

Low

  • [test-coverage-gap] .github/scripts/check-fix-eligibility-test.sh — No test covers the case where a PR has both fullsend-fix and fullsend-no-fix labels simultaneously. The docs state fullsend-no-fix takes priority, and the code achieves this by checking no-fix first, but there is no test verifying this priority ordering. If someone reorders the checks in the script, this invariant would silently break.
Previous run (2)

Review

Findings

Medium

  • [protected-path] .github/scripts/check-fix-eligibility.sh, .github/scripts/check-fix-eligibility-test.sh, .github/workflows/reusable-dispatch.yml, .github/workflows/reusable-fix.yml — This PR adds and modifies files under the protected .github/ path. The change is authorized by issue Fix agent eligibility check misidentifies bot-authored PRs as human-authored #5536 and the rationale is clearly explained in the PR description. Human approval is always required for protected-path changes, regardless of context.

Low

  • [test-coverage-gap] .github/scripts/check-fix-eligibility-test.sh — The test suite covers 9 scenarios but does not include a case where is_bot is false while login matches app/fullsend-ai-coder. Adding tests with is_bot=false, login=app/fullsend-ai-coder, no label (expected exit 1) and with label (expected exit 0) would confirm that the login alone does not bypass the label requirement when is_bot is not true.
Previous run (3)

Review

Findings

Medium

  • [protected-path] .github/scripts/check-fix-eligibility.sh, .github/scripts/check-fix-eligibility-test.sh, .github/workflows/reusable-dispatch.yml, .github/workflows/reusable-fix.yml — This PR adds and modifies files under the protected .github/ path. The change is authorized by issue Fix agent eligibility check misidentifies bot-authored PRs as human-authored #5536 and the rationale is clearly explained in the PR description. Human approval is always required for protected-path changes, regardless of context.

Low

  • [naming-clarity] .github/scripts/check-fix-eligibility.sh:37 — The condition if [[ "${PR_IS_BOT}" != "true" || "${PR_LOGIN}" != "app/fullsend-ai-coder" ]] uses De Morgan's law (NOT A || NOT BNOT (A && B)) to express "if the PR author is not the fullsend-ai-coder bot." A brief comment before the condition would help readers unfamiliar with the pattern.
Previous run (4)

Review

Findings

Medium

  • [protected-path] .github/scripts/check-fix-eligibility.sh, .github/scripts/check-fix-eligibility-test.sh, .github/workflows/reusable-dispatch.yml, .github/workflows/reusable-fix.yml — This PR adds and modifies files under the protected .github/ path. The change is authorized by issue Fix agent eligibility check misidentifies bot-authored PRs as human-authored #5536 and the rationale is clearly explained in the PR description. Human approval is always required for protected-path changes, regardless of context.

Low

  • [test-inadequate] .github/scripts/check-fix-eligibility-test.sh:98 — The test for "null is_bot without label skipped" verifies that a null is_bot value fails closed (exit 1), but there is no complementary test for null is_bot with the fullsend-fix label (exit 0). While the label-override logic is covered by other tests (is_bot=false follows the same code path), explicit coverage of the null-is_bot fallback would prevent regressions in the old-gh-CLI compatibility path.
Previous run (5)

Review

Findings

Medium

  • [protected-path] .github/scripts/check-fix-eligibility.sh, .github/scripts/check-fix-eligibility-test.sh, .github/workflows/reusable-dispatch.yml, .github/workflows/reusable-fix.yml — This PR adds and modifies files under the protected .github/ path. The change is authorized by issue Fix agent eligibility check misidentifies bot-authored PRs as human-authored #5536 and the rationale is clearly explained in the PR description. Human approval is always required for protected-path changes, regardless of context.
Previous run (6)

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/workflows/reusable-fix.yml — This PR modifies files under the protected .github/ path. The change is authorized by issue Fix agent eligibility check misidentifies bot-authored PRs as human-authored #5536 and the rationale is clearly explained in the PR description. Human approval is always required for protected-path changes, regardless of context.
Previous run (7)

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/workflows/reusable-fix.yml — This PR modifies files under the protected .github/ path. The change is authorized by issue Fix agent eligibility check misidentifies bot-authored PRs as human-authored #5536 and the rationale is clearly explained in the PR description. Human approval is always required for protected-path changes, regardless of context.

Low

  • [naming-consistency] .github/workflows/reusable-dispatch.yml:1085, .github/workflows/reusable-fix.yml:307 — The jq intermediate key isBot uses camelCase, while the upstream gh pr view --json API field is is_bot (snake_case) and the downstream shell variable is PR_IS_BOT (SCREAMING_SNAKE). Using is_bot as the jq key would keep the naming chain consistent across API field, jq key, and shell variable.
Previous run (8)

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/workflows/reusable-fix.yml — This PR modifies files under the protected .github/ path. The change is authorized by issue Fix agent eligibility check misidentifies bot-authored PRs as human-authored #5536 and the rationale is clearly explained in the PR description. Human approval is always required for protected-path changes, regardless of context.

  • [intent-mismatch] .github/workflows/reusable-dispatch.yml:1080 — PR title and body claim the fix switches to __typename == Bot detection (matching the existing-PR guard and docs/contributing/bot-identities.md), but the actual implementation uses .author.is_bot and .author.login from gh pr view --json — a different API surface and detection mechanism. The title says "use __typename for bot-author detection" while the code uses is_bot. Since PR titles feed into release notes, this mismatch could confuse future readers. Update the PR title and body to accurately describe the is_bot/login approach. Refactoring to use raw GraphQL __typename would be disproportionate since the fix eligibility step already uses gh pr view for label fetching, and gh pr view --json does not expose __typename.

Low

  • [internal consistency] .github/workflows/reusable-dispatch.yml:1097 — The prior finding about inconsistency between the GraphQL __typename check (existing-PR guard, ~line 357) and the gh pr view --json is_bot/app-prefixed-login check (fix eligibility, ~line 1097) has been mitigated. The PR now adds cross-reference comments at both locations explaining the different API surfaces and confirming the same identity logic. Residual risk is low: a future maintainer updating one guard without the other, but the comments reduce that likelihood.

  • [architectural-coherence] .github/workflows/reusable-dispatch.yml:1080 — The project documents a single canonical pattern for bot-identity detection: match on __typename == "Bot" plus the un-suffixed login (see bot-identities.md). This fix introduces a second pattern — is_bot boolean plus app/fullsend-ai-coder login — for the same logical check in the same file. However, this is a necessary consequence of using two different GitHub API surfaces: raw GraphQL (which exposes __typename) and gh pr view --json (which exposes is_bot but not __typename). Consider updating bot-identities.md to document is_bot as the equivalent pattern when using gh pr view --json.

  • [scope-coverage] .github/workflows/reusable-fix.yml:294 — Issue Fix agent eligibility check misidentifies bot-authored PRs as human-authored #5536 lists three affected locations. The third — the org-level scaffold dispatch.yml — is not modified in this PR. This is correct: that file uses PR_USER_LOGIN from the webhook payload (github.event.pull_request.user.login), which includes the [bot] suffix, so the existing regex match works correctly. Consider adding a note to the PR body explaining this exclusion.

Previous run (9)

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/workflows/reusable-fix.yml — This PR modifies files under the protected .github/ path. The change is authorized by issue Fix agent eligibility check misidentifies bot-authored PRs as human-authored #5536 and the rationale is clearly explained in the PR description. Human approval is always required for protected-path changes, regardless of context.

Low

  • [internal consistency] .github/workflows/reusable-dispatch.yml:1091 — The existing bot-author guard earlier in the same file (~line 339) uses GraphQL __typename == "Bot" and bare author.login (no app/ prefix), while this new code uses gh pr view --json fields is_bot (bool) and login with the app/ prefix format. The two guards serve different query needs (GraphQL batch query vs. single-PR REST lookup), but the inconsistency could confuse future maintainers updating bot identity logic. Consider adding cross-reference comments between the two guards.
Previous run (10)

Review

Findings

Medium

  • [protected-path] .github/workflows/reusable-dispatch.yml, .github/workflows/reusable-fix.yml — This PR modifies files under the protected .github/ path. The change is authorized by issue Fix agent eligibility check misidentifies bot-authored PRs as human-authored #5536 and the rationale is clearly explained in the PR description. Human approval is always required for protected-path changes, regardless of context.

Labels: PR modifies fix-agent eligibility logic in dispatch workflows

@fullsend-ai-review fullsend-ai-review Bot added requires-manual-review Review requires human judgment component/dispatch Workflow dispatch and triggers agent/fix Fix agent labels Aug 15, 2026

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

HIGH: Third location named in issue #5536 (org-level dispatch.yml) is left unfixed despite "Closes #5536"

Issue #5536's "Affected locations" list names three files with the same bug: reusable-fix.yml and reusable-dispatch.yml (both addressed by this PR) plus .fullsend/.github/workflows/dispatch.yml at the org level (in the fullsend-ai/.fullsend repo, out of tree for this PR — no inline location available here). That file still hardcodes BOT_LOGIN="fullsend-ai[bot]" / CODER_BOT_LOGIN="fullsend-ai-coder[bot]" (around lines 271-272) and compares them against .author.login from gh pr list --json number,author (around lines 275-276). gh pr list --json author also returns app/fullsend-ai-coder, not fullsend-ai-coder[bot] — the identical root-cause mismatch — so this comparison also never matches a real bot PR.

This PR's body says "Closes #5536", which will auto-close the issue on merge even though one of its three named locations remains broken (and, per the inline CRITICAL finding, the other two aren't actually fixed either).

Failure scenario: Merging this PR auto-closes #5536. The org-level dispatch routing in .fullsend/.github/workflows/dispatch.yml continues to misidentify bot-authored PRs as human because BOT_LOGIN/CODER_BOT_LOGIN are compared against a login format ([bot] suffix) that gh pr list --json author never returns, so the same class of bug silently persists there with no tracking issue left open.

Suggestion: Either fix .fullsend/.github/workflows/dispatch.yml in this PR (or a linked companion PR) before closing #5536, or scope this PR's title/body to only the two files it actually touches and open a follow-up issue for the org-level file so it isn't lost when #5536 auto-closes.


Two additional findings were left as inline comments on .github/workflows/reusable-dispatch.yml (a CRITICAL issue with the __typename field not being populated by gh pr view, and a MEDIUM precision gap relative to the existing-PR guard it's modeled on).

Comment thread .github/workflows/reusable-dispatch.yml Outdated
Comment thread .github/workflows/reusable-dispatch.yml Outdated
@ggallen

ggallen commented Aug 15, 2026

Copy link
Copy Markdown
Member Author

Re: HIGH (org-level dispatch.yml) — the org-level dispatch is generated from the scaffold template (internal/scaffold/fullsend-repo/.github/workflows/dispatch.yml), which does not have a fix eligibility check. The fix eligibility logic only exists in reusable-fix.yml and reusable-dispatch.yml, both fixed in this PR. No follow-up needed.

@ggallen
ggallen force-pushed the worktree-issue-5536 branch from 83a98f2 to a37adcc Compare August 15, 2026 15:20
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:21 PM UTC · Completed 3:36 PM UTC

Commit: a37adcc · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@ggallen
ggallen requested a review from waynesun09 August 15, 2026 15:37
@ggallen
ggallen force-pushed the worktree-issue-5536 branch from a37adcc to e3553d3 Compare August 15, 2026 15:41
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:42 PM UTC · Completed 3:55 PM UTC

Commit: e3553d3 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@ggallen ggallen changed the title fix(#5536): use __typename for bot-author detection in fix eligibility fix(#5536): use is_bot for bot-author detection in fix eligibility Aug 15, 2026
@ggallen
ggallen force-pushed the worktree-issue-5536 branch from e3553d3 to 5e3e64f Compare August 15, 2026 15:58
@github-actions

github-actions Bot commented Aug 15, 2026

Copy link
Copy Markdown

Site preview

Preview: https://1e23914e-site.fullsend-ai.workers.dev

Commit: 066c6b11ff9242f45e2941d04316548b4ddaef04

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:00 PM UTC · Completed 4:13 PM UTC

Commit: 5e3e64f · View workflow run →

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One additional finding that has no inline location in this PR's diff (the file isn't touched here):

MEDIUM: docs/agents/fix.md still describes auto-fix as applying to any bot-authored PR, not just the coder app (docs/agents/fix.md, line 122)

The new gate in both reusable-dispatch.yml:1097 and reusable-fix.yml:309 requires PR_IS_BOT == "true" AND PR_LOGIN == "app/fullsend-ai-coder" — only the coder app qualifies for label-free auto-fix. But docs/agents/fix.md (unchanged by this PR) still reads (lines 121-124): "For bot-authored PRs (e.g., PRs opened by the code agent), automatic fixing happens with no extra setup" — phrased as any bot-authored PR being eligible, not scoped to the coder app specifically. Since dispatch routing (unchanged by this PR) still admits any [bot]$-suffixed webhook login into the fix stage, a PR from a different bot (e.g. renovate-fullsend[bot], listed in docs/contributing/bot-identities.md) reaches this gate and is silently skipped with the misleading "Human-authored PR #N ... skipping bot-triggered fix" warning (reusable-fix.yml:312) — an operator reading fix.md would not expect that skip.

Suggestion: Update docs/agents/fix.md to scope the wording to the coder app specifically (e.g. "For PRs authored by the fullsend code agent") now that this PR makes the eligibility check precise, since this is the PR that changes/clarifies the actual gating behavior.

Comment thread .github/workflows/reusable-dispatch.yml Outdated
fullsend-ai-review[bot]

This comment was marked as outdated.

@ggallen

ggallen commented Aug 15, 2026

Copy link
Copy Markdown
Member Author

Re: MEDIUM (fix.md wording) — updated docs/agents/fix.md to scope auto-fix eligibility to "PRs authored by the fullsend code agent (fullsend-ai-coder[bot])" and explicitly note that other bots require the fullsend-fix label.

@ggallen
ggallen force-pushed the worktree-issue-5536 branch from 5e3e64f to 4ec2d3a Compare August 15, 2026 16:21
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:22 PM UTC · Completed 4:37 PM UTC

Commit: 4ec2d3a · View workflow run →

@ggallen
ggallen requested a review from waynesun09 August 15, 2026 17:05

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Two additional findings that don't have a commentable inline diff line in this PR (the surrounding lines aren't part of the diff hunk / no file changed):

MEDIUM: Control-labels table still says "Bot-authored PRs do not need this label," contradicting the updated prose above it (docs/agents/fix.md, line 141 — outside this PR's diff hunk for that file)

The prose earlier in the same file (~lines 119-124, updated by this PR) now says only PRs from fullsend-ai-coder[bot] auto-fix without the label, and "PRs from other bots (e.g., Renovate) are treated like human-authored PRs and require the fullsend-fix label." But the separate "Control labels" table (## Control labels, line 137) still has the row: fullsend-fix | ... Bot-authored PRs do not need this label. — a blanket statement no longer true after this change. This table wasn't touched by this PR's diff (unlike the prose paragraph), so the two sections of the same doc now directly contradict each other.

Suggestion: Update the table row to match the prose, e.g. "Enables automatic bot-triggered fix runs on human-authored PRs, and on bot-authored PRs other than the fullsend coder bot, which is always eligible without this label."


MEDIUM: Test-plan checkboxes for this CI-gating change remain unchecked (PR description, "Test plan" section)

Confirmed in the current PR body: all four test-plan checkboxes are still unchecked (verifying gh pr view --json author output for a bot PR, for a human PR, confirming auto-fix fires on a bot PR after CHANGES_REQUESTED, and confirming the label gate still applies to human PRs). This changes production CI eligibility gating for an automated write path (the fix agent auto-committing to PRs); it's already been live-verified via review comments that the is_bot/app/-prefix schema assumption holds (PR #6252 -> is_bot:true, login:app/fullsend-ai-coder), but the PR's own listed end-to-end verification steps were never marked done.

Suggestion: Check off the test-plan boxes after actually running the listed gh pr view commands against a live bot PR and a live human PR, and confirm the auto-fix trigger fires end-to-end before merging.

Comment thread .github/workflows/reusable-dispatch.yml Outdated
Comment thread .github/workflows/reusable-dispatch.yml Outdated
Comment thread .github/workflows/reusable-dispatch.yml Outdated
@ggallen

ggallen commented Aug 15, 2026

Copy link
Copy Markdown
Member Author

Responding to waynesun09's review on commit 4ec2d3a:

Control-labels table — Already fixed in a prior commit. The table row now says: "Enables automatic bot-triggered fix runs on human-authored PRs and PRs from bots other than the fullsend code agent. Without this label, the fix agent only runs when explicitly invoked via /fs-fix. PRs authored by fullsend-ai-coder[bot] are always eligible without this label."

Test-plan checkboxes — Acknowledged. Will check off items 1-2 (the gh pr view verification) since this has been live-verified in the review discussion (PR #6252 confirmed is_bot:true, login:app/fullsend-ai-coder). Items 3-4 (end-to-end auto-fix trigger) need the code deployed to verify.

Skip-log message — Fixed in shared script. Log now includes PR_LOGIN and PR_IS_BOT.

No test / duplication — Fixed by extracting into .github/scripts/check-fix-eligibility.sh with 8 fixture tests.

Hardcoded literal — Acknowledged, intentionally scoped to hosted app-set. Not a regression (old regex never matched any bot). Added scope note to PR description.

@ggallen
ggallen force-pushed the worktree-issue-5536 branch from 4ec2d3a to 6972772 Compare August 15, 2026 19:47
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 15, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:48 PM UTC · Completed 8:27 PM UTC

Commit: 6972772 · View workflow run →

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One additional finding that has no inline location in this PR's diff (Makefile is not touched by this PR):

HIGH: New check-fix-eligibility-test.sh fixture suite is not wired into the Makefile's script-test target, so CI never runs it (Makefile, script-test target, ~line 180)

Verified on HEAD 6972772: Makefile's script-test target explicitly lists each shell-script test file (scripts/check-e2e-authorization-test.sh, .github/scripts/redact-behaviour-artifacts-test.sh, internal/scaffold/fullsend-repo/scripts/reconcile-repos-test.sh, internal/scaffold/fullsend-repo/scripts/pre-fetch-prior-review-test.sh, plus two python test files) and was not updated to add .github/scripts/check-fix-eligibility-test.sh. .github/workflows/lint.yml runs make script-test in CI. The new 8-fixture test suite added by this PR therefore never executes in CI — it is dead code from CI's perspective, so a future regression in the eligibility predicate (including the deployment gap raised inline on reusable-fix.yml) would not be caught automatically.

Suggestion: Add bash .github/scripts/check-fix-eligibility-test.sh to the script-test target in the Makefile, and confirm a CI run actually exercises it before merge.

Comment thread .github/workflows/reusable-fix.yml Outdated
Comment thread .github/workflows/reusable-fix.yml Outdated
@ggallen
ggallen force-pushed the worktree-issue-5536 branch from 6972772 to 046675b Compare August 16, 2026 02:10
@ggallen
ggallen requested a review from waynesun09 August 16, 2026 02:10
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 16, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 2:11 AM UTC · Ended 2:17 AM UTC

Commit: 046675b · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 16, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:18 AM UTC · Completed 2:36 AM UTC

Commit: 02ca85c · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@ggallen
ggallen force-pushed the worktree-issue-5536 branch from 02ca85c to dcb3ce2 Compare August 16, 2026 02:38
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 16, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:40 AM UTC · Completed 2:57 AM UTC

Commit: dcb3ce2 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 16, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:00 AM UTC · Completed 3:17 AM UTC

Commit: f82a8b6 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 16, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:21 AM UTC · Completed 3:38 AM UTC

Commit: a588425 · View workflow run →

…gibility

The fix agent's eligibility check used gh pr view --json author which
returns app/fullsend-ai-coder format, but the code tested against a
[bot]$ regex that never matched. Switch to .author.is_bot (bool) plus
.author.login to precisely identify coder-bot PRs, following the same
identity-check pattern as fullsend-ai#5974 adapted for the gh pr view API surface.

Extract the eligibility logic into a shared script
(.github/scripts/check-fix-eligibility.sh) called by both
reusable-fix.yml and reusable-dispatch.yml, with 12 fixture tests.
Source the script from the trusted .defaults/ tree (upstream scaffold
checkout) to ensure it exists for consumer repos and runs with the
correct trust boundary.

Signed-off-by: Greg Allen <gallen@redhat.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
@ggallen
ggallen force-pushed the worktree-issue-5536 branch from a588425 to 066c6b1 Compare August 16, 2026 03:41
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 16, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:43 AM UTC · Completed 3:58 AM UTC

Commit: 066c6b1 · View workflow run →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent/fix Fix agent component/dispatch Workflow dispatch and triggers requires-manual-review Review requires human judgment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix agent eligibility check misidentifies bot-authored PRs as human-authored

2 participants