feat(dashboard): give BTW its own page under More Features - #220
Conversation
0a0cad7 to
f34bae7
Compare
BegoniaHe
left a comment
BegoniaHe
left a comment
There was a problem hiding this comment.
Review of the page-move plus CodingAgentsEditor (stacked on #206).
Blocking:
- Reordering or deleting a coding agent, then saving, can attach another agent's stored
api_keybecause config restore matches nested secrets by list index, not by id. GET redactsapi_keyto__ASTRBOT_REDACTED__; this editor posts that placeholder back aftermoveAgent/removeAgent. - Switching the profile select reloads immediately and drops unsaved BTW edits. Config page already prompts.
- Docs still describe Config → AI → Capabilities → BTW dual loops. This PR removes that entry and adds
/btwwithout updatingdocs/{zh,en}/use/webui.mdor the BTW sections indocs/{zh,en}/dev/astrbot-config.md.
Also: Dashboard copy says the Computer Use runtime may be local or sandbox and that provider keys are never written to disk/config. Runtime only enables agents when computer_use_runtime == "local", and keys persist in the AstrBot profile JSON.
cliConfigPage i18n and the tool.memory_write / tool.schedule_write registry edits look like leftovers from the #206 / CLI-switcher split.
| function removeAgent(index: number) { | ||
| commit(entries.value.filter((_, i) => i !== index)); | ||
| } | ||
|
|
||
| function moveAgent(index: number, delta: number) { | ||
| const target = index + delta; | ||
| if (target < 0 || target >= entries.value.length) return; | ||
| const next = [...entries.value]; | ||
| [next[index], next[target]] = [next[target], next[index]]; | ||
| commit(next); |
There was a problem hiding this comment.
Blocking: removeAgent / moveAgent plus a save will swap or steal stored provider keys.
GET /config redacts nested api_key to __ASTRBOT_REDACTED__ (SENSITIVE_CONFIG_KEYS). _restore_redacted_sensitive_config then restores list-of-object secrets by index, not by agent/provider id (astrbot/dashboard/services/config_service.py around the list branch of restore).
So:
[A, B]→ move B up → save → B is posted at index 0 with the placeholder → restore copies A's key into B (and A's key into the old B slot).- Delete index 0 → the next agent inherits the deleted agent's key; the deleted key is dropped.
This editor is the first Dashboard control that makes reordering a list of objects containing api_key a primary action. Match restore by id (agent + provider), or keep a side map of original keys keyed by identity and only send the placeholder for that same identity.
:key="index" on the agent/provider v-for (lines 43–44 and the provider cards) makes the same identity problem visible in the DOM: after a move, revealed is keyed by ${agentIndex}:${providerIndex} and follows the slot, not the preset.
| density="compact" | ||
| variant="outlined" | ||
| hide-details | ||
| @update:model-value="loadConfig" |
There was a problem hiding this comment.
v-model="scope" commits first, then this handler calls loadConfig() with no unsaved check. hasUnsavedChanges exists, but onBeforeRouteLeave only covers leaving the route.
Config page (ConfigPage.vue onConfigChange / openUnsavedChangesDialog) prompts save / discard / stay before switching profiles. Here, changing Work vs default silently replaces configData and bumps configKey, so in-progress work-loop / coding-agent edits disappear.
If the operator cancels, you also need to revert scope — v-model has already moved.
| persistent-hint | ||
| density="compact" | ||
| variant="outlined" | ||
| hide-details |
There was a problem hiding this comment.
hide-details and persistent-hint cancel each other in Vuetify 3: details (including the hint) are not rendered. btwPage.appliesToHint will never show.
Drop hide-details, or use hide-details="auto" if you only want to hide empty messages.
| 'agent_computer_use', | ||
| 'proactive_capability', | ||
| 'btw', | ||
| // `btw` lives on its own page now (More Features > BTW Dual Loop); the |
There was a problem hiding this comment.
The UI move is not reflected in docs. On this branch docs/zh/dev/astrbot-config.md / docs/en/dev/astrbot-config.md still say operators configure BTW via Config → AI → Capabilities → BTW dual loops (plugin/MCP/Skill assignment sections). This PR does not touch docs/.
AGENTS.md requires bilingual docs plus an old-to-new row in docs/zh/use/webui.md and docs/en/use/webui.md when a WebUI entry point moves. Please add something like:
配置文件 → AI → 能力 → BTW 双循环 → 更多功能 → BTW 双循环 (/btw)
and point the ConfigDocsLink target (dev/astrbot-config.html) at the new location in those BTW sections.
| }, | ||
| "coding_agents": { | ||
| "description": "Third-party coding agents", | ||
| "hint": "Local CLI agents that can be handed write tasks (for example Claude Code or Codex). Each entry carries an id, type, command, permission mode, and provider presets. A provider's endpoint and model take effect as that CLI's own config layer; its key is passed only in the child's environment, is never written to disk, and never rewrites the user's global configuration. A delegation starts a local process and writes files, so the work loop's Computer Use runtime must also be local or sandbox, and `tool.local_exec` and `tool.file_write` must both be authorized." |
There was a problem hiding this comment.
This overlay is what the Dashboard actually renders, and it contradicts the runtime.
enabled_coding_agents() returns [] unless btw.work_loop.computer_use_runtime == "local". sandbox is explicitly rejected (a delegated process is started on the host, not inside the sandbox). default.py's Chinese metadata hint is correct (local；sandbox 下委派等于绕过沙箱). codingAgentsEditor.hint also says local-only.
Here (and the zh-CN twin) say "local or sandbox". An operator who follows this hint, sets sandbox, and adds agents will get a silent no-op.
Same paragraph: "its key is passed only in the child's environment, is never written to disk" is also false. The key is stored on the AstrBot profile (btw.work_loop.coding_agents[].providers[].api_key) and only omitted from the generated Claude/Codex layer. Please say that it is persisted in the AstrBot config and not written into the CLI's own files.
| "timeoutSeconds": "Timeout (seconds)", | ||
| "maxOutputChars": "Output limit (characters)", | ||
| "providers": "Provider presets", | ||
| "providersHint": "The active preset's endpoint and model take effect as this CLI's own config layer. Its key is passed only in the child's environment, never written to disk.", |
There was a problem hiding this comment.
providersHint / providerApiKeyHint (line 272) repeat the same overclaim: "never written to disk" / "never written to the configuration".
Saving this form writes api_key into the profile via configProfileApi.update. Runtime then copies it into the child env and deliberately does not put it in the generated --settings / --profile layer (_claude_settings / _codex_profile). Please describe that split; operators will otherwise treat the field as ephemeral.
| "unsavedTitle": "Unsaved BTW settings", | ||
| "unsavedMessage": "Leaving now discards them. The work loop's boundary is among what would be lost." | ||
| }, | ||
| "cliConfigPage": { |
There was a problem hiding this comment.
Dead strings. This PR's body says the CLI provider switcher was split out, but cliConfigPage (~100 lines) is still added here and in zh-CN/features/config.json. There is no /cli route, page, or t('cliConfigPage.*') consumer on this branch.
Drop it so the page-move PR does not carry the other change's copy (and so i18n:check is not locking in unused keys).
| variant="outlined" | ||
| hide-details | ||
| @update:model-value=" | ||
| patchProvider(index, providerIndex, { id: text($event) }) |
There was a problem hiding this comment.
patchProvider always ends with repointActive(providers, agent.active_provider).
If the active preset is not first and its id is edited (official → foo while gw sits at index 0), current is still the old id, it is missing from the new id list, and active snaps to ids[0]. The operator just renamed the preset they were using and silently switched away from it.
When the patched row's old id === agent.active_provider, set active to the new id; only then fall back to repointActive.
| /> | ||
| </v-col> | ||
| <v-col cols="12" sm="6"> | ||
| <v-select |
There was a problem hiding this comment.
wire_api is Codex-only (providerWireApiHint, and _codex_profile is the only writer). The select is rendered for claude_code and custom as well.
Gate it with entry.type === 'codex', same as permission_mode / sandbox.
| "tool.web_search", | ||
| "tool.session_history", | ||
| "tool.memory_read", | ||
| "tool.memory_write", |
There was a problem hiding this comment.
Out of scope for a Dashboard page move.
feat/btw-read-only-delegation already declares required_actions=("tool.memory_write",) / ("tool.schedule_write",) on the tools and lists them in tool_catalog.py, but does not register them in ACTIONS / ACTION_ROLE_GRANTS. Landing that registry (and dropping from __future__ import annotations in the same files) belongs on #206, not here.
If #206 merges without it, those tools stay on unregistered actions; if this PR merges first, the authorization surface changes for a UI-only commit.
`_restore_redacted_sensitive_config` put a redacted secret back by list position, so a list of objects carrying an `api_key` handed the wrong one back the moment the list moved. Reordering two coding agents, or deleting the first, left the remaining entry holding its neighbour's key, and a copied entry -- whose id the profile has never seen -- kept the `__ASTRBOT_REDACTED__` marker itself, which the CLI switcher would then write into the CLI's own configuration as if it were a credential. Entries that carry an `id` are now matched by it, which is the reading that survives a reorder. An entry the stored list does not name -- a new one, or one the client renamed, which it has no way to say -- falls back to its own position, but only while that stored item is not already claimed by a name, so a fallback can never hand the same secret out twice. A marker with nothing to restore from is blanked rather than stored: it is what a response says instead of a secret, never a secret itself. `sensitive_config_changed` reads the same lists the same way, so a reordered list is no longer reported as a change to a credential -- it was, which made saving one demand `provider.credentials.write`. The editor keys a card on its agent rather than on its slot, so the reveal state of a preset follows the preset instead of the position, and renaming the active preset keeps it active rather than silently falling back to the first one. The wire API select is offered only for Codex, which is the only profile that writes it. AI-Generated: true Generated-At: 2026-09-18T08:29:00Z
The profile select reloaded the moment it changed, so an edit still in hand was replaced and the form remounted around it -- for a page whose unsaved state is the work loop's boundary, that is the one thing not to lose quietly. The config page already answers this with a save / discard / stay prompt, so the BTW page uses the same one rather than a second wording of the same question. The select is bound one way on purpose: it stays on the profile being edited until the page decides to move, so staying needs no undo of a move that should not have happened. The select also carried both a persistent hint and `hide-details`. In Vuetify 3 those cancel out and the hint is never rendered, so the hint is now the only one of the two left. AI-Generated: true Generated-At: 2026-09-18T08:29:30Z
The BTW settings left the AI panel on the configuration page for a page of their own, and the developer guide still told operators to open **Config -> AI -> Capabilities -> BTW dual loops** for the per-plugin loop assignments. That path no longer exists; the setting is on the BTW page now. The user guide gains the entry pair the navigation table is for, so an operator following an older sentence lands somewhere, and a short paragraph saying what the page holds -- it is the same group the AI panel used to render, loaded and saved per profile. AI-Generated: true Generated-At: 2026-09-18T08:30:00Z
Two claims were wrong about what configuring a coding agent does. The Computer Use runtime has to be `local`, not "local or sandbox": a delegated run is a process started on the host, and `sandbox` is explicitly rejected there, so an operator who followed the hint and chose sandbox would add agents and watch them never run. The Chinese hint in `default.py` already said so; this is the English one catching up, in the copy the Dashboard actually renders. The second claim was that a provider key "is never written to disk". It is: the key is stored in the AstrBot profile like any other provider credential. What it is not written into is the CLI's own configuration layer, which the run generates and the key only travels in the child's environment -- a narrower and more useful thing to say. `cliConfigPage` goes with them. This branch ships no `/cli` route and no consumer of those keys; they arrived with the split and only lock in unused copy against the i18n check. AI-Generated: true Generated-At: 2026-09-18T08:30:30Z
f34bae7 to
3153eaf
Compare
Adding a row widened the first column, and the table was re-padded by hand instead of by the formatter, so every pipe in it stopped lining up: `markdownlint-cli2` reports MD060 on the row and `prettier --check` rewrites the whole table. AI-Generated: true Generated-At: 2026-09-18T09:24:00Z
2 participants
Stacked on #206. Review that one first; this branch sits directly on it.
What this does
The BTW settings were a group inside the AI section of the configuration page,
where the work loop's own boundary was easy to miss among the model and
agent-runner options. This gives the feature a page of its own, reached from
More Features, and drops the group from the AI panel so there is one place to
edit it.
The page loads and saves whichever configuration profile it is pointed at, using
the same renderer the config page uses, so nothing about the settings themselves
changes. It takes its step-up retry from the shared
runMutationWithStepUpinutils/stepUp.ts.Why it is its own PR
This branch was previously part of #206, together with the CLI provider switcher
that is now stacked on top of it in a separate PR. The two are independent
changes that happened to be written together: one moves settings the work loop
already owns onto their own page, the other adds a second, explicitly-invoked
write path into files AstrBot does not own. Splitting them means the second one
can be reviewed on the terms it needs — credentials, backups, step-up — without
the page move in the same diff.
Verification
Local gates on this commit alone (checked out in isolation): dashboard
typecheck, i18n check,vitest(page, panel and both smoke suites), andpyright --pythonplatform Linuxwith 0 errors.uv runtargets in the Makefile do not run on this host (.python-versionpins3.14.6); the equivalent commands were run directly.