Skip to content

Fix: derive bot identity from the token, not the GITHUB_ACTIONS flag - #3

Merged
bilikaz merged 2 commits into
mainfrom
fix-3
Jun 9, 2026
Merged

bilikaz merged 2 commits into
mainfrom
fix-3

Conversation

@bilikaz

@bilikaz bilikaz commented Jun 9, 2026

Copy link
Copy Markdown
Owner

Under a user token (e.g. a PAT, including one used inside Actions) the reviewer posts comments as that user, but create() hardcoded botLogin to "github-actions[bot]" whenever GITHUB_ACTIONS=true. So its own prior inline threads came back classified "human", hasUnresolvedBotThreads() was false, and the reconcile/verify follow-up step was skipped.

Resolve botLogin from the token via GET /user (correct for user tokens + PATs), falling back to "github-actions[bot]" only when /user 403s (the default Actions App token). Also removes the direct process.env read from github.ts, honoring the "env is read only in loadConfig()" rule.

(Also: gitignore the local harness/ subdir.)

Under a user token (e.g. a PAT, including one used inside Actions) the reviewer
posts comments as that user, but create() hardcoded botLogin to
"github-actions[bot]" whenever GITHUB_ACTIONS=true. So its own prior inline
threads came back classified "human", hasUnresolvedBotThreads() was false, and
the reconcile/verify follow-up step was skipped.

Resolve botLogin from the token via GET /user (correct for user tokens + PATs),
falling back to "github-actions[bot]" only when /user 403s (the default Actions
App token). Also removes the direct process.env read from github.ts, honoring
the "env is read only in loadConfig()" rule.

(Also: gitignore the local harness/ subdir.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@bilikaz bilikaz left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

1 informational finding(s) (nits / suggestions, not blocking — listed here as summary, not tracked as individual threads):

  • src/vcs/github.ts — The bare catch without as any may trigger TypeScript strict error handling. Consider adding catch (err: unknown) or catch (_err: unknown) to satisfy strict TS, though the fallback behavior is …

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@bilikaz bilikaz left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

2 informational finding(s) (nits / suggestions, not blocking — listed here as summary, not tracked as individual threads):

  • src/vcs/github.ts — The approve() method silently returns when no summary is provided. While this matches the described behavior, consider adding a comment or assertion to make this no-op explicit for future maintainer…
  • src/vcs/github.ts — The try-catch around client.users.getAuthenticated() is correct. However, consider catching only specific errors (e.g., 403) rather than all errors, to avoid silently swallowing unexpected failures …

@bilikaz
bilikaz merged commit 7187842 into main Jun 9, 2026
2 checks passed
@bilikaz
bilikaz deleted the fix-3 branch June 9, 2026 14:52
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.

1 participant