Skip to content

Filter sessions by creator#688

Merged
ColeMurray merged 10 commits into
mainfrom
session-creator-filter
May 30, 2026
Merged

Filter sessions by creator#688
ColeMurray merged 10 commits into
mainfrom
session-creator-filter

Conversation

@ColeMurray

@ColeMurray ColeMurray commented May 30, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add createdBy session-list filtering for canonical user IDs in the control plane and web BFF
  • Add a sidebar All/Mine filter that resolves the current canonical user via /api/me
  • Revalidate all session-list SWR cache variants and add a D1 index for creator-filtered recency queries

Validation

  • npm test -w @open-inspect/control-plane -- src/db/session-index.test.ts
  • npm run test:integration -w @open-inspect/control-plane -- test/integration/auth.test.ts
  • npm test -w @open-inspect/web -- src/lib/session-list.test.ts src/lib/control-plane-query.test.ts src/components/session-sidebar.test.tsx
  • npm run typecheck -w @open-inspect/control-plane
  • npm run lint -w @open-inspect/control-plane -- --quiet
  • npm run typecheck -w @open-inspect/web
  • npm run lint -w @open-inspect/web -- --quiet
  • npm run build -w @open-inspect/shared
  • npm run build -w @open-inspect/control-plane
  • npm run build -w @open-inspect/web
  • npx prettier --check ...changed TS/TSX files
  • git diff --check

Summary by CodeRabbit

  • New Features

    • Sidebar ownership filter (All vs Mine); session list supports filtering by creator (createdBy) and scope=mine resolves the current user.
  • Performance

    • New DB index to speed creator-filtered session queries.
  • Improvements

    • Unified session-list cache keys and more reliable cache invalidation for rename, archive/unarchive, and pagination.
  • Bug Fixes

    • Invalid createdBy or scope now return proper 400 errors.
  • Tests

    • Added coverage for createdBy/scope handling, cache-key behavior, and related flows.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 30, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds creator-based session filtering end-to-end: control-plane parses/validates repeated createdBy params, DB uses user_id IN (...) and index, /api/sessions and /api/me integrate current-user resolution, frontend builds createdBy-aware cache keys and a sidebar “Mine” filter, and SWR invalidation is unified to the unarchived list key.

Changes

Session filtering by creator

Layer / File(s) Summary
Database schema and filtering support
packages/control-plane/src/db/session-index.ts, packages/control-plane/src/db/session-index.test.ts, terraform/d1/migrations/0021_add_sessions_user_updated_at_index.sql
ListSessionsOptions adds createdByUserIds; SQL builder appends user_id IN (...) with bound params. Fake D1 test DB supports user_id IN (...). Tests added for single/multiple creator filters. Migration adds (user_id, updated_at DESC) index.
Backend API validation and routing
packages/control-plane/src/router.ts, packages/control-plane/test/integration/auth.test.ts
parseCreatedByFilters reads repeated createdBy query params, validates canonical user IDs, dedupes, returns 400 on invalid input, and /sessions forwards createdByUserIds to the store. Integration tests seed sessions and assert filter and error behaviors.
Sessions API route and current-user resolution
packages/web/src/app/api/sessions/route.ts, packages/web/src/lib/current-user.ts, packages/web/src/app/api/me/route.ts, packages/web/src/app/api/sessions/route.test.ts
GET /api/sessions validates scope, forbids combining scope=mine with client createdBy, resolves current user via resolveCurrentUserId for scope=mine, rewrites createdBy and forwards to control plane. New resolveCurrentUserId helper added and /api/me updated to use it. Tests cover success and error/validation paths.
Frontend cache key infrastructure and allowlist
packages/web/src/lib/session-list.ts, packages/web/src/lib/session-list.test.ts, packages/web/src/lib/control-plane-query.ts
Adds SESSIONS_API_PATH; buildSessionsPageKey accepts scope and createdBy[] and emits one createdBy= per id; isSessionListKey validates keys against SESSIONS_API_PATH. SESSION_CONTROL_PLANE_QUERY_PARAMS include createdBy. Tests added for key generation and validators.
Sidebar session ownership filter UI
packages/web/src/components/session-sidebar.tsx, packages/web/src/components/session-sidebar.test.tsx
Adds "All"/"Mine" toggle, sessionScope state, uses filter-scoped SWR keys built by buildSessionsPageKey, adapts pagination/load-more and archive/rename handlers to mutate the filter-scoped cache. Tests stub fetch and verify the "Mine" filter fetch and stale-response behavior.
Cache coherency and SWR mutation updates
packages/web/src/app/(app)/page.tsx, packages/web/src/app/(app)/session/[id]/page.tsx, packages/web/src/components/settings/data-controls-settings.tsx, packages/web/src/hooks/use-session-socket.ts, related tests
Components now mutate/revalidate using the centralized unarchived-session list key (isUnarchivedSessionListKey) after archive/unarchive/title changes and socket events. Session rename flow refactored to use useSWRMutation and mutate with an updater for SessionListResponse. Tests updated to assert new keys.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant Sidebar as SessionSidebar
  participant MeAPI as /api/me
  participant API as /api/sessions
  participant ControlPlane as ControlPlane
  participant DB as Database

  User->>Sidebar: select "Mine" filter
  Sidebar->>MeAPI: GET /api/me (resolve current user)
  MeAPI->>ControlPlane: PUT /provider-identities/github/{id}
  ControlPlane-->>MeAPI: { userId }
  MeAPI-->>Sidebar: { userId }
  Sidebar->>Sidebar: buildSessionsPageKey({ createdBy: [userId] })
  Sidebar->>API: GET /api/sessions?createdBy=userId
  API->>ControlPlane: GET /sessions?createdBy=userId
  ControlPlane->>DB: list sessions WHERE user_id IN (?)
  DB-->>ControlPlane: filtered sessions
  ControlPlane-->>API: [filtered sessions]
  API-->>Sidebar: render filtered sessions
  User->>Sidebar: archive session
  Sidebar->>API: POST /sessions/{id}/archive
  API-->>Sidebar: success
  Sidebar->>SWR: mutate(isUnarchivedSessionListKey) remove archived
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • open-inspect

Poem

🐰 I hopped through queries, keys, and tests,
I nudged the sidebar to show "Mine" at best.
From PUT to index, IDs line by line,
Filters and caches now sing in time.
🥕✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% 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 'Filter sessions by creator' clearly and concisely summarizes the primary objective of the pull request, which adds filtering capability for sessions by their creator across the control plane and web BFF.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch session-creator-filter

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

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

@github-actions

Copy link
Copy Markdown

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

data: currentUser,
error: currentUserError,
isLoading: currentUserLoading,
} = useSWR<CurrentUserResponse>(shouldResolveCurrentUser ? "/api/me" : null);

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.

[deep review] This pushes canonical-user resolution and a second fetch path into SessionSidebar, which is already a 944-line component. The new scope feature now owns sessionScope, /api/me SWR state, a derived ID array, null-key gating, combined loading/error state, and pagination has to mirror the same filter state later. I think there is a code-judo move here: keep canonical identity resolution in the BFF layer and let this component express only the desired scope, e.g. a sessions key like /api/sessions?...&scope=mine that the BFF resolves to canonical createdBy before forwarding to the control plane. That would delete the /api/me coupling from this component and prevent another feature-specific orchestration branch in an already overgrown file. Can we restructure this boundary before landing it?

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.

Done in b4d6b71. SessionSidebar now only builds /api/sessions?...&scope=mine; /api/sessions resolves the signed-in user to canonical createdBy in the BFF before forwarding to the control plane. That removes the /api/me SWR branch, derived creator ID array, and null-key gating from the sidebar.


await mutate<SessionListResponse>(
SIDEBAR_SESSIONS_KEY,
sidebarSessionsKey,

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.

This path only updates the currently active session-list cache key. With the new All/Mine variants, archiving or renaming from the sidebar can leave the other variant stale, so switching scopes may briefly show the old title or an archived session until SWR revalidates. The other touched paths use mutate(isSessionListKey, ...); could we use the same predicate here so all cached session-list variants stay consistent?

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.

Done in b4d6b71. Sidebar archive and rename now mutate all unarchived session-list cache variants through isUnarchivedSessionListKey, so All and Mine stay consistent without touching archived settings-list caches.

@open-inspect open-inspect Bot left a comment

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.

Summary

PR #688, “Filter sessions by creator” by @ColeMurray, adds creator-based filtering through the control plane and web BFF, plus an All/Mine sidebar scope and cache-key helpers. Reviewed 15 changed files (+379/-60); the core query plumbing, validation, and D1 indexing look sound.

Critical Issues

None found.

Suggestions

  • [Cache consistency] packages/web/src/components/session-sidebar.tsx:276 - Sidebar archive/rename cache updates currently target only the active session-list key. I left an inline comment suggesting the same isSessionListKey predicate used elsewhere so cached All/Mine variants remain consistent after sidebar actions.

Nitpicks

None.

Positive Feedback

  • The control-plane filter uses validated canonical user IDs and parameterized SQL, which avoids injection issues.
  • The session-list key builder and predicate are small and covered by focused unit tests.
  • The migration adds the right creator/recency index for the new filtered query shape.

Questions

None.

Verdict

Comment: no blocking issues found; one non-blocking cache consistency suggestion is inline.

Tests not run locally because the workspace was on main; review was performed against the fetched PR head and gh pr diff 688.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
terraform/d1/migrations/0021_add_sessions_user_updated_at_index.sql (1)

1-3: 💤 Low value

Note the complementary index for different orderings.

This adds a second index on user_id with updated_at DESC, complementing the existing idx_sessions_user_id on (user_id, created_at DESC) from migration 0019. Both are necessary if queries filter by creator and order by either recency (updated_at) or chronological creation (created_at). Confirm that both orderings are actually used; if created_at ordering for user-filtered queries is deprecated, consider removing the older index in a future migration to reduce write overhead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@terraform/d1/migrations/0021_add_sessions_user_updated_at_index.sql` around
lines 1 - 3, You added idx_sessions_user_updated_at (ON sessions(user_id,
updated_at DESC)) which complements the existing idx_sessions_user_id on
(user_id, created_at DESC); verify that queries actually use both orderings by
checking application code and running EXPLAIN on user-filtered queries that
ORDER BY updated_at DESC and ORDER BY created_at DESC, and if created_at
ordering is no longer used schedule a follow-up migration to DROP INDEX
idx_sessions_user_id to avoid extra write overhead; also consider adding a brief
SQL comment in this migration referencing idx_sessions_user_id and the reason
both indexes exist to aid future maintainers.
packages/web/src/app/(app)/session/[id]/page.tsx (1)

214-272: ⚖️ Poor tradeoff

Session rename refactor improves robustness but expands PR scope.

The rename flow was refactored from a direct mutate-based pattern to useSWRMutation + mutate. This separates the API call (lines 214–227) from the cache update (lines 262–265), which is more robust and easier to test. The error handling at lines 258–261 and 267–269 ensures that failures are caught and the caller (renameSession) can return false to signal failure.

This is a good improvement, but it's a substantial refactor that goes beyond the PR's stated goal of adding creator filtering. Consider splitting such refactors into separate PRs or explicitly documenting them in the PR description to help reviewers and future maintainers understand the scope of changes.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/web/src/app/`(app)/session/[id]/page.tsx around lines 214 - 272, The
rename refactor (introducing useSWRMutation/triggerRename and the new
renameSession implementation) expands the PR scope; either revert to the
previous direct-mutate approach or split/document the refactor: revert changes
around useSWRMutation/triggerRename and restore the original mutate-based API
call inside renameSession (and keep handleArchive unchanged), or move the new
useSWRMutation logic into its own PR and update this PR description to
explicitly note the renameSession/useSWRMutation refactor and rationale,
referencing triggerRename, useSWRMutation, renameSession, mutate, and
isSessionListKey so reviewers can find the lines to revert or extract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/web/src/app/`(app)/session/[id]/page.tsx:
- Around line 214-272: The rename refactor (introducing
useSWRMutation/triggerRename and the new renameSession implementation) expands
the PR scope; either revert to the previous direct-mutate approach or
split/document the refactor: revert changes around useSWRMutation/triggerRename
and restore the original mutate-based API call inside renameSession (and keep
handleArchive unchanged), or move the new useSWRMutation logic into its own PR
and update this PR description to explicitly note the
renameSession/useSWRMutation refactor and rationale, referencing triggerRename,
useSWRMutation, renameSession, mutate, and isSessionListKey so reviewers can
find the lines to revert or extract.

In `@terraform/d1/migrations/0021_add_sessions_user_updated_at_index.sql`:
- Around line 1-3: You added idx_sessions_user_updated_at (ON sessions(user_id,
updated_at DESC)) which complements the existing idx_sessions_user_id on
(user_id, created_at DESC); verify that queries actually use both orderings by
checking application code and running EXPLAIN on user-filtered queries that
ORDER BY updated_at DESC and ORDER BY created_at DESC, and if created_at
ordering is no longer used schedule a follow-up migration to DROP INDEX
idx_sessions_user_id to avoid extra write overhead; also consider adding a brief
SQL comment in this migration referencing idx_sessions_user_id and the reason
both indexes exist to aid future maintainers.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2182219e-92c4-4ac7-87e1-d5fd17332aa6

📥 Commits

Reviewing files that changed from the base of the PR and between 481892b and d3a3424.

📒 Files selected for processing (15)
  • packages/control-plane/src/db/session-index.test.ts
  • packages/control-plane/src/db/session-index.ts
  • packages/control-plane/src/router.ts
  • packages/control-plane/test/integration/auth.test.ts
  • packages/web/src/app/(app)/page.tsx
  • packages/web/src/app/(app)/session/[id]/page.tsx
  • packages/web/src/components/session-sidebar.test.tsx
  • packages/web/src/components/session-sidebar.tsx
  • packages/web/src/components/settings/data-controls-settings.tsx
  • packages/web/src/hooks/use-session-socket.ts
  • packages/web/src/lib/control-plane-query.test.ts
  • packages/web/src/lib/control-plane-query.ts
  • packages/web/src/lib/session-list.test.ts
  • packages/web/src/lib/session-list.ts
  • terraform/d1/migrations/0021_add_sessions_user_updated_at_index.sql

@github-actions

Copy link
Copy Markdown

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/web/src/app/`(app)/session/[id]/page.tsx:
- Around line 262-265: The cache update only targets isUnarchivedSessionListKey
causing archived lists to drift; update the two mutate calls that currently use
isUnarchivedSessionListKey (the ones invoking
mutate<SessionListResponse>(isUnarchivedSessionListKey, updateSessionsTitle,
...)) so they either use the broader session-list predicate key used elsewhere
(the shared session list predicate) or call mutate for both
isUnarchivedSessionListKey and the corresponding archived list key (e.g.,
isArchivedSessionListKey) with the same
updateSessionsTitle/populateCache/revalidate options to keep archived and
unarchived caches in sync during rename and unarchive flows.
🪄 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: d13e7e88-744f-4a84-866b-5db2fc6f9342

📥 Commits

Reviewing files that changed from the base of the PR and between d3a3424 and e52b57c.

📒 Files selected for processing (7)
  • packages/web/src/app/(app)/page.tsx
  • packages/web/src/app/(app)/session/[id]/page.tsx
  • packages/web/src/components/settings/data-controls-settings.tsx
  • packages/web/src/hooks/use-session-socket.test.tsx
  • packages/web/src/hooks/use-session-socket.ts
  • packages/web/src/lib/session-list.test.ts
  • packages/web/src/lib/session-list.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/web/src/hooks/use-session-socket.test.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
  • packages/web/src/components/settings/data-controls-settings.tsx
  • packages/web/src/app/(app)/page.tsx
  • packages/web/src/hooks/use-session-socket.ts
  • packages/web/src/lib/session-list.test.ts
  • packages/web/src/lib/session-list.ts

Comment thread packages/web/src/app/(app)/session/[id]/page.tsx
@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

@ColeMurray

Copy link
Copy Markdown
Owner Author

Review feedback has been addressed and pushed.

Commits:

  • e52b57c fixed the failing web test by avoiding broad archived-list revalidation during unarchive.
  • b4d6b71 moved scope=mine resolution into the sessions BFF and removed /api/me orchestration from SessionSidebar.
  • 1868bc1 added targeted archived-list cache updates for detail-page rename/unarchive flows.

Local validation passed:

  • npm run lint -w @open-inspect/web -- --quiet
  • npm run typecheck -w @open-inspect/web
  • npm test -w @open-inspect/web
  • npm run build -w @open-inspect/web

@github-actions

Copy link
Copy Markdown

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/web/src/app/api/sessions/route.ts`:
- Around line 34-47: Do not call resolveCurrentUserId() unconditionally for
scope === "mine" because it performs a control-plane PUT; instead, use the user
id already present on session.user (e.g., session.user.id or session.user.sub)
to append createdBy to searchParams, and only fall back to calling
resolveCurrentUserId() if session.user lacks an id. Update the block that
currently calls resolveCurrentUserId and appends createdBy so it first checks
session.user for an id and uses that value, calling resolveCurrentUserId() only
as a fallback.
- Around line 27-31: The validation currently treats an empty scope value as
falsy and allows it through; update the check in the route handler where
searchParams, scopes and scope are used so that you reject when scopes.length >
1 OR scope is an empty string OR scope is not in SESSION_SCOPES (i.e.,
explicitly test scope === "" in addition to the existing invalid membership
check) and return the same 400 NextResponse.json({ error: "Invalid scope" });
also add a regression test that sends ?scope= (empty value) to this sessions
route and asserts a 400 response.
🪄 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: e8e0faf8-1b5e-445b-aadc-4c180a2b9e6e

📥 Commits

Reviewing files that changed from the base of the PR and between e52b57c and 1868bc1.

📒 Files selected for processing (9)
  • packages/web/src/app/(app)/session/[id]/page.tsx
  • packages/web/src/app/api/me/route.ts
  • packages/web/src/app/api/sessions/route.test.ts
  • packages/web/src/app/api/sessions/route.ts
  • packages/web/src/components/session-sidebar.test.tsx
  • packages/web/src/components/session-sidebar.tsx
  • packages/web/src/lib/current-user.ts
  • packages/web/src/lib/session-list.test.ts
  • packages/web/src/lib/session-list.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/web/src/lib/session-list.ts
  • packages/web/src/app/(app)/session/[id]/page.tsx

Comment thread packages/web/src/app/api/sessions/route.ts Outdated
Comment thread packages/web/src/app/api/sessions/route.ts Outdated
Comment thread packages/web/src/components/session-sidebar.tsx Outdated
@github-actions

Copy link
Copy Markdown

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

@ColeMurray

ColeMurray commented May 30, 2026

Copy link
Copy Markdown
Owner Author

Follow-up CI and review fixes are pushed.

  • All PR checks are now passing on the latest commit.
  • Fixed the control-plane integration race in 791d9a1.
  • Addressed the follow-up review items in 7c5d30d: empty scope validation, cached/coalesced current-user resolution, and stale pagination guarding on scope changes.
  • Replied to the inline review threads.

Local validation passed:

  • npm run test:integration -w @open-inspect/control-plane
  • npm run lint -w @open-inspect/control-plane -- --quiet
  • npm run typecheck -w @open-inspect/control-plane
  • npm run lint -w @open-inspect/web -- --quiet
  • npm run typecheck -w @open-inspect/web
  • npm test -w @open-inspect/web
  • npm run build -w @open-inspect/web

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

@github-actions

Copy link
Copy Markdown

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

@ColeMurray ColeMurray merged commit 7a27bd2 into main May 30, 2026
18 checks passed
@ColeMurray ColeMurray deleted the session-creator-filter branch May 30, 2026 21:48
hreiten added a commit to watchdog-no/background-agents that referenced this pull request May 31, 2026
* Allow Slack sessions to use OpenCode titles (ColeMurray#685)

## Summary
- Stop pre-filling Slack-created session titles from the Slack message
or repo fallback
- Leave Slack session titles unset so OpenCode-generated title events
can populate them
- Add a regression assertion for the Slack session creation payload

## Validation
- npm run build -w @open-inspect/shared
- npm test -w @open-inspect/slack-bot
- npm run typecheck -w @open-inspect/slack-bot
- npm run lint -w @open-inspect/slack-bot

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Refactor**
* Removed title field from session creation requests to the control
plane.

* **Tests**
* Updated tests to verify title field is not included in session
creation requests.

<!-- review_stack_entry_start -->

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/685?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

<!-- review_stack_entry_end -->

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

* Add provider identity resolution API (ColeMurray#686)

## Summary

- Add a shared canonical user ID validator for 32-character lowercase
hex D1 user IDs.
- Add an internal HMAC-authenticated `PUT
/provider-identities/:provider/:providerUserId` control-plane route that
upserts provider identity metadata through
`UserStore.resolveOrCreateUser` and returns the canonical `userId`.
- Add browser-facing `GET /api/me`, deriving GitHub identity from the
server-side NextAuth session and returning only the canonical user ID.

## Validation

- `npm run build -w @open-inspect/shared`
- `npm test -w @open-inspect/shared -- user-id`
- `npm run typecheck -w @open-inspect/shared`
- `npm run lint -w @open-inspect/shared -- --quiet`
- `npm test -w @open-inspect/control-plane --
routes/provider-identities.test.ts router.provider-identities.test.ts`
- `npm run typecheck -w @open-inspect/control-plane`
- `npm run lint -w @open-inspect/control-plane -- --quiet`
- `npm test -w @open-inspect/web -- app/api/me/route.test.ts`
- `npm run typecheck -w @open-inspect/web`
- `npm run lint -w @open-inspect/web -- --quiet`


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* New API to return the authenticated user's GitHub-derived identity and
canonical user ID.
* New endpoints to upsert GitHub provider identities and return a
canonical user ID.
* Shared validator ensuring canonical 32-character lowercase hex user
IDs.

* **Bug Fixes**
* Improved input validation and explicit error responses for invalid
provider-user IDs and malformed requests.
* Better error handling when identity resolution returns unexpected
values.

* **Tests**
* Added integration and unit tests covering success, validation
failures, and error paths.

<!-- review_stack_entry_start -->

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/686?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

<!-- review_stack_entry_end -->
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

* Filter sessions by creator (ColeMurray#688)

## Summary
- Add `createdBy` session-list filtering for canonical user IDs in the
control plane and web BFF
- Add a sidebar All/Mine filter that resolves the current canonical user
via `/api/me`
- Revalidate all session-list SWR cache variants and add a D1 index for
creator-filtered recency queries

## Validation
- `npm test -w @open-inspect/control-plane --
src/db/session-index.test.ts`
- `npm run test:integration -w @open-inspect/control-plane --
test/integration/auth.test.ts`
- `npm test -w @open-inspect/web -- src/lib/session-list.test.ts
src/lib/control-plane-query.test.ts
src/components/session-sidebar.test.tsx`
- `npm run typecheck -w @open-inspect/control-plane`
- `npm run lint -w @open-inspect/control-plane -- --quiet`
- `npm run typecheck -w @open-inspect/web`
- `npm run lint -w @open-inspect/web -- --quiet`
- `npm run build -w @open-inspect/shared`
- `npm run build -w @open-inspect/control-plane`
- `npm run build -w @open-inspect/web`
- `npx prettier --check ...changed TS/TSX files`
- `git diff --check`


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Sidebar ownership filter (All vs Mine); session list supports
filtering by creator (createdBy) and scope=mine resolves the current
user.

* **Performance**
  * New DB index to speed creator-filtered session queries.

* **Improvements**
* Unified session-list cache keys and more reliable cache invalidation
for rename, archive/unarchive, and pagination.

* **Bug Fixes**
  * Invalid createdBy or scope now return proper 400 errors.

* **Tests**
* Added coverage for createdBy/scope handling, cache-key behavior, and
related flows.

<!-- review_stack_entry_start -->

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/688?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

<!-- review_stack_entry_end -->
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

* fix(modal): support non-main Modal environments (ColeMurray#687)

## Summary
- Takes over ColeMurray#629 on a new branch because the original PR head is
company-owned.
- Adds `modal_environment` through the Modal Terraform module so secrets
and deploy commands run in the selected Modal environment.
- Adds `modal_environment_web_suffix` / `MODAL_ENVIRONMENT_WEB_SUFFIX`
for Modal endpoint URL hosts, keeping it separate from the CLI
environment name.
- Keeps `MODAL_WORKSPACE` as the raw workspace, passes
`MODAL_ENVIRONMENT` for dashboard links, and uses the web suffix for
Modal API endpoint slugs.
- Documents the new Modal environment settings and wires them into
Terraform GitHub Actions.

## Addresses Review Feedback
- Owner thread on `terraform/environments/production/modal.tf`: Modal
CLI now receives `MODAL_ENVIRONMENT` for both secret creation and
deploys.
- Owner thread on `packages/control-plane/src/sandbox/client.ts`:
endpoint hosts now use explicit Modal web suffix instead of deriving
from environment name.
- CodeRabbit validation note on `modal_environment`: rejects empty
values, colons, slashes, and backslashes for Modal deployments,
including at the module boundary.
- CodeRabbit deploy script note: validates all required env vars before
first use under `set -u`.

## Validation
- `npm run build -w @open-inspect/shared`
- `npm test -w @open-inspect/control-plane -- --run
src/sandbox/client.test.ts`
- `npm run typecheck -w @open-inspect/control-plane`
- `npm run lint -w @open-inspect/control-plane`
- `terraform fmt -recursive -check terraform`
- `terraform -chdir=terraform/environments/production init
-backend=false`
- `terraform -chdir=terraform/environments/production validate`
- `bash -n terraform/modules/modal-app/scripts/deploy.sh`
- `bash -n terraform/modules/modal-app/scripts/create-secrets.sh`

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Add selectable Modal deployment environments and pass environment to
Modal clients and deploy tooling
* Configurable environment-to-endpoint web-suffix for Modal URLs and
health checks

* **Documentation**
* Updated setup, CI, and Terraform docs to document workspace,
environment, and web-suffix and new secrets/vars

* **Tests**
* Added tests for workspace-slug generation and environment-specific
client health endpoints

<!-- review_stack_entry_start -->

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/687?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

<!-- review_stack_entry_end -->
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Kyle Adams <kadams54@users.noreply.github.com>

* fix(slack-bot): cap App Home repo-override list under Slack's block limit (ColeMurray#689)

## Problem

The App Home tab renders **one block per repo-specific branch override**
with no upper bound (`slack-bot/src/index.ts`). Slack's `views.publish`
rejects any view over **100 blocks**, so once a user configures ~85+
repo overrides the entire Home tab fails to publish — including the UI
needed to manage those overrides, leaving the user stuck.

## Fix

- Cap rendered overrides at **50** (`MAX_RENDERED_REPO_OVERRIDES`), well
under the 100-block ceiling (~15 base blocks).
- Surface the remainder as a `…and N more` summary instead of dropping
it silently.
- Hidden overrides stay **fully manageable** via the existing "Search
repository" selector → modal, which clears any repo's override on empty
submit — so the cap strands nothing.

## Test

Adds a regression test that drives the real `app_home_opened` →
`views.publish` path with 60 overrides and asserts the published view
stays ≤ 100 blocks, renders exactly 50 rows, and includes the "10 more
overrides" note. Full slack-bot suite (87 tests) + typecheck pass.

Found while syncing this change into a downstream fork. Happy to adjust
the cap value or copy.


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved handling of large numbers of repository overrides in the
Slack App Home interface. The interface now displays up to 50 overrides
with a summary note indicating additional hidden overrides, ensuring
functionality within Slack's technical constraints.

<!-- review_stack_entry_start -->

[![Review Change
Stack](https://storage.googleapis.com/coderabbit_public_assets/review-stack-in-coderabbit-ui.svg)](https://app.coderabbit.ai/change-stack/ColeMurray/background-agents/pull/689?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)

<!-- review_stack_entry_end -->

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

* Refactor Slack App Home module (ColeMurray#690)

## Summary
- move Slack App Home publishing, interaction handling, view
construction, modals, metadata, model lookup, and local Slack view types
into focused `app-home/` modules
- extract user preference persistence/resolution into
`user-preferences.ts`
- keep the `/interactions` route delegating App Home-specific payloads
while leaving generic Slack interaction handling in `index.ts`
- move App Home view unit coverage into `app-home.test.ts`

## Validation
- `npm run typecheck -w @open-inspect/slack-bot`
- `npm run lint -w @open-inspect/slack-bot`
- `npx prettier --check packages/slack-bot/src/app-home
packages/slack-bot/src/app-home.test.ts
packages/slack-bot/src/user-preferences.ts`
- `git diff --check`
- `npm test -w @open-inspect/slack-bot`

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* App Home settings: model selection, reasoning-effort choice, global
and per-repo branch overrides with modals.
* Repo search suggestions and truncated option labels to fit Slack
limits; long override lists show a “more” summary.

* **Refactor**
* Centralized App Home interaction routing and unified preference
resolution.
  * App Home publishing made asynchronous and robust.

* **Tests**
* Expanded coverage for App Home view, suggestions, truncation, and
preference updates.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Cole Murray <colemurray.cs@gmail.com>
Co-authored-by: Kyle Adams <kadams54@users.noreply.github.com>
Co-authored-by: Merlin <40275364+merlin-mk@users.noreply.github.com>
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