[codex] Add scoped observer tokens#215
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces scoped, read-only observer tokens (ot_live_*) to secure workspace realtime streams and read-only REST operations, updating the database schema, engine routes, documentation, and SDKs (TypeScript, Python, Rust, Swift) accordingly. It also fixes thread reply mention deliveries for muted members and ensures response-mode HTTP push deliveries remain queued when an ACK signal is missing. Feedback on the changes includes addressing a logical bug where DM events are incorrectly blocked by public channel filters, optimizing token authentication by performing lastUsedAt database updates asynchronously to avoid lock contention, refining the mention-parsing regex to prevent matching email addresses, and utilizing Zod's built-in .datetime() validator for stricter ISO-8601 timestamp validation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if ((type === 'dm.received' || type === 'group_dm.received') && !observerAllowsConversation( | ||
| observer, | ||
| typeof event.conversation_id === 'string' ? event.conversation_id : null, | ||
| )) { | ||
| return false; | ||
| } | ||
|
|
||
| if (!observerAllowsChannel(observer, { | ||
| id: typeof event.channel_id === 'string' ? event.channel_id : null, | ||
| name: eventChannelName(event), | ||
| })) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
When an observer token has channel filters (e.g., channel_names) and include_dms is enabled, DM events will be incorrectly blocked because they are subjected to the observerAllowsChannel check, which will return false since DM conversations do not match public channel filters. We should bypass the channel filter check for DM events.
const conversationId = typeof event.conversation_id === 'string' ? event.conversation_id : null;
const isDm = conversationId !== null || type === 'dm.received' || type === 'group_dm.received';
if (isDm) {
if (!observerAllowsConversation(observer, conversationId)) {
return false;
}
} else {
if (!observerAllowsChannel(observer, {
id: typeof event.channel_id === 'string' ? event.channel_id : null,
name: eventChannelName(event),
})) {
return false;
}
}There was a problem hiding this comment.
Addressed in 3c115b0: observerAllowsEvent now treats events with conversation_id or DM event types as DM resources, so channel filters are bypassed while include_dms / DM id filters still apply. Added conformance coverage for a DM stream observer with channel filters plus include_dms.
| try { | ||
| await db | ||
| .update(observerTokens) | ||
| .set({ lastUsedAt: now }) | ||
| .where(eq(observerTokens.id, row.id)); | ||
| } catch { | ||
| // last_used_at is best-effort audit metadata; authentication should not fail | ||
| // solely because a read replica or adapter rejects this update. | ||
| } |
There was a problem hiding this comment.
Updating lastUsedAt synchronously on every token authentication request can introduce significant database write load and lock contention, especially in SQLite where concurrent writes are serialized. Since lastUsedAt is best-effort audit metadata, we should perform this update asynchronously in the background without blocking the authentication path.
// last_used_at is best-effort audit metadata; do not await to avoid blocking the auth path
void db
.update(observerTokens)
.set({ lastUsedAt: now })
.where(eq(observerTokens.id, row.id))
.catch(() => {});| const mentionPattern = /@(\w+)/g; | ||
| const mentionMatches = data.text.match(mentionPattern) || []; | ||
| const mentionedHandles = new Set(mentionMatches.map((m: string) => m.slice(1))); |
There was a problem hiding this comment.
The regex /@(\w+)/g matches any @ followed by word characters, which will incorrectly match email addresses (e.g., user@example.com will match @example as a mention). We should use a more robust pattern that requires a word boundary or space before the @ symbol.
const mentionPattern = /(?:^|\s)@(\w+)/g;
const mentionedHandles = new Set<string>();
let match;
while ((match = mentionPattern.exec(data.text)) !== null) {
mentionedHandles.add(match[1]);
}| const isoTimestamp = z.string().refine((value) => !Number.isNaN(Date.parse(value)), { | ||
| message: 'expires_at must be an ISO-8601 timestamp', | ||
| }); |
There was a problem hiding this comment.
Using Date.parse inside a custom Zod .refine can be permissive and parse non-standard date formats. Since Zod has a built-in .datetime() validator that strictly enforces ISO-8601 UTC format, we should use it instead for better validation and cleaner code.
const isoTimestamp = z.string().datetime({ message: 'expires_at must be an ISO-8601 timestamp' });![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b32d86adc2
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!observerAllowsChannel(getObserverTokenFromContext(c), channel)) { | ||
| return jsonNotFound(c, 'channel_not_found', `Channel "${channelName}" not found`); |
There was a problem hiding this comment.
When an observer token is created with filters.agent_ids, this route only verifies the channel and then returns the full messageEngine.getMessages result, so a token scoped to one sender can read messages from every agent in an allowed channel via REST. The workspace stream already applies observerAllowsAgent, so channel message reads need the same per-message agent_id filtering before responding.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 3c115b0: channel message reads now filter each returned message through observerAllowsMessage, which enforces agent_ids and created_after. Search/console/activity filtering also uses the same helper. Added conformance coverage for agent_ids on channel message reads and search.
| reactionRoutes.get( | ||
| '/messages/:id/reactions', | ||
| requireAuth, | ||
| requireWorkspaceRead(['messages:read', 'reactions:read']), |
There was a problem hiding this comment.
requireWorkspaceRead grants access when an observer has any listed scope, so this allows a token with only messages:read to call /v1/messages/:id/reactions and receive reaction metadata; the later resource check only applies channel/DM filters. Since reactions:read is a distinct observer scope, this endpoint should require that scope rather than treating messages:read as sufficient.
Useful? React with 👍 / 👎.
| ...(filters?.dm_conversation_ids?.length ? { dm_conversation_ids: [...new Set(filters.dm_conversation_ids)] } : {}), | ||
| ...(filters?.agent_ids?.length ? { agent_ids: [...new Set(filters.agent_ids)] } : {}), | ||
| ...(filters?.event_types?.length ? { event_types: [...new Set(filters.event_types)] } : {}), | ||
| ...(filters?.created_after ? { created_after: filters.created_after } : {}), |
There was a problem hiding this comment.
filters.created_after is accepted and persisted, but no observer filter path compares it against message or event timestamps, so a token minted with a cutoff can still read older REST/search results and receive events before that cutoff. Either reject this filter until it is implemented or apply it wherever observer-visible data is returned.
Useful? React with 👍 / 👎.
|
Addressed the review in d33cff1.
Verification:
|
|
Addressed the DM-channel bypass in 284b6c8.
Verification:
|
|
Addressed this review batch in c0cd16c.
Verification:
|
|
Addressed the latest observer review in 38e9455.
Verification:
|
|
Addressed this review batch in 3eb1bcf.
Verification:
|
|
Addressed this batch in What changed:
On Verification:
|
Delete generated relay workspace metadata (memory/workspace/.relay/outbox/capabilities.json and memory/workspace/.relay/state.json) and remove the large SDK setup workflow script (workflows/sdk-setup-client-80-100.ts). These are cleanup deletions of workspace state/artifact files and an obsolete workflow; no functional source code changes were made.
* Split node kind and role for delivery adapters * Address node delivery review feedback * Make realtime delivery node-only * [codex] Add scoped observer tokens (#215) * Add scoped observer tokens * Add observer token SDK parity coverage * Address observer token review feedback * Tighten observer token scope filtering * Block observer access to DM channels * Align observer file and stream filters * Close observer stream DM thread gaps * Filter observer node and channel results * Make observer DM gating authoritative * Remove relay workspace state and SDK workflow Delete generated relay workspace metadata (memory/workspace/.relay/outbox/capabilities.json and memory/workspace/.relay/state.json) and remove the large SDK setup workflow script (workflows/sdk-setup-client-80-100.ts). These are cleanup deletions of workspace state/artifact files and an obsolete workflow; no functional source code changes were made. * Align observer auth contracts * Clean up migration leftovers and React races * Remove dead dashboard auth and feed code * Validate channel message attachments * Wire API usage quotas * Fix node delivery review blockers * Add replay demo script and trajectory records Introduce a replay utility and add multiple AgentWorkforce trajectory artifacts. Adds .agentworkforce/replay-demo.py to rebuild the Relay Rush demo workspace (teardown, re-register agents, replay messages/reactions, checkpoint state). Also adds numerous trajectory JSONs, traces and summary files under .agentworkforce/trajectories (active and completed for 2026-06) to record work history and traces. Minor .gitignore update included. * Address PR #214 review feedback Node delivery / capacity: - node.ts: recompute delivery adapter + maxAgents when node shape changes; make post-node.register pending flush best-effort to avoid double error reply - routes/node.ts: return node_not_found (not 200 []) for observer-hidden nodes - realtime.ts: drop dead no-op agent-socket flush; revalidate observer tokens at publish time (5s TTL) so revoked/narrowed tokens stop receiving - fanout.ts: add message.read and message.reacted to node-delivery skip-set to kill duplicate context frames Release flow: - action.ts: guard release double-decrement, clear agent node location on release, stop release retries from falling through to generic placement - mcp test: fix release fixture input shape (name vs agent_name) Auth / observer scoping: - auth/index.ts: reject node tokens on user/workspace routes (node tokens authenticate only via node transport) - search.ts: allowNode:false - observerToken.ts: skip channel filter for workspace-wide events so agent_ids can match; still fail closed for channel-scoped events missing metadata - channel.ts: filter member lists by observerAllowsAgent - routes/console.ts: classify conversation logs via channel_type, not both DM types - workspace.ts + dmAll.ts + types/workspace.ts: plumb agent_id into DM preview and filter previews via observerAllowsMessage Other correctness: - dm.ts: validate attachments before creating conversation rows - ChatFeed.tsx: clear stale messages on conversation switch; fix stale pagination - sdk-rust ws.rs: surface auto-ack send failures instead of dropping them - sdk-typescript ws.ts: dedupe node deliver frames via seenEventIds - sdk-python ws.py: stop _handle_node_frame swallowing reply/error/pong/resync_ack Docs: - openapi.yaml: /v1/ws is query-token only (no Authorization header) - README.md: align /node/ws -> /v1/node/ws Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Centralize auth token kind validation --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Summary
ot_live_*observer tokens with storage, lifecycle routes, scope checks, and REST/stream filteringstream:readobserver tokens for/v1/wswhile keeping agents on node delivery and nodes on/v1/node/wsValidation
npm --workspace @relaycast/engine test -- nodeUpgradeAuth.test.ts conformance/node.test.ts conformance/nodeDeliveryContracts.test.tsnpm --workspace @relaycast/types test -- sdk-openapi-sync.test.tsnpx turbo build && npx turbo test && npx turbo lintnpx turbo lint.venv/bin/python -m pytest testsinpackages/sdk-pythoncargo test --manifest-path packages/sdk-rust/Cargo.tomlswift test --package-path packages/sdk-swiftStacked on #214. Refs #213.