fix(inference-profile): self-review gap fixes by credence-the-bot[bot] · Pull Request #29957 · vellum-ai/vellum-assistant

credence-the-bot · 2026-05-09T00:05:14Z

Summary

Fix test mismatch: update no-ttl test to assert ttlSeconds: 1800 (default)
Fix Zod schema: accept null for ttlSeconds in open route
Add parseDuration tests for partial-parse rejection inputs
Use config defaultTtlSeconds if present, else 1800 hardcoded fallback

Part of plan: inference-profile-cli.md (fix PR)

…istry (#29956) * feat(inference-profile): CLI open/close/list + shared cli/utils + registry * fix: address review feedback (help text, null safety, llm alias comment) - Replace "daemon" with "assistant" in profile open help text (user-facing) - Fix sticky branch condition: use `== null` to catch ttlSeconds null/undefined - Remove unsafe non-null assertion on resultTtlSeconds in non-sticky branch - Add null guard on expiresAt in list --json output - Update JSDoc and llm command description/help to accurately reflect that llm only aliases inference send, not all subcommands (profile is inference-only) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(inference-profile-cli): correct help text TTL default and guard clamp notice in JSON mode - Update `profile open` help text: omitting --ttl yields a sticky session (no expiry), not a "configured default TTL" which does not exist in the daemon handler. - Update --ttl option description to mention that omitting it also gives a sticky session. - Move the TTL-clamped notice into the non-JSON (else) branch so that `--json` output is always valid JSON with no non-JSON prefix. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(inference-profile-cli): default TTL to 30m and reject partial duration parses - `profile open` without `--ttl` now sends `ttlSeconds: 1800` (30 min default) instead of omitting it, so the daemon creates a session-backed override that `profile close` can properly undo (sticky overrides with no sessionId cannot be closed by the session handler). - `parseDuration` now validates that the entire input string is composed of `<digits><unit>` groups before parsing, rejecting inputs like "1hxyz" or "1h-30m" that would previously silently return partial matches. - Updated `--ttl` option description and help text to reflect the new default. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: credence-the-bot <credence@vellum.ai> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

…tion coverage Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…able change

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a2c5667147

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-05-09T00:09:04Z

+          process.stdout.write(
+            JSON.stringify({
+              ok: true,
+              conversationId,


Return resolved conversationId in JSON responses

In profile open --json, the command emits the locally resolved input conversationId instead of the daemon response field (result.conversationId). This loses canonicalization when callers pass a conversation key/alias rather than an internal ID: the operation applies to the resolved conversation on the server, but the JSON output reports a different identifier, which can break automation that chains IDs from CLI output. The same pattern is repeated in profile close --json.

Useful? React with 👍 / 👎.

…tests Replace loadConfig() with getConfigReadOnly() in the profile open action handler to avoid filesystem side effects (ensureMigratedDataDir, default config writes) during CLI reads. Add a hermetic mock.module for ../../config/loader.js in the test file so the no-TTL test case does not depend on local config state. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

devin-ai-integration

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

credence-the-bot · 2026-05-09T00:15:36Z

@codex review this PR

credence-the-bot · 2026-05-09T00:15:37Z

@devin review this PR

In the `profile open --json` and `profile close --json` handlers, the JSON output was using the locally-resolved `conversationId` variable instead of `result.conversationId` (the server's canonical conversation ID). When callers pass a conversation key/alias rather than an internal ID, the server canonicalizes it — the JSON output now returns what the server resolved. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

credence-the-bot · 2026-05-09T00:18:34Z

@codex review this PR again — the previous issues have been fixed in commit d6e9319

credence-the-bot · 2026-05-09T00:18:35Z

@devin review this PR again — the previous issues have been fixed in commit d6e9319

credence-the-bot

Review (cycle 3)

Looks good — would approve if I could. Ready for Noa's merge.

What I verified

Default TTL: getConfigReadOnly().llm?.profileSession?.defaultTtlSeconds ?? 1800. Closes the sticky-trap (default open creates a session-backed override that close can undo).
Codex P2 (conversationId canonicalization): both open and close JSON paths now emit result.conversationId from the daemon response, not local input.
--ttl never plumbing: Zod schema in inference-profile-session-routes.ts accepts null, OpenAPI regenerated to match.
parseDuration strictness: anchored regex ^(\d+[hms])+$ rejects partial parses like 1hxyz / 1h-30m. Test coverage added.
Shared utils refactor: clean lift of resolveConversationId + parseDuration into cli/utils/. browser.ts correctly skipped (different return semantics).
Trust rule: inference profile open|close|list registered with risk: "low". Auto-approves at Relaxed (threshold ≥ low), prompts at Strict (threshold = none).
Test mismatch + isolation fixes from self-review remediation.

Cycle history

Cycle 1 (sub-PR #29956): Devin findings (help text daemon→assistant, null safety, llm alias, list null guard) + Codex P2 (clamp/JSON purity, sticky-trap on default) addressed across 9d25911c, f339abc9, 68f821d0.
Cycle 2 (PR #29957 open): self-review remediation (test mismatch, Zod null, parseDuration coverage, getConfigReadOnly, test mocking) addressed across a2c5667, fcb6dff, 7e6164d3.
Cycle 3 (current HEAD d6e9319e2): Codex P2 conversationId canonicalization addressed; CI 9/9 green; all prior findings resolved.

Ready to merge.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d6e9319e28

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-05-09T00:25:10Z

+  }),
+}));
+
+mock.module("../../config/loader.js", () => ({


Mock the correct config module path

mock.module("../../config/loader.js", ...) resolves relative to this test file, so it targets assistant/src/cli/config/loader.js instead of the assistant/src/config/loader.js module imported by inference-profile.ts. As a result, these tests silently use the real config loader and can become environment-dependent (for example if llm.profileSession.defaultTtlSeconds differs from 1800 in the active workspace), reducing test reliability.

Useful? React with 👍 / 👎.

Codex P3: `mock.module("../../config/loader.js", ...)` resolves to `assistant/src/cli/config/loader.js` from this test's location, not `assistant/src/config/loader.js` which is what `inference-profile.ts` actually imports. The mock therefore wasn't taking effect and tests silently used the real config loader. Fix: change to `../../../` (one more level up) to match the canonical pattern used by the other mocks in this same file.

credence-the-bot · 2026-05-09T00:27:53Z

@codexbot review again — Codex P3 (config loader mock path) addressed in commit f74fca7ce: mock.module("../../config/loader.js", ...) was resolving to src/cli/config/loader.js (non-existent) from the test's location. Changed to ../../../config/loader.js to match the other mocks in this file and resolve to src/config/loader.js — what inference-profile.ts actually imports. Mock now takes effect; tests are properly hermetic.

chatgpt-codex-connector · 2026-05-09T00:31:59Z

Codex Review: Didn't find any major issues. More of your lovely PRs please.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

credence-the-bot Bot and others added 3 commits May 8, 2026 23:51

fix(inference-profile-cli): test mismatch, Zod null schema, parseDura…

a2c5667

…tion coverage Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

fix(inference-profile): regenerate OpenAPI spec after ttlSeconds null…

fcb6dff

…able change

chatgpt-codex-connector Bot reviewed May 9, 2026

View reviewed changes

devin-ai-integration Bot reviewed May 9, 2026

View reviewed changes

credence-the-bot Bot commented May 9, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed May 9, 2026

View reviewed changes

noanflaherty merged commit 0953336 into main May 9, 2026
14 checks passed

noanflaherty deleted the credence-the-bot/inference-profile-cli branch May 9, 2026 02:48

This was referenced May 9, 2026

feat(host-browser): accept target_client_id LLM param to override interface preference #30066

Merged

refactor(cli): rename inference profile namespace to inference session #30145

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(inference-profile): self-review gap fixes#29957

fix(inference-profile): self-review gap fixes#29957
noanflaherty merged 6 commits into
mainfrom
credence-the-bot/inference-profile-cli

credence-the-bot Bot commented May 9, 2026 •

edited by devin-ai-integration Bot

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 9, 2026

Uh oh!

devin-ai-integration Bot left a comment

Uh oh!

credence-the-bot Bot commented May 9, 2026

Uh oh!

credence-the-bot Bot commented May 9, 2026

Uh oh!

credence-the-bot Bot commented May 9, 2026

Uh oh!

credence-the-bot Bot commented May 9, 2026

Uh oh!

credence-the-bot Bot left a comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot May 9, 2026

Uh oh!

credence-the-bot Bot commented May 9, 2026

Uh oh!

chatgpt-codex-connector Bot commented May 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

credence-the-bot Bot commented May 9, 2026 • edited by devin-ai-integration Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 9, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

✅ Devin Review: No Issues Found

Uh oh!

credence-the-bot Bot commented May 9, 2026

Uh oh!

credence-the-bot Bot commented May 9, 2026

Uh oh!

credence-the-bot Bot commented May 9, 2026

Uh oh!

credence-the-bot Bot commented May 9, 2026

Uh oh!

credence-the-bot Bot left a comment

Choose a reason for hiding this comment

Review (cycle 3)

What I verified

Cycle history

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 9, 2026

Choose a reason for hiding this comment

Uh oh!

credence-the-bot Bot commented May 9, 2026

Uh oh!

chatgpt-codex-connector Bot commented May 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

credence-the-bot Bot commented May 9, 2026 •

edited by devin-ai-integration Bot

Loading