fix(github-bot): use numeric user ID in prompt authorId

[ColeMurray]

Summary

• Changes authorId in all 4 github-bot handlers from github:${sender.login} to github:${sender.id}, using the stable numeric GitHub user ID instead of the mutable login name.
• This is Step 1 of the git attribution fix plan. The D1 user_identities table is keyed by provider_user_id (numeric), so this change enables the router-level identity enrichment in Step 2 to resolve user identity via UserStore.getIdentity(provider, providerUserId) for all three bot types without additional query methods.

Affected handlers

Handler	Trigger
handleReviewRequested	Review assignment on a PR
handlePullRequestOpened	Auto-review when a PR is opened
handleIssueComment	Issue or PR conversation comment
handleReviewComment	PR review comment

Backward compatibility

Existing participants created with the old github:${login} format will not be found by getByUserId("github:${id}"), so a new participant will be created on the next prompt. This is expected — the enrichment in Step 2/3 will create it with correct identity data.

Test plan

• All 39 handler tests pass with updated assertions (github:alice → github:1001, etc.)
• Verify no regressions in CI (lint, typecheck, full test suite)

Summary by CodeRabbit

• Bug Fixes

  • Improved error messaging when archiving sessions fails—users now receive toast notifications instead of silent failures.

• Tests

  • Added coverage for archive failures on mobile devices.

• Refactor

  • Updated author identification mechanism in GitHub integrations.
  • Simplified callback signatures for dialog components.

• Documentation

  • Added clarifying comments to session management code.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR changes GitHub webhook handlers to use numeric sender id instead of username for identity purposes, updates error reporting in session archival from console logging to toast notifications, narrows the ArchiveSessionDialog callback type signature to sync-only, and adds test coverage for mobile archive failure scenarios with documentation comments.

Changes

Cohort / File(s)	Summary
GitHub Bot Identity Provider packages/github-bot/src/handlers.ts, packages/github-bot/test/handlers.test.ts	Switched all four webhook handlers to derive authorId from numeric sender.id (e.g., github:1001) instead of sender.login (e.g., github:alice). Updated corresponding test expectations to validate numeric ID format.
Archive Session Error Handling packages/web/src/components/archive-session-dialog.tsx, packages/web/src/lib/archive-session.ts	Narrowed onConfirm callback type from `() => void
Session Management & Testing packages/web/src/components/session-sidebar.tsx, packages/web/src/components/session-sidebar.test.tsx	Added inline documentation comments explaining archive cache invalidation and pagination handling. Added mobile-specific test case verifying session persistence when archive request fails with HTTP 500.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat: forward identity fields from all bots to control plane #552: Directly related — modifies the same GitHub bot handlers to switch identity usage from login to numeric sender.id for downstream session creation.
• feat: Add a button to archive sessions from the sidebar #570: Directly related — previously added ArchiveSessionDialog and archiveSession; this PR refines their type signatures and error handling behavior.
• fix: add error toasts and failure test for sidebar archive #576: Directly related — matches identical file-level edits across web components (dialog type narrowing, sidebar comments, error handling toast changes, and mobile test coverage).

Suggested reviewers

• open-inspect

Poem

🐰 Numeric IDs now guide the way,
Console logs fade, toasts here to stay,
Types grow tighter, stricter and keen,
Mobile tests catch what's in between,
The handlers hop with renewed ID pride! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: converting authorId in github-bot handlers from login-based to numeric ID format, which is the primary focus across the file modifications.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/github-bot-author-id-format

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[github-actions]

Terraform Validation Results

Step	Status
Format	✅
Init	✅
Validate	✅

Note: Terraform plan was skipped because secrets are not configured. This is expected for external contributors. See docs/GETTING_STARTED.md for setup instructions.

Pushed by: @ColeMurray, Action: pull_request

[open-inspect]

Summary

PR Title and number: fix(github-bot): use numeric user ID in prompt authorId (#577)
Author: @ColeMurray
Files changed: 6 files, +58 / -12

This updates the github-bot handlers to use the stable numeric GitHub user ID in authorId, and includes a small web follow-up to surface archive failures to users. I reviewed the changed files and did not find any correctness, security, performance, or maintainability issues that should block merging.

Critical Issues

None.

Suggestions

None.

Nitpicks

None.

Positive Feedback

• The authorId change is applied consistently across all four github-bot entry points, and the handler assertions were updated to match.
• The archive-session follow-up improves user-facing behavior by replacing silent console logging with an error toast.
• The added failure-path sidebar test covers the key regression risk for the web change.

Questions

None.

Verdict

Approve

Verified locally:

• npm test -w @open-inspect/github-bot: all 100 tests passed
• npm test -w @open-inspect/web -- session-sidebar: all 6 tests passed

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/web/src/lib/archive-session.ts (1)

44-46: Preserve diagnostics in catch path.

Line 44 swallows the thrown error entirely. Keep the toast, but also capture/log the exception so production failures remain debuggable.

Suggested patch

-  } catch {
+  } catch (error) {
+    console.error("Failed to archive session", error);
     toast.error("Failed to archive session");
     return false;
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/web/src/lib/archive-session.ts` around lines 44 - 46, The catch
block in archiveSession currently swallows exceptions; update the catch to
accept the error (e.g., catch (err)) and log or report it before returning false
— for example call console.error("Failed to archive session", err) and, if a
telemetry helper (Sentry.captureException or appLogger) exists in this module,
forward the error there as well — keep the existing toast.error call and then
return false.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/web/src/components/archive-session-dialog.tsx`:
- Line 17: The onConfirm prop on ArchiveSessionDialog is currently typed as ()
=> void but callers pass async functions; update its type to () => Promise<void>
(or () => void | Promise<void> if you prefer to allow sync handlers) in the
component props/interface (the onConfirm declaration) and then ensure the local
confirmation handler that invokes onConfirm (e.g., the component's
confirm/handleConfirm function) awaits the call (make it async) so the async
contract is respected.

---

Nitpick comments:
In `@packages/web/src/lib/archive-session.ts`:
- Around line 44-46: The catch block in archiveSession currently swallows
exceptions; update the catch to accept the error (e.g., catch (err)) and log or
report it before returning false — for example call console.error("Failed to
archive session", err) and, if a telemetry helper (Sentry.captureException or
appLogger) exists in this module, forward the error there as well — keep the
existing toast.error call and then return false.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 86a19bfd-9009-4f1e-837b-09cc95b6b0dd

📥 Commits

Reviewing files that changed from the base of the PR and between 891abe3 and 45f4fde.

📒 Files selected for processing (6)

• packages/github-bot/src/handlers.ts
• packages/github-bot/test/handlers.test.ts
• packages/web/src/components/archive-session-dialog.tsx
• packages/web/src/components/session-sidebar.test.tsx
• packages/web/src/components/session-sidebar.tsx
• packages/web/src/lib/archive-session.ts

[github-actions]

Terraform Validation Results

Step	Status
Format	✅
Init	✅
Validate	✅

Note: Terraform plan was skipped because secrets are not configured. This is expected for external contributors. See docs/GETTING_STARTED.md for setup instructions.

Pushed by: @ColeMurray, Action: pull_request