Skip to content

Fix bot author allowlists#3162

Merged
wwwillchen merged 4 commits intomainfrom
fix-bot-allowlist
Apr 7, 2026
Merged

Fix bot author allowlists#3162
wwwillchen merged 4 commits intomainfrom
fix-bot-allowlist

Conversation

@keppo-bot
Copy link
Copy Markdown
Contributor

@keppo-bot keppo-bot bot commented Apr 7, 2026

Summary

  • accept keppo-bot[bot] in GitHub Actions author checks that read raw github.* logins
  • switch Claude PR Review to allowed_bots: \"keppo-bot[bot]\" instead of treating the bot as a non-write user
  • trust copilot-pull-request-reviewer in the PR comment-fixing skill

Test plan

  • npm run fmt
  • npm run lint:fix
  • npm run ts
  • npm test

πŸ€– Generated with Claude Code


Open with Devin

claude and others added 2 commits April 7, 2026 13:33
…GitHub workflow author checks\n- use allowed_bots for claude-code-action bot access\n- trust copilot-pull-request-reviewer in pr-fix-comments\n\nCo-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@keppo-bot keppo-bot bot requested a review from a team April 7, 2026 22:15
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

πŸ’‘ Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4f823d1089

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with πŸ‘.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

echo "is_privileged=false" >> $GITHUB_OUTPUT
fi
case "$AUTHOR" in
wwwillchen|keppo-bot|keppo-bot[bot]|app/keppo-bot|dyad-assistant|app/dyad-assistant|azizmejri1)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Escape bot login in case pattern

In Bash case, [ and ] are glob metacharacters, so keppo-bot[bot] is treated as a character class (matching keppo-botb/o/t) rather than the literal login keppo-bot[bot]. As a result, PRs authored by keppo-bot[bot] will still fall through to is_privileged=false, so this change does not actually grant that bot privileged CI routing.

Useful? React with πŸ‘Β / πŸ‘Ž.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 7 files

Confidence score: 3/5

  • There is a concrete high-severity issue in .github/workflows/ci.yml: in a case pattern, keppo-bot[bot] is interpreted as a character class, so the intended literal bot account is not actually allowlisted.
  • Because this can change CI/workflow behavior in a deterministic way (confidence 10/10), it introduces real merge risk even though the problem is localized to workflow logic.
  • Pay close attention to .github/workflows/ci.yml - fix the case matching to treat the bot login as a literal value so allowlisting works as intended.
Prompt for AI agents (unresolved issues)

Check if these issues are valid β€” if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".github/workflows/ci.yml">

<violation number="1" location=".github/workflows/ci.yml:90">
P1: `keppo-bot[bot]` is treated as a glob character class in `case`, so the literal bot login is not actually allowlisted.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

echo "is_privileged=false" >> $GITHUB_OUTPUT
fi
case "$AUTHOR" in
wwwillchen|keppo-bot|keppo-bot[bot]|app/keppo-bot|dyad-assistant|app/dyad-assistant|azizmejri1)
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Apr 7, 2026

Choose a reason for hiding this comment

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

P1: keppo-bot[bot] is treated as a glob character class in case, so the literal bot login is not actually allowlisted.

Prompt for AI agents
Check if this issue is valid β€” if so, understand the root cause and fix it. At .github/workflows/ci.yml, line 90:

<comment>`keppo-bot[bot]` is treated as a glob character class in `case`, so the literal bot login is not actually allowlisted.</comment>

<file context>
@@ -86,17 +86,20 @@ jobs:
-            echo "is_privileged=false" >> $GITHUB_OUTPUT
-          fi
+          case "$AUTHOR" in
+            wwwillchen|keppo-bot|keppo-bot[bot]|app/keppo-bot|dyad-assistant|app/dyad-assistant|azizmejri1)
+              echo "is_privileged=true" >> $GITHUB_OUTPUT
+              ;;
</file context>
Suggested change
wwwillchen|keppo-bot|keppo-bot[bot]|app/keppo-bot|dyad-assistant|app/dyad-assistant|azizmejri1)
wwwillchen|keppo-bot|keppo-bot\[bot\]|app/keppo-bot|dyad-assistant|app/dyad-assistant|azizmejri1)
Fix with Cubic

…p/keppo-bot matches from GitHub login checks\n- keep raw bot login handling on keppo-bot[bot]\n\nCo-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

View 3 additional findings in Devin Review.

Open in Devin Review

echo "is_privileged=false" >> $GITHUB_OUTPUT
fi
case "$AUTHOR" in
wwwillchen|keppo-bot|keppo-bot[bot]|app/keppo-bot|dyad-assistant|app/dyad-assistant|azizmejri1)
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot Apr 7, 2026

Choose a reason for hiding this comment

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

πŸ”΄ Bash case pattern treats [bot] as a character class, never matching keppo-bot[bot]

In the "Detect privileged author" step, the bash case pattern keppo-bot[bot] uses unquoted brackets. In bash glob matching (used by case), [bot] is a character class matching a single character: 'b', 'o', or 't'. So the pattern matches keppo-botb, keppo-boto, or keppo-bott β€” but never the literal string keppo-bot[bot]. I verified this directly by running a test in bash which returned NO MATCH for the literal keppo-bot[bot] input. The brackets need to be quoted (e.g. "keppo-bot[bot]") to match literally.

Open in Devin Review

Was this helpful? React with πŸ‘ or πŸ‘Ž to provide feedback.

Comment on lines +62 to +63
allowed_non_write_users: "princeaden1,nourzakhama2003,ryangroch" # remember, we already filter above.
allowed_bots: "keppo-bot[bot]"
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.

πŸ”΄ dyad-assistant removed from allowed_non_write_users without being added to allowed_bots

In claude-pr-review.yml:62, the old code had dyad-assistant in allowed_non_write_users. This PR removed it (along with keppo-bot), but while keppo-bot was moved to the new allowed_bots parameter as keppo-bot[bot], dyad-assistant was dropped entirely. If dyad-assistant doesn't have write access to the repo, PRs from this account will pass the workflow's if condition (line 24-28) but then be rejected by claude-code-action's internal access check, since dyad-assistant is in neither allowed_non_write_users nor allowed_bots. This is a regression from the previous behavior where dyad-assistant PRs were processed.

Prompt for agents
In claude-pr-review.yml, dyad-assistant was removed from allowed_non_write_users but not added to allowed_bots. Determine whether dyad-assistant is a GitHub App bot (in which case add 'dyad-assistant[bot]' to allowed_bots alongside keppo-bot[bot]) or a regular service account without write access (in which case add it back to allowed_non_write_users). The old code had it in allowed_non_write_users, suggesting it lacks write access. The analogous treatment of keppo-bot (moved from allowed_non_write_users to allowed_bots as keppo-bot[bot]) should inform how dyad-assistant is handled.
Open in Devin Review

Was this helpful? React with πŸ‘ or πŸ‘Ž to provide feedback.

…te app/dyad-assistant matches from GitHub login checks\n- keep dyad-assistant as the remaining allowed assistant login\n\nCo-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

πŸ’‘ Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cac55c8337

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with πŸ‘.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

echo "is_privileged=false" >> $GITHUB_OUTPUT
fi
case "$AUTHOR" in
wwwillchen|keppo-bot|keppo-bot[bot]|dyad-assistant|azizmejri1)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Escape keppo-bot[bot] in privileged-author case arm

In Bash case patterns, [ and ] are glob metacharacters, so keppo-bot[bot] is interpreted as a character class rather than the literal login string. This arm will match values like keppo-botb/keppo-boto/keppo-bott, but not the actual GitHub App author keppo-bot[bot], causing that bot to be marked is_privileged=false and routed to the non-privileged CI matrix. Use an escaped literal pattern (for example keppo-bot\[bot\]) or a direct string comparison.

Useful? React with πŸ‘Β / πŸ‘Ž.

@wwwillchen wwwillchen merged commit 7b74c5f into main Apr 7, 2026
13 of 14 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

🎭 Playwright Test Results

❌ Some tests failed

OS Passed Failed Flaky Skipped
🍎 macOS 409 5 7 131
πŸͺŸ Windows 405 8 3 131

Summary: 814 passed, 13 failed, 10 flaky, 262 skipped

Failed Tests

🍎 macOS

  • context_limit_banner.spec.ts > context limit banner shows 'running out' when near context limit
    • Error: expect(locator).toBeVisible() failed
  • queued_message.spec.ts > editing queued message restores attachments and selected components
    • Error: expect(locator).toBeVisible() failed
  • queued_message.spec.ts > canceling queued message edit clears restored components
    • Error: expect(locator).toBeVisible() failed
  • setup_flow.spec.ts > Setup Flow > node.js install flow
    • TimeoutError: locator.dispatchEvent: Timeout 30000ms exceeded.
  • visual_editing.spec.ts > swap image via URL
    • TimeoutError: locator.click: Timeout 30000ms exceeded.

πŸͺŸ Windows

  • context_manage.spec.ts > manage context - default
    • Error: expect(locator).toMatchAriaSnapshot(expected) failed
  • context_manage.spec.ts > manage context - smart context
    • Error: expect(locator).toMatchAriaSnapshot(expected) failed
  • context_manage.spec.ts > manage context - exclude paths
    • Error: expect(locator).toMatchAriaSnapshot(expected) failed
  • context_manage.spec.ts > manage context - exclude paths with smart context
    • Error: expect(locator).toMatchAriaSnapshot(expected) failed
  • github.spec.ts > create and sync to new repo
    • Error: expect(locator).toMatchAriaSnapshot(expected) failed
  • github.spec.ts > create and sync to existing repo
    • Error: expect(locator).toMatchAriaSnapshot(expected) failed
  • github.spec.ts > create and sync to existing repo - custom branch
    • Error: expect(locator).toMatchAriaSnapshot(expected) failed
  • github.spec.ts > github clear integration settings
    • Error: expect(locator).toBeVisible() failed

πŸ“‹ Re-run Failing Tests (macOS)

Copy and paste to re-run all failing spec files locally:

npm run e2e \
  e2e-tests/context_limit_banner.spec.ts \
  e2e-tests/queued_message.spec.ts \
  e2e-tests/setup_flow.spec.ts \
  e2e-tests/visual_editing.spec.ts

⚠️ Flaky Tests

🍎 macOS

  • approve.spec.ts > write to index, approve, check preview (passed after 1 retry)
  • context_manage.spec.ts > manage context - exclude paths with smart context (passed after 1 retry)
  • fix_error.spec.ts > fix error with AI (passed after 1 retry)
  • select_component.spec.ts > upgrade app to select component (passed after 1 retry)
  • setup_flow.spec.ts > Setup Flow > setup banner shows correct state when node.js is installed (passed after 1 retry)
  • smart_context_options.spec.ts > switching smart context mode saves the right setting (passed after 1 retry)
  • template-create-nextjs.spec.ts > create next.js app (passed after 1 retry)

πŸͺŸ Windows

  • concurrent_chat.spec.ts > concurrent chat (passed after 1 retry)
  • edit_code.spec.ts > edit code (passed after 1 retry)
  • setup_flow.spec.ts > Setup Flow > setup banner shows correct state when node.js is installed (passed after 1 retry)

πŸ“Š View full report

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants