chore(cli): remove dead autonomy command#30261
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cec7d87200
ℹ️ 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".
| registerAuthCommand(program); | ||
| registerAutonomyCommand(program); | ||
| registerAvatarCommand(program); |
There was a problem hiding this comment.
Unregistering autonomy here removes assistant autonomy get/set from the CLI, but the built-in self-knowledge skill still tells the assistant to run assistant autonomy get (skills/vellum-self-knowledge/SKILL.md, line 43). That makes this documented introspection path fail at runtime with an unknown-command error whenever the skill is used, so the command removal should be paired with updating/removing that skill instruction (or providing a replacement).
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 Zsh completion still advertises removed autonomy command
The PR removed autonomy from the subcommands map and topLevel array (which drive bash and fish completions), but the generateZshCompletion function has a hardcoded commands array in its template string that was not updated. Line 133 still includes 'autonomy:View and configure autonomy tiers'. Zsh users will see autonomy offered as a valid tab-completion candidate, but running it will fail since the command no longer exists.
(Refers to line 133)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🟡 Fish completion descriptions map still contains removed autonomy entry
The descriptions map in generateFishCompletion at line 172 still has autonomy: "View and configure autonomy tiers". While this is functionally dead code (the topLevel array no longer includes autonomy, so the loop at line 178 never reaches this entry), it violates the repository's Dead Code Removal rule in AGENTS.md: "Proactively remove unused code during every change. Remove code your change makes unused, clean up adjacent dead code."
(Refers to line 172)
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🚩 Gateway command registry and test still reference removed autonomy command
The CLI AGENTS.md (assistant/src/cli/AGENTS.md:18) states: "When adding/removing/renaming assistant CLI commands or subcommands, update the gateway bash risk registry coverage in gateway/src/risk/command-registry/commands/assistant.ts." The gateway registry at gateway/src/risk/command-registry/commands/assistant.ts:21-23 still lists autonomy, autonomy get, and autonomy set as supported command paths, and line 367 has a risk override for autonomy set. The corresponding test at gateway/src/risk/command-registry.test.ts:543 also expects autonomy in the required top-level list. These are stale references to a command that no longer exists.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
🚩 SKILL.md still directs assistant to use assistant autonomy get
The vellum-self-knowledge skill at skills/vellum-self-knowledge/SKILL.md:43 documents assistant autonomy get as the command for checking autonomy tiers. Since this skill is used by the assistant to learn about its own capabilities, it will attempt to run a command that no longer exists. The CLI AGENTS.md deprecation hygiene rule requires: "Update references — search for string references to the old command in docs, skills, fixtures, help text, and comments." This reference should be removed or replaced.
Was this helpful? React with 👍 or 👎 to provide feedback.
Addresses Codex + Devin review findings on #30261: - assistant/src/cli/commands/completions.ts: drop `autonomy` from the hardcoded zsh `commands` array template (line 133) and from the fish `descriptions` map (line 172). The first leaked at runtime as a zsh tab-complete candidate for a dead command; the second was orphaned dead code. - gateway/src/risk/command-registry/commands/assistant.ts: remove `autonomy`, `autonomy get`, `autonomy set` from supported paths and the `autonomy set` medium-risk override. Per assistant/src/cli/AGENTS.md, the gateway bash risk registry must be updated whenever a CLI command is added/removed/renamed. - gateway/src/risk/command-registry.test.ts: drop `autonomy` from the requiredTopLevel expectation in 'covers expanded top-level assistant command groups'. - skills/vellum-self-knowledge/SKILL.md: remove the `assistant autonomy get` row from the introspection-commands table. The skill is loaded by the assistant to learn its own surface; pointing it at a removed command would fail at runtime. Verified: assistant typecheck clean, gateway typecheck clean, gateway command-registry test 50/50 pass, assistant + gateway lint clean.
|
Addressed all 5 review findings (2 Devin bugs in completions.ts, 1 Devin analysis on gateway registry, 1 Devin analysis + 1 Codex P2 on SKILL.md). New commit 6ba95e6. Verified locally: assistant typecheck/lint clean, gateway typecheck/lint clean, command-registry test 50/50 pass. |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
|
Pushed Note: the unrelated |
|
Opened #30267 to fix the pre-existing |
The `assistant autonomy` command reads and writes `autonomy-config.json` in the workspace dir, but **nothing in the codebase reads that file**. Last meaningful change was #21402 (2021). The "autonomy" concept that lives in the assistant today is `PlaybookAutonomyLevel` (auto/draft/notify per playbook in `playbooks/types.ts`) — a completely separate construct that does not consult `autonomy-config.json`. Removed: - `assistant/src/cli/commands/autonomy.ts` (371 lines) - registration in `cli/program.ts` - entry in shell-completion command list No other follow-ups required: no docs, scripts, or tests reference the command, and any pre-existing user `autonomy-config.json` files are inert and will simply remain on disk.
Addresses Codex + Devin review findings on #30261: - assistant/src/cli/commands/completions.ts: drop `autonomy` from the hardcoded zsh `commands` array template (line 133) and from the fish `descriptions` map (line 172). The first leaked at runtime as a zsh tab-complete candidate for a dead command; the second was orphaned dead code. - gateway/src/risk/command-registry/commands/assistant.ts: remove `autonomy`, `autonomy get`, `autonomy set` from supported paths and the `autonomy set` medium-risk override. Per assistant/src/cli/AGENTS.md, the gateway bash risk registry must be updated whenever a CLI command is added/removed/renamed. - gateway/src/risk/command-registry.test.ts: drop `autonomy` from the requiredTopLevel expectation in 'covers expanded top-level assistant command groups'. - skills/vellum-self-knowledge/SKILL.md: remove the `assistant autonomy get` row from the introspection-commands table. The skill is loaded by the assistant to learn its own surface; pointing it at a removed command would fail at runtime. Verified: assistant typecheck clean, gateway typecheck clean, gateway command-registry test 50/50 pass, assistant + gateway lint clean.
Bumps the vellum-self-knowledge skill's updatedAt timestamp to reflect removing the autonomy row from its SKILL.md. Restores CI's catalog freshness check to green.
credence-the-bot
Bot
force-pushed
the
credence/delete-autonomy-cli
branch
from
842cdd9 to
6c5a715
Compare
* refactor(cli): route `config` command through daemon IPC
The `config` CLI was the last general-purpose command still tagged
`transport: "local"`, reading and writing `config.json` directly from the
CLI process. That bypassed the daemon's in-memory config cache, file
watcher, embedding cache invalidation, and provider re-init - so every
`assistant config set ...` left the running daemon stale until a restart
or unrelated PATCH happened to flush the cache.
This change wires every subcommand through the existing daemon IPC
surface so the daemon stays coherent on every write and is the single
source of truth on every read.
What changed
- `cli/commands/config.ts`: re-tagged `transport: "ipc"`, dropped direct
`loadRawConfig` / `saveRawConfig` / `AssistantConfigSchema` /
`getSchemaAtPath` imports.
- `set`: builds a single-key deep-merge patch from the dotted path and
sends it to `config_patch`. Daemon does the merge + cache/provider
reinit (already implemented in `handlePatchConfig`).
- `get` / `list`: fetch the full raw config via `config_get` and walk
the dotted path in the CLI process. Pure read.
- `schema`: calls the new `config_schema_get` IPC route (see below).
- `validate-allowlist`: kept as-is - uses a dynamic `require()` to
load `security/secret-allowlist.js`, which has no daemon-globals
and only reads `secret-allowlist.json` from disk. Marked with a
TODO to add a `config_allowlist_validate` daemon route later so
the subcommand can become a pure thin wrapper too.
- `cli/lib/nested-value.ts` (new): thin re-export of
`getNestedValue`/`setNestedValue` from `config/loader.js`. These two
helpers are pure and dependency-free, but `cli/no-daemon-internals`
forbids `ipc` commands from importing `config/loader.js` directly
(it pulls in daemon-only globals via other exports). Helper modules
with no `registerCommand` call are exempt from the rule, so this is
the canonical bridge.
- `runtime/routes/conversation-query-routes.ts`: new
`handleGetConfigSchema` handler + `config_schema_get` route
(`GET config/schema?path=...`). Pure read; calls
`z.toJSONSchema(AssistantConfigSchema)` or `getSchemaAtPath` for the
scoped case, returns `{ schema }`. No daemon state touched.
- `runtime/auth/route-policy.ts`: new actor-endpoint entry
`config/schema:GET → settings.read`.
After this change the only commands still tagged non-`ipc` are:
- `transport: "local"`: `keys` (hybrid - uses daemon-credential-client
side-channel), `completions` (shell scripts), `credential-execution`
(CES RPC, different transport)
- `transport: "bootstrap"`: `default-action` (interactive entry, starts
the daemon)
`autonomy` is being removed in #30261.
* chore(openapi): regenerate spec for config_schema_get route
* fix(cli/config): preserve null + replace-on-set semantics, address review
Three issues caught in #30262 review, fixed together:
1. **Functional regression (Codex P1).** Routing `config set` through
`config_patch` went through `deepMergeOverwrite`, which strips `null`
leaves under newly-introduced subtrees and merges object subtrees
rather than replacing them. So `config set heartbeat.activeHoursStart
null` lost the null and `config set llm.profiles {}` no-op'd instead
of clearing. Add a separate `config_set` IPC route that calls
`setNestedValue` directly. Direct replacement, null preserved.
2. **User-facing text (Devin).** AGENTS.md mandates "assistant" not
"daemon" in help/error text shown to users. Rewrote the addHelpText
blocks accordingly. Internal docstrings/comments still say "daemon"
(matches the rest of the codebase's internal terminology).
3. **validate-allowlist under ipc tag (Devin).** Was doing a dynamic
`require("../../security/secret-allowlist.js")` from an IPC-tagged
command. Move it behind a new `config_allowlist_validate` route so
the CLI no longer reaches into daemon-internal modules.
Implementation notes:
- New routes `config_set` (POST /v1/config/set) and
`config_allowlist_validate` (GET /v1/config/allowlist/validate), both
in `conversation-query-routes.ts`. Both write paths share a new
`commitConfigWrite` helper (watcher suppress + saveRawConfig +
cache invalidation + provider re-init + fingerprint).
- Route-policy entries added: `config/set:POST → settings.write`,
`config/allowlist/validate:GET → settings.read`.
- `cli/lib/nested-value.ts` is now an inlined duplicate of the loader
helpers (not a bridge re-export). Documented in the file header. The
CLI must not import `config/loader.js` because loader has module-load
side effects (cache + watcher) that shouldn't run in the CLI process.
- Tests rewritten to mock `ipc/cli-client.js` instead of
`config/loader.js`. The platform-guard test now asserts on IPC call
shape; the schema CLI test verifies that a daemon 400 maps to exit
code 2 via `exitFromIpcResult`.
- OpenAPI regenerated for the two new routes.
* fix(cli/config): address Codex round-2 review on IPC handlers
Two findings from Codex review 4260902788 on 7e526a1:
**P2 — `config_set` accepts missing `value` field**
`handleSetConfig` validated `path` but not the presence of `value`. A
request body like `{ "path": "foo" }` (no `value` key) passed `undefined`
into `setNestedValue`, which then got dropped by `JSON.stringify` at save
time — silently deleting the key. That is distinct from the documented
"set to null to clear" semantics and made the route a footgun.
Add an explicit `"value" in bodyRecord` check that throws a
`BadRequestError("`value` is required (use `null` to clear a key)")`.
Explicit `null` is still accepted (the existing null-preservation path
in the writer handles it).
**P3 — `config_allowlist_validate` doesn't catch JSON.parse errors**
`validateAllowlistFile()` does a raw `JSON.parse` on
`secret-allowlist.json` and can throw on malformed JSON. With the route
calling it directly, a parse error propagated as a 500 — losing the
pre-IPC behavior of printing a user-readable message and exiting 1.
Wrap the handler in try/catch and return
`{ exists: true, parseError: message, errors: [] }` on failure. The CLI
side now renders `parseError` as
`Failed to read secret-allowlist.json: <msg>` and exits 1, matching the
pre-refactor surface.
**Tests**
New `assistant/src/__tests__/config-set-route.test.ts` covering both
routes daemon-side (9 tests, all pass):
- `config_set`: rejects missing `value`, accepts explicit null, rejects
missing/empty `path`, rejects non-object body, writes scalar through
to raw config.
- `config_allowlist_validate`: returns `parseError` when underlying
validator throws, returns `{ exists: false }` when file is absent,
returns `{ exists: true, errors }` (empty or with pattern details)
for the well-formed cases.
No public API change beyond the new `parseError` field on the validate
response, which the CLI is the only consumer of.
---------
Co-authored-by: credence-the-bot[bot] <277301654+credence-the-bot[bot]@users.noreply.github.com>
Co-authored-by: credence-the-bot[bot] <credence-the-bot[bot]@users.noreply.github.com>
1 participant
Summary
Removes the
assistant autonomyCLI command. Nothing in the codebase reads theautonomy-config.jsonfile it manages — verified via repo-wide grep forautonomy-config,AutonomyConfig,loadAutonomy, etc. Zero hits outside the command itself.The "autonomy" concept used in the assistant today (
PlaybookAutonomyLevelinplaybooks/types.ts) is per-playbook, has different semantics, and lives in its own schema. The CLI command is left over from an earlier design.Evidence the command is dead
cli/commands/autonomy.tsother thancli/program.ts(registration).PlaybookAutonomyLevel: that one lives in playbook records, gated byisValidAutonomyLevel, with no path to or fromautonomy-config.json.Changes
assistant/src/cli/commands/autonomy.tsassistant/src/cli/program.tsassistant/src/cli/commands/completions.tsRisk
assistant autonomy …will now get the standard "unknown command" error. The command was undocumented, unmentioned in the assistant CLI section ofAGENTS.md, and had no daemon-side consumers. Any priorautonomy-config.jsonon disk is inert and remains untouched.Local checks
bun run typecheck✅bun run lint✅