feat(cli): registerCommand helper + transport tags for 21 commands + §3.7 daemon-down message by credence-the-bot[bot] · Pull Request #30156 · vellum-ai/vellum-assistant

credence-the-bot · 2026-05-09T20:44:04Z

Summary

Add registerCommand helper at assistant/src/cli/lib/register-command.ts with transport tag mechanism (ipc/local/bootstrap)
Tag 15 already-thin IPC commands and 6 local commands with their transport class
Fix daemon-down message in cli-client.ts to match §3.7 canonical format
Add §3.7 daemon-down fallback to status command (stat socket, print running/down, exit 0)
Move cache-fs.ts utility from commands/ to lib/ (not a command, should not be subject to lint rule)

Part of plan: cli-foundation-tags-lint.md (PR 1 of 3)

…§3.7 daemon-down message

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c14e3cfd69

ℹ️ 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-09T20:46:09Z

+        if (!result.ok || !result.result) {
+          const socketPath = getAssistantSocketPath();
+          const socketExists = existsSync(socketPath);
+          const workspace = getWorkspaceDirDisplay();
+          if (socketExists) {
+            log.info("Daemon: running");
+          } else {
+            log.info("Daemon: down");
+          }
+          log.info(`Workspace: ${workspace}`);
+          process.exit(0);


Preserve health-call failures in status command

This branch treats every failed cliIpcCall("health") as a daemon-down fallback and then exits with code 0, which masks real runtime errors when the assistant is reachable but the health RPC itself fails (e.g., handler exceptions or protocol-level errors). In those cases, assistant status now reports success (Daemon: running) and suppresses the actual error text, so automation and operators can get a false healthy signal instead of a failure.

Useful? React with 👍 / 👎.

…t.ts

devin-ai-integration

Devin Review found 5 potential issues.

View 2 additional findings in Devin Review.

devin-ai-integration · 2026-05-09T20:48:11Z

🚩 mcp.ts string comparison now matches the updated error message format

At assistant/src/cli/commands/mcp.ts:557, there's a pre-existing startsWith("Could not connect to assistant daemon") check that was NOT matching the old cli-client.ts error messages (which said "Could not connect to the assistant"). With this PR changing cli-client.ts error messages to use "Could not connect to assistant daemon", this startsWith check now actually matches. So while the AGENTS.md terminology violation is a bug, there's a silver lining: the mcp.ts error detection path now works as originally intended. If the terminology fix reverts these messages back to "the assistant", the mcp.ts startsWith check at line 557 should also be updated.

(Refers to line 557)

Was this helpful? React with 👍 or 👎 to provide feedback.

devin-ai-integration · 2026-05-09T20:48:12Z

+            log.info("Daemon: running");
+          } else {
+            log.info("Daemon: down");


🔴 User-facing CLI output uses "Daemon" instead of "Assistant", violating AGENTS.md terminology rule

The status command prints "Daemon: running" and "Daemon: down" in its CLI output (lines 36 and 38). These are user-facing strings — they appear directly in terminal output when a user runs assistant status. AGENTS.md mandates: "In all user-facing text — CLI output, error messages, help strings…use 'assistant' instead of 'daemon'." The labels should say "Assistant: running" / "Assistant: down" instead. The new test at assistant/src/cli/commands/__tests__/status.test.ts:168 and :178 also hardcode the incorrect wording and would need to be updated alongside the fix.

Suggested change

log.info("Daemon: running");

} else {

log.info("Daemon: down");

log.info("Assistant: running");

} else {

log.info("Assistant: down");

Was this helpful? React with 👍 or 👎 to provide feedback.

devin-ai-integration · 2026-05-09T20:48:13Z

      finish({
        ok: false,
-        error: `Could not connect to the assistant at ${socketPath}.\nRun \`assistant status\` to check, or \`assistant gateway start\` to start it.`,
+        error: `Could not connect to assistant daemon at ${socketPath}.\nRun \`assistant status\` to check, or \`assistant gateway start\` to start the daemon.`,


🔴 User-facing error messages changed from "the assistant" to "assistant daemon", violating AGENTS.md terminology rule

All 9 error messages in cli-client.ts were changed from the correct "Could not connect to the assistant at …start it" to "Could not connect to assistant daemon at …start the daemon". These error strings are shown to users when CLI commands fail to connect to the IPC socket. AGENTS.md mandates: "In all user-facing text — CLI output, error messages…use 'assistant' instead of 'daemon'." The old wording was correct; this PR introduces a regression. Affected lines: 102, 119, 130, 230, 243, 253, 370, 384, 398.

Example of correct vs incorrect wording

Old (correct): Could not connect to the assistant at /path/to/assistant.sock.

New (incorrect): Could not connect to assistant daemon at /path/to/assistant.sock.

Prompt for agents

All 9 error message strings in cli-client.ts that were changed from "Could not connect to the assistant at ...start it" to "Could not connect to assistant daemon at ...start the daemon" need to be reverted to use the original user-facing wording (without the word "daemon"). The affected lines are 102, 119, 130, 230, 243, 253, 370, 384, and 398 in assistant/src/ipc/cli-client.ts. Each instance uses the same template string, so the fix is mechanical: replace "assistant daemon" with "the assistant" and "start the daemon" with "start it" (or similar phrasing that avoids the word "daemon"). This is required by the AGENTS.md rule on user-facing terminology.

Was this helpful? React with 👍 or 👎 to provide feedback.

devin-ai-integration · 2026-05-09T20:48:15Z

+export type CommandTransport = "ipc" | "local" | "bootstrap";
+
+interface RegisterCommandOpts {
+  name: string;
+  transport: CommandTransport;
+  description: string;
+  build: (cmd: Command) => void;


🚩 registerCommand transport field is unused metadata — no runtime effect

The registerCommand helper at assistant/src/cli/lib/register-command.ts:3-9 accepts a transport field of type CommandTransport = 'ipc' | 'local' | 'bootstrap', but this value is never stored, attached to the command instance, or used in any logic. It's purely a documentation/metadata annotation. This is fine if the intent is to tag commands for future tooling (e.g., auto-generating docs, filtering commands by transport type), but currently it has zero runtime impact. Worth confirming this is intentional and that there's a planned consumer for this metadata.

Was this helpful? React with 👍 or 👎 to provide feedback.

devin-ai-integration · 2026-05-09T20:48:16Z

+            log.info("Daemon: down");
+          }
+          log.info(`Workspace: ${workspace}`);
+          process.exit(0);


🚩 Behavioral change: status command now exits 0 (not 1) when daemon is unreachable

The old status command called process.exit(1) when it couldn't connect to the IPC socket. The new implementation calls process.exit(0) and prints diagnostic info (socket presence, workspace path). This is an intentional design change — status is an informational command and reporting "down" is a valid successful output, not an error. The test at assistant/src/cli/commands/__tests__/status.test.ts:152-158 explicitly asserts exit code 0. This is a reasonable UX improvement (scripts checking daemon liveness can parse the output rather than relying on exit code), but callers that previously relied on assistant status returning non-zero when the daemon is down will break.

Was this helpful? React with 👍 or 👎 to provide feedback.

… §3.7 message

…user-facing messages; 'Assistant: running/down' in status

…COMMAND_INVENTORY (#30159) * feat(cli): registerCommand helper + transport tags for 21 commands + §3.7 daemon-down message (#30156) * feat(cli): registerCommand helper + transport tags for 21 commands + §3.7 daemon-down message * fix(lint): sort imports in cache.ts, clients.ts, register-command.test.ts * fix(tests): use console.log in status.ts; update exit-helper test for §3.7 message * fix(tests): use process.stdout.write in status.ts for testable output * fix: revert daemon terminology per AGENTS.md; use 'the assistant' in user-facing messages; 'Assistant: running/down' in status --------- Co-authored-by: credence-the-bot[bot] <test@test.com> * feat(cli): advisory ESLint rule cli/no-daemon-internals (PR #30157) * feat(cli): advisory ESLint rule cli/no-daemon-internals * fix(lint): convert ESLint rule to plain JS so Node.js ESM loader can find it Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: credence-the-bot[bot] <test@test.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(cli): COMMAND_INVENTORY.md + bidirectional CI check + AGENTS.md canonical example (PR #30158) * feat(cli): COMMAND_INVENTORY.md + bidirectional CI check + AGENTS.md canonical example Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(lint): sort named imports alphabetically in check-cli-inventory.ts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(lint): correct named import sort order in check-cli-inventory.ts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: credence-the-bot[bot] <test@test.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(cli): address Codex + Devin findings on Section B rollup Cycle 1 fixes for PR #30159 review feedback: 1. Allowlist gap (Codex P2 / Devin 🟡): Add `../logger` and `../output` to `local` and `bootstrap` allowlists in `cli-no-daemon-internals.js`. Without this, every newly tagged `local` command (autonomy, config, completions, keys, credential-execution) false-positives on its standard logger/output imports, killing the rule's signal-to-noise ratio. 2. Doc reference (Devin 🟡): `forbiddenImport` error message pointed to nonexistent `DESIGN.md §3.1`. Now points to `src/cli/AGENTS.md` which is the actual shipped reference. 3. Target glob too broad (Codex P2): The rule fired `missingTransport` on every file under `src/cli/commands/**/*.ts` including helper modules like `oauth/shared.ts` that aren't command registrars. Narrowed by making the rule skip files that don't call `registerCommand` directly (now a tri-state `findTransport`: `string` for tagged, `MISSING_TRANSPORT` for registrars without transport prop, `null` for non-registrars). Helper modules pass silently. Backdoor closed because helpers can't be imported via blessed paths anyway (sibling `./helper.js` doesn't match any allowlist prefix). 4. Type-only imports (extends Codex P2 #1): `import type {...}` is erased at compile time and doesn't constitute a runtime boundary violation. Rule now skips type-only imports. Drops 3 false-positives on `memory-v2.ts` and `mcp.ts` type imports from runtime/routes/. 5. Canonical-example claim (Devin 🟡): `AGENTS.md` claimed `pending.ts` uses `exitFromIpcResult` but it didn't. Migrated `pending.ts` to actually use the helper added in Section A. Now matches the doc. 6. mcp.ts misclassification (Devin 🚩): `mcp.ts` was marked `THIN` in COMMAND_INVENTORY but imports `mcp/client.js`, `mcp/mcp-oauth-provider.js`, `util/browser.js` — actual daemon internals. Reclassified to `LEGACY` with a note that Section C/D will thin it. Until then the advisory rule will warn on this file (and that's the right signal). Lint: 18 warnings remain (down from 21) on partially-migrated commands (config, keys, credential-execution, status, task, mcp). All are legitimate signals — those files have non-allowlisted imports that need migration before Section J flips the rule from advisory → error. Tests: rule test suite extended with helper-module + type-only + local-with-logger regression cases; 5/5 manual smoke tests pass; tsc clean; lint:inventory passes. * fix(eslint-rule): cycle-1 review findings — chained registerCommand + inline-type imports Codex P2 (cli-no-daemon-internals.js:88) — traverse callees: The AST walk only enqueued CallExpression.arguments, never the callee. Chained patterns like `registerCommand({...}).command(...).description(...)` were treated as if no registerCommand call existed (findTransport returned null), silently skipping all checks for those command-entry files. Fix: enqueue node.callee on CallExpression and node.object on MemberExpression so the walk reaches through the receiver chain. Codex P2 (cli-no-daemon-internals.js:183) — inline type-only specifiers: The rule only checked importNode.importKind === "type", which catches `import type {...}` but not `import { type X }` — for the inline form the declaration's importKind stays "value" and each ImportSpecifier carries its own importKind. All-type-only inline imports are erased at compile time and must not flag forbidden-import. Fix: also skip when every specifier is an ImportSpecifier with importKind === "type". Side-effect-only imports (no specifiers) and mixed value+type imports still get linted. * fix(cli): status — narrow zero-exit fallback to daemon-unreachable cases (cycle-3) --------- Co-authored-by: credence-the-bot[bot] <277301654+credence-the-bot[bot]@users.noreply.github.com> Co-authored-by: credence-the-bot[bot] <test@test.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: credence-the-bot <195577519+credence-the-bot@users.noreply.github.com> Co-authored-by: credence-the-bot <226213748+credence-the-bot@users.noreply.github.com>

feat(cli): registerCommand helper + transport tags for 21 commands + …

c14e3cf

…§3.7 daemon-down message

chatgpt-codex-connector Bot reviewed May 9, 2026

View reviewed changes

fix(lint): sort imports in cache.ts, clients.ts, register-command.tes…

6fe30db

…t.ts

devin-ai-integration Bot reviewed May 9, 2026

View reviewed changes

credence-the-bot[bot] added 3 commits May 9, 2026 20:52

fix(tests): use console.log in status.ts; update exit-helper test for…

9916483

… §3.7 message

fix(tests): use process.stdout.write in status.ts for testable output

594fc57

fix: revert daemon terminology per AGENTS.md; use 'the assistant' in …

301bbca

…user-facing messages; 'Assistant: running/down' in status

credence-the-bot Bot merged commit a95e2a0 into credence/cli-foundation-tags-lint May 9, 2026
13 checks passed

credence-the-bot Bot deleted the run-plan/cli-found-tags/pr-1 branch May 9, 2026 21:10

credence-the-bot Bot mentioned this pull request May 9, 2026

feat(cli): Section B foundation — registerCommand tags, ESLint rule, COMMAND_INVENTORY #30159

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cli): registerCommand helper + transport tags for 21 commands + §3.7 daemon-down message#30156

feat(cli): registerCommand helper + transport tags for 21 commands + §3.7 daemon-down message#30156
credence-the-bot[bot] merged 5 commits into
credence/cli-foundation-tags-lintfrom
run-plan/cli-found-tags/pr-1

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!

devin-ai-integration Bot May 9, 2026

Uh oh!

devin-ai-integration Bot May 9, 2026

Uh oh!

devin-ai-integration Bot May 9, 2026

Uh oh!

devin-ai-integration Bot May 9, 2026

Uh oh!

devin-ai-integration Bot May 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

0 participants

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

Uh oh!

devin-ai-integration Bot May 9, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot May 9, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot May 9, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot May 9, 2026

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration Bot May 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

0 participants

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

edited by devin-ai-integration Bot

Loading