feat: add Mastra curator with Codex OAuth-only auth - #122
Closed
Coldaine wants to merge 12 commits into
Closed
Conversation
Add a real, sanitized Codex CLI rollout (3,677 lines, 4.5h homelab database-recovery coordinator session) as the deterministic replay corpus, with human-verified ground truth as assertable expectations, and the design for the Codex harness driver + deterministic replay runner (docs/plans/plan-codex-replay.md). Fixture provenance and sanitization (17 secrets -> REDACTED_*) are documented in the ground-truth file; both files are frozen. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U9EpjZNoWaXLzfG93tdvTU
Record the nuke-and-copy-agent-flow plan (req-v1-curator, visual substrate inventory) and point roadmap M2/M3 at them so the v0 snapshot preserves the restart rationale. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Nuke the classifier-era src/tests/docs and copy agent-flow web/ + extension ingestion (minus VS Code glue) as the v1 renderer substrate. Apache-2.0 LICENSE + NOTICE; fresh governing docs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Add flat root package.json, main/preload, and an Electron Vite SPA entry that reuses vscode-bridge over contextBridge IPC. Document copy-verify findings (prefer app-style SPA over webview IIFE; SSE host is reference-only). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Substrate slice has no host/ingestion package yet; stub test scripts so stacked PR pushes work with the existing pre-push hook. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
M2 prep for curator lookbacks: ring-buffered agent-flow events with query helpers, Claude/Codex drivers wrapping extension parsers, and an IPC-injectable ingestion adapter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Start HarnessDriver on renderer ready (Claude transcript or fixture), forward agent-event messages over the vscode-bridge IPC shape, and bundle host/ingestion into dist-electron via esbuild. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Recover typed exhibit components, fixture gallery, and stage CSS under web/components/exhibits; restore exhibit-prototype.jsx from the v0 pin. Additive ?mode=exhibits entry leaves AgentVisualizer as the default surface. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…M13) Read-only ObservationStore lookback tools, in-memory gallery session memory, single-flight runner, ChatGPT Pro Codex OAuth provider (device-code + browser scaffolding) with OPENAI_API_KEY fallback, and Electron timer/event-count trigger publishing exhibit artifacts over IPC. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
ChatGPT Pro Codex OAuth is the only live model auth. Spec and code no longer treat Platform API keys as a fallback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
🤖 CodeAnt AI — Review Status
|
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
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: ASSERTIVE 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 |
3 tasks
Comment on lines
+74
to
+76
| if (!json.access_token || !json.refresh_token) { | ||
| throw new Error('Codex token response missing access_token or refresh_token') | ||
| } |
There was a problem hiding this comment.
Suggestion: The refresh-token response type allows refresh_token to be omitted, but tokensFromResponse rejects any response without it. A successful refresh that does not rotate the refresh token will therefore fail and leave the existing access token unusable. Preserve the current refresh token when the refresh response omits one. [api mismatch]
Severity Level: Critical 🚨
- ❌ Expired Codex sessions cannot refresh without token rotation.
- ⚠️ Curator inference falls back to mock or reports authentication failure.
- ⚠️ Existing refresh credentials are discarded from the successful response path.Steps of Reproduction ✅
1. Store an expired OAuth token through `TokenStore.save()` at
`host/curator/src/auth/token-store.ts:70-87`, including its current refresh token.
2. Trigger the authenticated curator path from `CuratorRunner.runInvestigate()` at
`host/curator/src/runner.ts:109-121`; the provider calls `ensureFreshAccess()` through
`buildCodexOAuthFetch()` at `host/curator/src/auth/codex-provider.ts:58-60`.
3. Have `refreshCodexTokens()` at `host/curator/src/auth/codex-oauth.ts:91-105` receive a
successful response containing `access_token` but no rotated `refresh_token`, which is
permitted by `TokenResponseJson` at lines 56-60.
4. `tokensFromResponse()` at `host/curator/src/auth/codex-oauth.ts:73-88` rejects that
successful response at lines 74-76, so `ensureFreshAccess()` cannot save or return the new
access token and curator inference falls back or fails.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/curator/src/auth/codex-oauth.ts
**Line:** 74:76
**Comment:**
*Api Mismatch: The refresh-token response type allows `refresh_token` to be omitted, but `tokensFromResponse` rejects any response without it. A successful refresh that does not rotate the refresh token will therefore fail and leave the existing access token unusable. Preserve the current refresh token when the refresh response omits one.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+112
to
+124
| const current = store.load() | ||
| if (!current) { | ||
| throw new Error('Not logged in to ChatGPT Codex. Run OAuth login first.') | ||
| } | ||
| if (Date.now() < current.expires - 60_000) { | ||
| return current | ||
| } | ||
| const refreshed = await refreshCodexTokens(current.refresh) | ||
| // Preserve accountId if refresh response omits it. | ||
| if (!refreshed.accountId && current.accountId) { | ||
| refreshed.accountId = current.accountId | ||
| } | ||
| store.save(refreshed) |
There was a problem hiding this comment.
Suggestion: Concurrent requests can all load the same expiring token, refresh it independently, and then overwrite the store in an unpredictable order. If refresh tokens rotate, one request can invalidate the token used by another and persist a stale or unusable token. Deduplicate refreshes per TokenStore or serialize the refresh-and-save operation. [race condition]
Severity Level: Major ⚠️
- ❌ Concurrent Codex requests can invalidate shared refresh credentials.
- ⚠️ Subsequent curator requests may lose authenticated access.
- ⚠️ Electron’s shared token store has no refresh coordination.Steps of Reproduction ✅
1. Configure the Electron host with one shared `TokenStore`, as constructed at
`electron/curator-host.ts:54-68` and passed to `CuratorRunner` at lines 71-79.
2. Allow the stored access token to enter the refresh window checked by
`ensureFreshAccess()` at `host/curator/src/auth/codex-oauth.ts:116-119`.
3. Start two authenticated fetches through `buildCodexOAuthFetch()` at
`host/curator/src/auth/codex-provider.ts:58-60` before either refresh completes; both
calls independently execute `store.load()` and `refreshCodexTokens()` at
`codex-oauth.ts:112-119`.
4. If the provider rotates refresh tokens, each response calls `store.save()` at
`codex-oauth.ts:124`; whichever request saves last can overwrite the store with a stale or
already-invalid refresh credential. The runner currently prevents overlapping
investigations at `host/curator/src/runner.ts:73-87`, but the exported fetch and refresh
functions remain concurrently callable and the race is not serialized.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/curator/src/auth/codex-oauth.ts
**Line:** 112:124
**Comment:**
*Race Condition: Concurrent requests can all load the same expiring token, refresh it independently, and then overwrite the store in an unpredictable order. If refresh tokens rotate, one request can invalidate the token used by another and persist a stale or unusable token. Deduplicate refreshes per `TokenStore` or serialize the refresh-and-save operation.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+276
to
+280
| const error = url.searchParams.get('error') | ||
| const code = url.searchParams.get('code') | ||
| const returnedState = url.searchParams.get('state') | ||
|
|
||
| if (error) { |
There was a problem hiding this comment.
Suggestion: The callback rejects the pending login whenever an error parameter is present before validating the OAuth state. Any local process able to reach the localhost callback can therefore terminate an in-progress login with a forged error response. Validate the returned state before accepting either success or error callbacks. [security]
Severity Level: Major ⚠️
- ❌ Forged localhost callbacks can abort browser authentication.
- ⚠️ Users may receive false login-failure results.
- ⚠️ OAuth state validation is bypassed for error responses.Steps of Reproduction ✅
1. Start `startBrowserLogin()` at `host/curator/src/auth/codex-oauth.ts:230-250`; it
generates the expected state at lines 236-237 and begins waiting on the localhost server
through `waitForCallback()` at lines 255-268.
2. Before the identity provider returns, send a localhost request to
`/auth/callback?error=access_denied` without the generated state, or with an arbitrary
state.
3. The callback parses `error`, `code`, and `returnedState` at lines 276-278, but checks
`error` first at line 280.
4. The handler rejects the pending login at lines 284-287 without comparing
`returnedState` to `state`; a local process able to reach the callback can terminate the
legitimate login flow.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/curator/src/auth/codex-oauth.ts
**Line:** 276:280
**Comment:**
*Security: The callback rejects the pending login whenever an `error` parameter is present before validating the OAuth state. Any local process able to reach the localhost callback can therefore terminate an in-progress login with a forged error response. Validate the returned state before accepting either success or error callbacks.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix|
|
||
| if (error) { | ||
| res.writeHead(200, { 'Content-Type': 'text/html' }) | ||
| res.end(`<html><body><h1>Login failed</h1><p>${error}</p></body></html>`) |
There was a problem hiding this comment.
Suggestion: The OAuth error value is inserted directly into an HTML response. A crafted callback request can inject markup or script into the browser page displaying the login result. Escape the value or return a non-HTML response. [security]
Severity Level: Major ⚠️
- ❌ Local OAuth callback page can render attacker-controlled markup.
- ⚠️ Login error display is vulnerable to callback-page XSS.
- ⚠️ Electron browser login scaffolding exposes a localhost HTML response.Steps of Reproduction ✅
1. Start browser login through `startBrowserLogin()` at
`host/curator/src/auth/codex-oauth.ts:230-250`; it creates a localhost callback server at
lines 268-345.
2. Send a request to the actual callback path `/auth/callback` with an `error` query value
containing HTML markup or script, for example an encoded `<img>` payload.
3. The handler reads the attacker-controlled value at lines 276-277 and interpolates it
directly into the HTML response at line 282.
4. The browser rendering the localhost callback displays the injected markup before the
login promise is rejected at lines 284-287.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/curator/src/auth/codex-oauth.ts
**Line:** 282:282
**Comment:**
*Security: The OAuth error value is inserted directly into an HTML response. A crafted callback request can inject markup or script into the browser page displaying the login result. Escape the value or return a non-HTML response.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+363
to
+365
| cancel: () => { | ||
| server?.close() | ||
| }, |
There was a problem hiding this comment.
Suggestion: cancel only closes the server; it does not settle the promise returned by waitForCallback or clear its timeout. Callers awaiting the promise will remain blocked until the five-minute timeout, and the timer retains the login flow unnecessarily. Cancellation must reject or otherwise settle the pending promise and clear the timer. [missing cleanup]
Severity Level: Major ⚠️
- ❌ Browser login cancellation does not notify awaiting callers.
- ⚠️ UI cancellation remains pending for up to five minutes.
- ⚠️ Timeout resources remain active after cancellation.Steps of Reproduction ✅
1. Call `startBrowserLogin()` at `host/curator/src/auth/codex-oauth.ts:230-250`, then
invoke the returned `waitForCallback()` function, which creates the pending promise and
five-minute timeout at lines 255-266.
2. Invoke the returned `cancel()` function from the object created at lines 357-366.
3. `cancel()` only calls `server?.close()` at lines 363-365; it does not set `settled`,
clear the timeout, or reject the promise.
4. The caller awaiting `waitForCallback()` remains blocked until the timeout callback at
lines 258-265 rejects after five minutes, retaining the login flow unnecessarily.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/curator/src/auth/codex-oauth.ts
**Line:** 363:365
**Comment:**
*Missing Cleanup: `cancel` only closes the server; it does not settle the promise returned by `waitForCallback` or clear its timeout. Callers awaiting the promise will remain blocked until the five-minute timeout, and the timer retains the login flow unnecessarily. Cancellation must reject or otherwise settle the pending promise and clear the timer.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+89
to
+93
| const parsed = | ||
| url instanceof URL ? url : new URL(typeof url === 'string' ? url : (url as Request).url) | ||
| const shouldRewrite = | ||
| parsed.pathname.includes('/v1/responses') || parsed.pathname.includes('/chat/completions') | ||
| const finalUrl = shouldRewrite ? new URL(CODEX_API_ENDPOINT) : parsed |
There was a problem hiding this comment.
Suggestion: This function is exposed as a fetch implementation and accepts a Request, but when the input is a Request it copies only headers from init and then calls fetch with init rather than the original request's method, body, or headers. A caller passing a Request therefore sends an incomplete request, and matching paths may also be rewritten incorrectly. Preserve the Request properties or restrict the wrapper contract to URL plus init arguments. [api mismatch]
Severity Level: Major ⚠️
- ❌ Request-based model calls can lose POST bodies.
- ⚠️ Authorization and content headers may be discarded.
- ⚠️ Codex endpoint rewriting can receive incomplete requests.Steps of Reproduction ✅
1. Create the OAuth fetch implementation through `buildCodexOAuthFetch()` at
`host/curator/src/auth/codex-provider.ts:58-60`; this wrapper is passed to
`createOpenAI()` at lines 170-173.
2. Invoke the returned fetch with a `Request` object containing a POST method, body, and
headers, rather than a URL plus `init`.
3. The wrapper extracts only the URL at lines 89-90 and builds a new `Headers` object from
`init` at lines 62-80; it does not copy the original `Request` method, body, or headers.
4. It calls native `fetch(finalUrl, { ...init, headers, body })` at line 108. With no
`init`, the original request becomes a GET with no body, and response-path requests are
rewritten based only on the URL at lines 91-93.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/curator/src/auth/codex-provider.ts
**Line:** 89:93
**Comment:**
*Api Mismatch: This function is exposed as a `fetch` implementation and accepts a `Request`, but when the input is a `Request` it copies only headers from `init` and then calls `fetch` with `init` rather than the original request's method, body, or headers. A caller passing a `Request` therefore sends an incomplete request, and matching paths may also be rewritten incorrectly. Preserve the Request properties or restrict the wrapper contract to URL plus init arguments.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+90
to
+99
| if (this.canEncrypt() && this.safeStorage && fs.existsSync(this.encPath())) { | ||
| try { | ||
| const buf = fs.readFileSync(this.encPath()) | ||
| const json = this.safeStorage.decryptString(buf) | ||
| return parseTokens(json) | ||
| } catch { | ||
| return null | ||
| } | ||
| } | ||
| if (!fs.existsSync(this.plainPath())) return null |
There was a problem hiding this comment.
Suggestion: The encrypted file takes precedence whenever encryption becomes available, even if a newer plaintext file was written during a fallback period. If safe-storage availability changes or an encrypted write is interrupted, load can return stale credentials from codex-oauth.enc and ignore newer tokens in codex-oauth.json. Use an atomic single-file replacement strategy or reconcile and remove stale representations. [stale reference]
Severity Level: Major ⚠️
- ❌ Auth recovery can select stale Codex credentials.
- ⚠️ Curator startup may ignore newer fallback tokens.
- ⚠️ Storage-mode transitions can cause unnecessary reauthentication.Steps of Reproduction ✅
1. Construct a `TokenStore` with the Electron safe-storage bridge used by
`electron/curator-host.ts:56-67`, while encryption is initially available, and save
credentials; `TokenStore.save()` writes `codex-oauth.enc` at
`host/curator/src/auth/token-store.ts:70-83`.
2. Run during a period when `canEncrypt()` returns false, such as safe-storage
initialization failure, and save newer credentials; the fallback path writes
`codex-oauth.json` at lines 85-86 but does not remove the existing encrypted file.
3. Restore encryption availability and call `load()` or `hasOAuth()` from
`token-store.ts:89-123`.
4. `load()` unconditionally reads and returns the encrypted file first at lines 90-95, so
the newer plaintext credentials at lines 99-104 are ignored and the curator may refresh
using stale credentials.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/curator/src/auth/token-store.ts
**Line:** 90:99
**Comment:**
*Stale Reference: The encrypted file takes precedence whenever encryption becomes available, even if a newer plaintext file was written during a fallback period. If safe-storage availability changes or an encrypted write is interrupted, `load` can return stale credentials from `codex-oauth.enc` and ignore newer tokens in `codex-oauth.json`. Use an atomic single-file replacement strategy or reconcile and remove stale representations.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+37
to
+38
| this.byId.set(merged.id, merged) | ||
| out.push(merged) |
There was a problem hiding this comment.
Suggestion: byId treats artifact IDs as unique, but upsertMany appends every input item to out even when multiple artifacts have the same ID. The map retains only the last artifact while the returned result contains duplicate entries, so callers can publish conflicting duplicate artifacts and renderers can receive duplicate React keys. Reject duplicate IDs or deduplicate the returned artifacts consistently with the map. [logic error]
Severity Level: Major ⚠️
- ⚠️ Curator output can contain conflicting duplicate exhibits.
- ⚠️ Exhibit rail receives duplicate React keys.
- ⚠️ Gallery results disagree with stored artifact state.Steps of Reproduction ✅
1. A curator response containing two artifacts with the same `id` reaches
`CuratorRunner.runInvestigate()` at `host/curator/src/runner.ts:130-142`.
2. `GalleryMemory.upsertMany()` processes both entries at
`host/curator/src/gallery-memory.ts:21-39`; the second `this.byId.set()` overwrites the
first map entry, but both merged artifacts are appended to `out`.
3. `CuratorRunner` returns the duplicate-containing `upserted` array at
`host/curator/src/runner.ts:142-151`.
4. `CuratorHost.fire()` publishes that array through
`host/curator/electron/curator-host.ts:128-150` to consumers such as `ExhibitStage`, whose
rail uses `key={artifact.id}` at `web/components/exhibits/ExhibitStage.tsx:154-158`,
producing duplicate React keys and conflicting displayed entries.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/curator/src/gallery-memory.ts
**Line:** 37:38
**Comment:**
*Logic Error: `byId` treats artifact IDs as unique, but `upsertMany` appends every input item to `out` even when multiple artifacts have the same ID. The map retains only the last artifact while the returned result contains duplicate entries, so callers can publish conflicting duplicate artifacts and renderers can receive duplicate React keys. Reject duplicate IDs or deduplicate the returned artifacts consistently with the map.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| refreshedAtEvent: z.number().optional(), | ||
| status: z.enum(['fresh', 'active', 'stale', 'retired']).optional(), | ||
| retirementReason: z.string().optional(), | ||
| payload: z.record(z.string(), z.unknown()).optional(), |
There was a problem hiding this comment.
Suggestion: The parser accepts any object as payload and then casts it to the discriminated payload type without runtime validation. A model response such as a momentum artifact with {} or a missing payload passes parsing, but ExhibitStage forwards it to components that immediately destructure required fields such as stats, next, or curation, causing rendering failures. Validate payload according to exhibitType or reject the artifact before it reaches the renderer. [type error]
Severity Level: Critical 🚨
- ❌ Malformed momentum exhibits can break rendering.
- ❌ Other exhibit components receive unchecked payload shapes.
- ⚠️ Curator IPC can publish unusable gallery artifacts.Steps of Reproduction ✅
1. A model response containing a valid envelope such as a `momentum` artifact with no
payload or with `{}` reaches `parseExhibitArtifacts()` through
`CuratorRunner.runInvestigate()` at `host/curator/src/runner.ts:130-132`.
2. The optional record schema at `host/curator/src/parse-artifacts.ts:7-20` accepts the
missing or empty payload, and line 72 replaces a missing payload with `{}` before casting
it to the discriminated payload type.
3. The parsed artifact is upserted and returned by `host/curator/src/runner.ts:142-151`,
then published by `electron/curator-host.ts:128-150`.
4. When supplied to `ExhibitStage`, the `momentum` discriminator selects `Momentum` at
`web/components/exhibits/ExhibitStage.tsx:72-79`; `Momentum.tsx:18-19` immediately
destructures `value`, `label`, `stats`, `next`, and `curation`, so missing fields can
cause rendering failures or invalid output.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/curator/src/parse-artifacts.ts
**Line:** 19:19
**Comment:**
*Type Error: The parser accepts any object as `payload` and then casts it to the discriminated payload type without runtime validation. A model response such as a `momentum` artifact with `{}` or a missing payload passes parsing, but `ExhibitStage` forwards it to components that immediately destructure required fields such as `stats`, `next`, or `curation`, causing rendering failures. Validate payload according to `exhibitType` or reject the artifact before it reaches the renderer.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+58
to
+59
| const parsed = artifactSchema.safeParse(item) | ||
| if (!parsed.success) continue |
There was a problem hiding this comment.
Suggestion: Invalid artifacts are silently discarded while valid artifacts in the same response are retained. This makes a partially malformed model response look successful and can silently publish an incomplete gallery instead of triggering the runner's fallback or surfacing a parse failure. Reject the whole response, or return an explicit validation error when any artifact fails schema validation. [incomplete implementation]
Severity Level: Major ⚠️
- ⚠️ Gallery updates can silently omit requested exhibits.
- ⚠️ Parse fallback is bypassed for partial failures.
- ⚠️ Curator output quality becomes difficult to diagnose.Steps of Reproduction ✅
1. A curator model returns an array containing at least one valid artifact and one
artifact violating `artifactSchema` at `host/curator/src/parse-artifacts.ts:7-20`.
2. `parseExhibitArtifacts()` calls `safeParse()` for each item at
`host/curator/src/parse-artifacts.ts:56-59`; the invalid item is skipped while the valid
item remains in `out`.
3. Because no exception is thrown, `CuratorRunner.runInvestigate()` does not enter its
parse-error fallback at `host/curator/src/runner.ts:130-136`.
4. The partial result is returned and published at `host/curator/src/runner.ts:142-151`
and `electron/curator-host.ts:128-150`, making the incomplete gallery appear successful
without exposing the malformed response.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/curator/src/parse-artifacts.ts
**Line:** 58:59
**Comment:**
*Incomplete Implementation: Invalid artifacts are silently discarded while valid artifacts in the same response are retained. This makes a partially malformed model response look successful and can silently publish an incomplete gallery instead of triggering the runner's fallback or surfacing a parse failure. Reject the whole response, or return an explicit validation error when any artifact fails schema validation.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| - Narratives are mandatory: every artifact needs a meaningful \`narrative\` (why it belongs on the floor), not a raw dump. | ||
| - Ground claims in queried history; cite observation \`id\` values in narratives/payloads where applicable. | ||
| - Prefer a small, structurally distinct gallery (2–5 exhibits) over stuffing every type. | ||
| - You may refresh, retire, or update prior gallery artifacts via session memory tools. |
There was a problem hiding this comment.
Suggestion: The prompt instructs the agent that it can refresh, retire, or update prior artifacts through session memory tools, but the registered curator tools expose only list_prior_artifacts; the mutation methods on GalleryMemory are never available to the agent. The model therefore cannot perform the advertised retirement or refresh workflow, and omitted or obsolete artifacts remain in memory unless another path mutates them. [incomplete implementation]
Severity Level: Major ⚠️
- ⚠️ Curator cannot retire obsolete gallery artifacts.
- ⚠️ Curator cannot mark prior exhibits stale.
- ⚠️ Gallery memory accumulates artifacts omitted from later responses.Steps of Reproduction ✅
1. `createCuratorAgent()` loads `CURATOR_SYSTEM_PROMPT` and creates tools through
`createCuratorTools()` at `host/curator/src/agent.ts:20-29`.
2. The prompt advertises refresh, retirement, and updates at
`host/curator/src/prompts.ts:17`, but `createCuratorTools()` registers only five tools at
`host/curator/src/tools.ts:97-103`.
3. The only gallery tool exposed to the agent is `list_prior_artifacts` at
`host/curator/src/tools.ts:71-95`; no tool calls `GalleryMemory.retire()` or
`GalleryMemory.markStale()`.
4. Subsequent runs only call `gallery.upsertMany()` through
`host/curator/src/runner.ts:142`; artifacts omitted from a model response are therefore
neither retired nor marked stale, so the advertised curation workflow cannot be performed
by the agent.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/curator/src/prompts.ts
**Line:** 17:17
**Comment:**
*Incomplete Implementation: The prompt instructs the agent that it can refresh, retire, or update prior artifacts through session memory tools, but the registered curator tools expose only `list_prior_artifacts`; the mutation methods on `GalleryMemory` are never available to the agent. The model therefore cannot perform the advertised retirement or refresh workflow, and omitted or obsolete artifacts remain in memory unless another path mutates them.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+97
to
+103
| const poll = setInterval(() => { | ||
| const size = this.query.size | ||
| if (size - this.lastSize >= this.eventCountThreshold) { | ||
| this.lastSize = size | ||
| void this.fire('event-count') | ||
| } | ||
| }, 2_000) |
There was a problem hiding this comment.
Suggestion: The event-count trigger compares the ring-buffer length rather than a monotonic event count. Once ObservationStore reaches capacity, query.size remains constant while new observations replace old ones, so this condition never becomes true again and event-count curation stops permanently. Track the latest observation sequence or subscribe to append events instead. [incorrect condition logic]
Severity Level: Major ⚠️
- ❌ Event-count curation stops after ring-buffer capacity is reached.
- ⚠️ Long-running sessions rely only on timer triggers afterward.
- ⚠️ Newly ingested observations are not independently detected.Steps of Reproduction ✅
1. Start the Electron application through `electron/main.ts:70-73`; the renderer-ready
path constructs and starts `CuratorHost` at `electron/main.ts:46-61`.
2. `CuratorHost.start()` initializes `lastSize` from `query.size` at
`electron/curator-host.ts:82-86`, then polls that value every two seconds at
`electron/curator-host.ts:95-105`.
3. The backing `ObservationStore` exposes `size` as the current ring-buffer length at
`host/ingestion/src/observation-store.ts:60-62`, while `append()` removes the oldest row
whenever capacity is exceeded at `host/ingestion/src/observation-store.ts:79-85`.
4. Ingest more than the store capacity through `IngestionAdapter.push()` at
`host/ingestion/src/ingestion-adapter.ts:71-81`. Once the buffer is full, new observations
replace old ones and `query.size - lastSize` remains zero, so
`electron/curator-host.ts:99-102` never fires the event-count investigation again.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** electron/curator-host.ts
**Line:** 97:103
**Comment:**
*Incorrect Condition Logic: The event-count trigger compares the ring-buffer length rather than a monotonic event count. Once `ObservationStore` reaches capacity, `query.size` remains constant while new observations replace old ones, so this condition never becomes true again and event-count curation stops permanently. Track the latest observation sequence or subscribe to append events instead.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+47
to
62
| void ingestion | ||
| ?.onRendererReady() | ||
| .then(() => { | ||
| if (!ingestion) return | ||
| curator?.dispose() | ||
| curator = new CuratorHost({ | ||
| query: ingestion.query, | ||
| send: sendToRenderer, | ||
| sendExhibits: (message) => { | ||
| mainWindow?.webContents.send(IPC.EXHIBIT_ARTIFACTS, message) | ||
| }, | ||
| }) | ||
| curator.start() | ||
| // First curation shortly after events are available. | ||
| void curator.kick('renderer-ready') | ||
| }) |
There was a problem hiding this comment.
Suggestion: Each renderer ready message starts another asynchronous onRendererReady() call, which clears and restarts the shared ingestion store. Without a generation check or cancellation, an earlier completion can install a curator after a later restart, and its immediate kick() can publish stale artifacts after the curator has been replaced. Serialize readiness handling or discard completions from obsolete readiness requests, and await or handle the kick promise. [race condition]
Severity Level: Major ⚠️
- ❌ Stale curator results can reach the active renderer.
- ⚠️ Renderer reloads can restart shared ingestion concurrently.
- ⚠️ Gallery artifacts may reflect an obsolete session state.Steps of Reproduction ✅
1. Send two `{ type: 'ready' }` messages to the Electron renderer-message handler at
`electron/main.ts:42-46` before the first asynchronous `onRendererReady()` call completes.
2. Each message independently calls `IngestionHost.onRendererReady()` at
`electron/main.ts:47-48`; that method synchronously stops drivers and clears the shared
store at `electron/ingestion-host.ts:141-144`, then awaits `startPlanned()` at
`electron/ingestion-host.ts:172`.
3. The asynchronous starts can overlap because `startPlanned()` awaits driver startup at
`electron/ingestion-host.ts:206-224`, while `ClaudeCodeDriver.start()` replaces its tail
at `host/ingestion/src/drivers/claude-code.ts:100-121`.
4. If the older readiness promise completes after the newer request has begun, its
continuation at `electron/main.ts:49-61` installs a curator and calls `kick()`; the newer
continuation can then dispose and replace it, but the older curator's in-flight runner can
still publish through `electron/curator-host.ts:128-148`.
5. A renderer reload or repeated bridge initialization is a realistic trigger: the normal
Electron bridge sends `ready` at `web/electron-entry.tsx:17-21` via
`web/lib/electron-bridge.ts:41-47`, while the main handler has no generation,
serialization, or cancellation check.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** electron/main.ts
**Line:** 47:62
**Comment:**
*Race Condition: Each renderer `ready` message starts another asynchronous `onRendererReady()` call, which clears and restarts the shared ingestion store. Without a generation check or cancellation, an earlier completion can install a curator after a later restart, and its immediate `kick()` can publish stale artifacts after the curator has been replaced. Serialize readiness handling or discard completions from obsolete readiness requests, and await or handle the kick promise.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fixThere was a problem hiding this comment.
Pull request overview
Adds a new host/curator package that runs a Mastra “curator” agent in the Electron main process, producing typed Exhibit artifacts from ObservationStore lookbacks. The implementation intentionally supports Codex OAuth only for live inference, with a deterministic offline mock fallback when OAuth tokens are absent.
Changes:
- Introduces
@agentvisualcrazy/curator(tools, prompts, runner, gallery memory, artifact parsing, and Codex OAuth auth utilities) plus Vitest coverage. - Wires a new
CuratorHostinto Electron main to trigger curation on timers/event-count and publish exhibit artifacts over IPC. - Updates roadmap/requirements docs to explicitly forbid any
OPENAI_API_KEY/ Platform API auth path.
Reviewed changes
Copilot reviewed 29 out of 31 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds curator-related dependencies and a test:curator script hooked into root npm test. |
| host/curator/vitest.config.ts | Adds Vitest configuration for the curator package. |
| host/curator/tsconfig.json | Adds TypeScript config for the curator package. |
| host/curator/tests/tools-memory.test.ts | Tests read-only tool registration and gallery memory behavior. |
| host/curator/tests/runner-auth.test.ts | Tests OAuth-mode resolution, token store, Codex fetch rewrite, JWT claim extraction, PKCE, and runner single-flight/mock behavior. |
| host/curator/src/types.ts | Defines host-side Exhibit artifact contracts aligned with the web exhibit types. |
| host/curator/src/tools.ts | Implements read-only ObservationQuery + gallery memory tools (and forbids mutation tools). |
| host/curator/src/runner.ts | Implements the single-flight curator runner with mock fallback and artifact upsert into gallery memory. |
| host/curator/src/prompts.ts | Adds curator system prompt and hard read-only constraints. |
| host/curator/src/parse-artifacts.ts | Parses model output into artifacts (tolerates markdown fences; validates envelope fields). |
| host/curator/src/observation.ts | Defines the ObservationQuery/CuratorObservation contract and summary helper for evidence-safe tool output. |
| host/curator/src/mock-model.ts | Adds deterministic offline/mock artifact generation. |
| host/curator/src/index.ts | Exports the curator public API surface. |
| host/curator/src/gallery-memory.ts | Adds in-memory gallery memory for refresh/retire/list behavior across triggers. |
| host/curator/src/auth/token-store.ts | Adds OAuth token persistence with safeStorage support and plaintext fallback. |
| host/curator/src/auth/jwt.ts | Adds JWT decode + account id extraction helpers for ChatGPT account headers. |
| host/curator/src/auth/codex-provider.ts | Implements Codex OAuth-only AI SDK model wiring via fetch rewrite + middleware. |
| host/curator/src/auth/codex-oauth.ts | Implements device-code flow and scaffolds browser PKCE login via localhost callback. |
| host/curator/src/agent.ts | Adds Mastra Agent factory configured with read-only tools and system prompt. |
| host/curator/README.md | Documents curator layout, auth modes (OAuth-only), Electron wiring, and tests. |
| host/curator/package.json | Defines curator package scripts and dependencies. |
| electron/tsconfig.json | Includes curator source in Electron typechecking. |
| electron/preload.ts | Exposes a dedicated onExhibits renderer subscription for exhibit artifacts IPC. |
| electron/main.ts | Instantiates/starts CuratorHost after ingestion becomes ready; adds a curator-run trigger. |
| electron/ipc-channels.ts | Adds EXHIBIT_ARTIFACTS IPC channel constant. |
| electron/curator-host.ts | New Electron main-side CuratorHost for timed/event-threshold triggers and artifact publishing. |
| electron/build.mjs | Marks npm packages as external during Electron bundling (requires runtime deps in root node_modules). |
| docs/plans/roadmap.md | Clarifies M4 exit criteria as OAuth-only (no API key path). |
| docs/plans/req-v1-curator.md | Updates requirements to forbid OPENAI_API_KEY fallback and reiterates OAuth-only auth. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+70
to
+87
| save(tokens: CodexOAuthTokens): void { | ||
| this.ensureDir() | ||
| const json = JSON.stringify(tokens, null, 2) | ||
| if (this.canEncrypt() && this.safeStorage) { | ||
| const buf = this.safeStorage.encryptString(json) | ||
| fs.writeFileSync(this.encPath(), buf) | ||
| // Remove plaintext if we upgraded to encrypted storage. | ||
| if (fs.existsSync(this.plainPath())) { | ||
| try { | ||
| fs.unlinkSync(this.plainPath()) | ||
| } catch { | ||
| /* ignore */ | ||
| } | ||
| } | ||
| return | ||
| } | ||
| fs.writeFileSync(this.plainPath(), json, 'utf8') | ||
| } |
Comment on lines
+363
to
+365
| cancel: () => { | ||
| server?.close() | ||
| }, |
There was a problem hiding this comment.
The curator pipeline currently produces artifacts that no renderer path consumes, so its primary visible behavior is absent. Please wire the live artifact stream into the exhibit UI before merging.
The existing CI run passed; this review is based on read-only inspection of the current head and its integration paths.
| mainWindow?.webContents.send(IPC.EXHIBIT_ARTIFACTS, message) | ||
| }, | ||
| }) | ||
| curator.start() |
There was a problem hiding this comment.
This only publishes the result; nothing on the renderer side consumes it. web/electron-entry.tsx skips configureElectronBridge() when ?mode=exhibits, and ExhibitGalleryApp always passes fixtureGallery rather than listening to agentVisual.onMessage/onExhibits. In the default mode, the bridge forwards this message but AgentVisualizer has no exhibit-artifacts handler. Consequently a real curator run can complete and send IPC while the UI never renders its artifacts. Wire a single live artifact state path through the bridge and replace the fixture for the Electron exhibit surface.
| } | ||
|
|
||
| private async runInvestigate(reason: string): Promise<CuratorInvestigateResult> { | ||
| const eventCursor = this.query.size |
There was a problem hiding this comment.
Suggestion: The runner uses the ring-buffer length as the event cursor. Once the ObservationStore reaches capacity, query.size remains constant while new observations continue receiving larger sequence numbers, so newly refreshed artifacts keep stale or duplicate createdAtEvent/refreshedAtEvent values and the prompt reports no progress. Use the latest observation sequence or another monotonic ingestion cursor instead of the retained item count. [logic error]
Severity Level: Major ⚠️
- ⚠️ Curator progress metadata stops advancing after buffer capacity.
- ⚠️ Refreshed gallery artifacts receive stale event cursors.
- ⚠️ OAuth prompts report inaccurate observation-store progress.Steps of Reproduction ✅
1. Start ingestion through the ObservationStore implementation at
`host/ingestion/src/observation-store.ts:49`, which uses a bounded buffer and assigns each
appended observation a monotonic `seq` at lines 65-70.
2. Append more observations than the store capacity, causing the retained buffer length
exposed as `ObservationQuery.size` to remain capped while newly appended observations
continue receiving larger sequence numbers.
3. Trigger the curator through `CuratorHost.fire()` at `electron/curator-host.ts:128`,
which calls `CuratorRunner.trigger()` and reaches `runInvestigate()` at
`host/curator/src/runner.ts:90`.
4. `runInvestigate()` records `eventCursor` from `query.size` at
`host/curator/src/runner.ts:91`; after capacity is reached, repeated investigations
therefore use the same cursor even though `recent_events` exposes observations with newer
`seq` values.
5. `GalleryMemory.upsertMany()` at `host/curator/src/gallery-memory.ts:21-30` stores that
cursor as `refreshedAtEvent`, while `buildPrompt()` at
`host/curator/src/runner.ts:155-162` reports it as the observation cursor, producing stale
progress metadata and duplicate cursor values.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/curator/src/runner.ts
**Line:** 91:91
**Comment:**
*Logic Error: The runner uses the ring-buffer length as the event cursor. Once the ObservationStore reaches capacity, `query.size` remains constant while new observations continue receiving larger sequence numbers, so newly refreshed artifacts keep stale or duplicate `createdAtEvent`/`refreshedAtEvent` values and the prompt reports no progress. Use the latest observation sequence or another monotonic ingestion cursor instead of the retained item count.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+122
to
+127
| } catch (err) { | ||
| // No credentials → deterministic offline gallery. | ||
| console.warn('[curator] model unavailable, using mock:', err) | ||
| mode = 'mock' | ||
| text = mockInvestigateText(eventCursor) | ||
| } |
There was a problem hiding this comment.
Suggestion: This catch converts every live-model failure—including expired OAuth credentials, token refresh failures, provider incompatibility, and network errors—into a successful-looking mock investigation with mode = 'mock'. Callers cannot distinguish “no credentials, use offline mode” from “authenticated curation failed,” and the UI may display stale deterministic artifacts instead of surfacing the live failure. Restrict fallback to the explicitly credential-less case and propagate authenticated provider errors. [possible bug]
Severity Level: Critical 🚨
- ❌ OAuth failures are reported as successful mock curation.
- ⚠️ Electron UI cannot distinguish stale fallback artifacts.
- ⚠️ Token refresh and network failures lose diagnostics.Steps of Reproduction ✅
1. Start `CuratorHost` at `electron/curator-host.ts:82` with OAuth tokens present,
allowing its constructor at lines 70-79 to create a `CuratorRunner` configured for OAuth.
2. A timer or event-count trigger invokes `CuratorHost.fire()` at
`electron/curator-host.ts:128`, which calls `CuratorRunner.trigger()` and then
`runInvestigate()` at `host/curator/src/runner.ts:90`.
3. `createCuratorModel()` at `host/curator/src/auth/codex-provider.ts:143-179` selects
OAuth when `tokenStore.hasOAuth()` is true; the generated agent then calls the Codex
provider through `generateWithAgent()` at `host/curator/src/runner.ts:165-179`.
4. During generation, an expired or revoked token causes `ensureFreshAccess()` at
`host/curator/src/auth/codex-oauth.ts:111-125` to throw, while a refresh failure is
explicitly thrown at lines 101-105; network and provider failures similarly reject the
agent generation promise.
5. The catch at `host/curator/src/runner.ts:122-127` catches all of those failures,
changes the result mode to `mock`, generates deterministic artifacts, and returns a
successful result.
6. `CuratorHost.fire()` at `electron/curator-host.ts:129-135` publishes those fallback
artifacts, so the UI receives apparently valid mock exhibits instead of an authentication
or provider failure.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/curator/src/runner.ts
**Line:** 122:127
**Comment:**
*Possible Bug: This catch converts every live-model failure—including expired OAuth credentials, token refresh failures, provider incompatibility, and network errors—into a successful-looking mock investigation with `mode = 'mock'`. Callers cannot distinguish “no credentials, use offline mode” from “authenticated curation failed,” and the UI may display stale deterministic artifacts instead of surfacing the live failure. Restrict fallback to the explicitly credential-less case and propagate authenticated provider errors.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+97
to
+103
| return { | ||
| recentEvents, | ||
| eventsByType, | ||
| searchTranscript, | ||
| getObservation, | ||
| listPriorArtifacts, | ||
| } |
There was a problem hiding this comment.
Suggestion: The curator instructions require refreshing, retiring, and updating prior gallery artifacts through session memory tools, but this tool set only exposes list_prior_artifacts; no tool can perform those operations. The runner only upserts artifacts emitted in the current response, so omitted stale artifacts remain active indefinitely unless the model manually emits the same artifact with a retired status. Add the required gallery mutation tools or remove that workflow requirement. [incomplete implementation]
Severity Level: Major ⚠️
- ⚠️ Obsolete gallery artifacts remain active indefinitely.
- ⚠️ Curator cannot execute its documented retirement workflow.
- ⚠️ Electron exhibit publishing can include stale exhibits.Steps of Reproduction ✅
1. Construct the production curator through `createCuratorAgent()` at
`host/curator/src/agent.ts:20-29`; it registers the object returned by
`createCuratorTools()` as the agent tool set.
2. The system prompt at `host/curator/src/prompts.ts:12-17` tells the model it may
refresh, retire, or update prior gallery artifacts, and its workflow at lines 43-47
requires deciding those actions.
3. The returned tool object at `host/curator/src/tools.ts:97-103` exposes only lookback
tools and `list_prior_artifacts`; that tool only calls `gallery.list()` at lines 71-95.
4. `GalleryMemory` does have `retire()` and `markStale()` methods at
`host/curator/src/gallery-memory.ts:43-60`, but neither is reachable from the agent tools.
5. After generation, `CuratorRunner.runInvestigate()` at
`host/curator/src/runner.ts:130-151` only parses the current response and calls
`gallery.upsertMany()`. It does not retire or mark stale artifacts omitted from that
response.
6. Therefore, when a later authenticated investigation omits an obsolete artifact, it
remains active in `GalleryMemory.list()` at `host/curator/src/gallery-memory.ts:10-14`
unless the model manually emits that artifact again with `status: retired`.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** host/curator/src/tools.ts
**Line:** 97:103
**Comment:**
*Incomplete Implementation: The curator instructions require refreshing, retiring, and updating prior gallery artifacts through session memory tools, but this tool set only exposes `list_prior_artifacts`; no tool can perform those operations. The runner only upserts artifacts emitted in the current response, so omitted stale artifacts remain active indefinitely unless the model manually emits the same artifact with a retired status. Add the required gallery mutation tools or remove that workflow requirement.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
This was referenced Jul 27, 2026
- Fix PKCE invariant test in runner-auth.test.ts to assert
challenge === base64url(sha256(verifier)) instead of just length.
- Add auth-hardening.test.ts covering:
- PKCE invariant (challenge derivation, base64url charset, freshness)
- refreshCodexTokens: POSTs to auth.openai.com/oauth/token with
refresh_token grant; throws on non-OK
- buildCodexOAuthFetch: rewrites /v1/responses and /chat/completions
to the Codex backend endpoint, injects Authorization /
ChatGPT-Account-ID / originator / OpenAI-Beta headers, sets
store:false on JSON bodies, and leaves unrelated URLs untouched
- TokenStore negative cases: corrupted JSON, missing fields,
wrong type, clear()
- ensureFreshAccess: returns current token when fresh, refreshes
and persists when expired (preserving accountId), throws when
not logged in
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Address High/Medium findings from the PR #122 review: - token-store.ts: restrict file fallback to mode 0o600 and config dir to 0o700 (was default 0644/0755, world-readable on POSIX). Re-tighten existing files on save/load via chmod. - codex-oauth.ts (startBrowserLogin): HTML-escape the `error` query param before reflecting it in the callback error page (was a reflected XSS). Add Content-Security-Policy: default-src 'none' + X-Content-Type-Options: nosniff + charset to all callback responses. Reject non-GET methods with 405. - codex-oauth.ts (ensureFreshAccess): memoize the in-flight refresh promise per refresh_token so concurrent fetches within the refresh window await the same refresh instead of racing the (potentially single-use/rotated) refresh_token. Tests added to auth-hardening.test.ts: - Strips a caller-supplied Authorization header and injects the OAuth bearer (the security-critical invariant of buildCodexOAuthFetch). - Deletes max_output_tokens from the JSON body on rewrite. - Dedups concurrent refreshes against the same refresh_token (asserts refresh fires exactly once for three overlapping callers). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Coldaine
force-pushed
the
feat/v1-exhibits
branch
from
July 27, 2026 12:01
7e3ab0a to
1104423
Compare
Coldaine
added a commit
that referenced
this pull request
Jul 27, 2026
Address High/Medium findings from the PR #122 review: - token-store.ts: restrict file fallback to mode 0o600 and config dir to 0o700 (was default 0644/0755, world-readable on POSIX). Re-tighten existing files on save/load via chmod. - codex-oauth.ts (startBrowserLogin): HTML-escape the `error` query param before reflecting it in the callback error page (was a reflected XSS). Add Content-Security-Policy: default-src 'none' + X-Content-Type-Options: nosniff + charset to all callback responses. Reject non-GET methods with 405. - codex-oauth.ts (ensureFreshAccess): memoize the in-flight refresh promise per refresh_token so concurrent fetches within the refresh window await the same refresh instead of racing the (potentially single-use/rotated) refresh_token. Tests added to auth-hardening.test.ts: - Strips a caller-supplied Authorization header and injects the OAuth bearer (the security-critical invariant of buildCodexOAuthFetch). - Deletes max_output_tokens from the JSON body on rewrite. - Dedups concurrent refreshes against the same refresh_token (asserts refresh fires exactly once for three overlapping callers). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
3 tasks
Coldaine
added a commit
that referenced
this pull request
Jul 27, 2026
* feat(curator): add Mastra curator agent with Codex OAuth (M3-M7, M10-M13)
Read-only ObservationStore lookback tools, in-memory gallery session memory, single-flight runner, ChatGPT Pro Codex OAuth provider (device-code + browser scaffolding) with OPENAI_API_KEY fallback, and Electron timer/event-count trigger publishing exhibit artifacts over IPC.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(curator): remove OPENAI_API_KEY auth path
ChatGPT Pro Codex OAuth is the only live model auth. Spec and code no
longer treat Platform API keys as a fallback.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* test(curator): harden Codex OAuth auth path
- Fix PKCE invariant test in runner-auth.test.ts to assert
challenge === base64url(sha256(verifier)) instead of just length.
- Add auth-hardening.test.ts covering:
- PKCE invariant (challenge derivation, base64url charset, freshness)
- refreshCodexTokens: POSTs to auth.openai.com/oauth/token with
refresh_token grant; throws on non-OK
- buildCodexOAuthFetch: rewrites /v1/responses and /chat/completions
to the Codex backend endpoint, injects Authorization /
ChatGPT-Account-ID / originator / OpenAI-Beta headers, sets
store:false on JSON bodies, and leaves unrelated URLs untouched
- TokenStore negative cases: corrupted JSON, missing fields,
wrong type, clear()
- ensureFreshAccess: returns current token when fresh, refreshes
and persists when expired (preserving accountId), throws when
not logged in
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* fix(curator): harden Codex OAuth auth path per review
Address High/Medium findings from the PR #122 review:
- token-store.ts: restrict file fallback to mode 0o600 and config dir to
0o700 (was default 0644/0755, world-readable on POSIX). Re-tighten
existing files on save/load via chmod.
- codex-oauth.ts (startBrowserLogin): HTML-escape the `error` query param
before reflecting it in the callback error page (was a reflected XSS).
Add Content-Security-Policy: default-src 'none' + X-Content-Type-Options:
nosniff + charset to all callback responses. Reject non-GET methods
with 405.
- codex-oauth.ts (ensureFreshAccess): memoize the in-flight refresh
promise per refresh_token so concurrent fetches within the refresh
window await the same refresh instead of racing the (potentially
single-use/rotated) refresh_token.
Tests added to auth-hardening.test.ts:
- Strips a caller-supplied Authorization header and injects the OAuth
bearer (the security-critical invariant of buildCodexOAuthFetch).
- Deletes max_output_tokens from the JSON body on rewrite.
- Dedups concurrent refreshes against the same refresh_token (asserts
refresh fires exactly once for three overlapping callers).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
Summary
host/curatorMastra agent with read-only ObservationStore lookback tools, gallery memory, single-flight runnerelectron/curator-host.tstimer/event triggers publishing exhibit artifacts over IPC~/.agentvisualcrazy/OPENAI_API_KEY/ Platform API auth path (oauth | mock only)Honest status
shell.openExternal) still TODOTest plan
npm test(ingestion + curator)OPENAI_API_KEYdoes not unlock live auth (resolveAuthModestays mock)npm run buildCodeAnt-AI Description
Add OAuth-only session curation and publish exhibit artifacts in Electron
What Changed
OPENAI_API_KEYno longer enables inference.Impact
✅ Read-only session exhibits published to the gallery✅ OAuth tokens refreshed without repeated login✅ No accidental Platform API billing from OPENAI_API_KEY💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.