Skip to content

feat(btw): keep the agent settings on the BTW page - #225

Merged
YUZHEthefool merged 3 commits into
cli-provider-switchfrom
btw-agent-config-split
Sep 18, 2026
Merged

YUZHEthefool merged 3 commits into
cli-provider-switchfrom
btw-agent-config-split

Conversation

@YUZHEthefool

Copy link
Copy Markdown
Member

Stacked on #219. Review cli-provider-switch first.

The page could not load — two faults under one symptom

The provider list was never shown, and fixing it took two separate changes,
because the first fault hid the second.

  1. The state route asked for coding_cli.config.write, a high-risk action. The
    Dashboard answers a high-risk action with a step-up challenge until the
    operator proves themself, and a plain GET has no way to answer one, so every
    page load was denied — auth_audit_log holds nothing but step_up_required
    for that action. Reading a file path and whether a key is stored is not the
    write that action guards, so the read now asks for platform.read, and the
    switch / restore routes keep the high-risk action to themselves.
  2. Behind that denial, _config_of looked for the config profile service on the
    core runtime's services, where it does not exist (RuntimeServices carries
    the authorization service and the other core-owned pieces; the profile
    service is on the Dashboard's own app.state.services). The route answered
    503 Configuration is unavailable even once it was authorized, which is why
    fixing the action alone changed nothing visible.

The editor also answers a challenge instead of dead-ending on one: the state
call travels the step-up path, so a session that still has to prove itself gets
the dialog and the request is retried with the token.

Where the settings live now

  • BTW Dual Loop (/btw) — the loops and the coding-agent editor:
    command, model, output cap, permission mode, sandbox, extra writable
    directory. Which CLI carries out a delegated task, and how it is set up, is
    part of configuring the work loop, so it is read next to the loop.
  • Third-party Agent Config (/third-party-agents) — only the list each CLI
    is switched between, plus the switch. It always edits the running profile: a
    switch resolves the provider in that profile's list, so the profile selector
    the page used to offer could only produce a dead end.

The per-agent provider presets are gone

They chose the endpoint a delegated run was layered with. A provider is now
configured once — on the third-party agent page, into the CLI's own
configuration — instead of once for every agent.

A profile that still carries a preset would keep an invisible setting alive, so
the editor drops providers / active_provider from an entry the moment it is
saved. The backend still honors a preset written into the configuration by hand;
the docs say so, together with the consequence that follows: an agent with no
preset of its own runs against the CLI's configuration, so switching a provider
there changes the provider a task uses, not only your own sessions.

Notes for review

  • The fix commit is separate from the rework, but both faults are in code feat(btw): switch a coding CLI's own provider from the dashboard #219
    introduces, so it can be folded into that PR if you would rather not carry a
    follow-up that fixes the PR directly below it.
  • mdi-console-line for the sidebar entry is already in the generated MDI
    subset; no new icon dependency.

Verification

  • dashboard: vitest 320 passed (the 4 favicon.svg suite failures predate this
    change), vue-tsc, prettier --check, i18n check, design-system check.
  • python: metadata i18n tests, and the coding-CLI route tests, which now cover
    that the read action is not high-risk, that the GET runs through the read
    authorizer, that the profile is read from the Dashboard's services, and that a
    step-up challenge is answered and retried.
  • live: the backend was rebuilt with this branch, data/dist synced, the page
    loaded against it, and the audit log no longer records a denial for the state
    request.

uv run targets in the Makefile do not run on this host (.python-version pins
3.14.6); the equivalent commands were run directly. pyright --pythonplatform Linux reports 0 errors.

@YUZHEthefool
YUZHEthefool added this pull request to stack #221 September 18, 2026 06:00

@BegoniaHe BegoniaHe left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI-assisted review of the split and the GET auth fix. CI is green; the gaps below are mostly untested product/auth paths, not lint.

Highest severity: configProfileApi.get redacts api_key to __ASTRBOT_REDACTED__, but this page treats that string as the real credential. Duplicate / add / delete-and-save can persist the placeholder or restore keys by list index. The new tests hand the page sk-secret and never exercise the redaction contract.

Also: leftover per-agent presets are stripped from every agent on the first BTW-page edit; GET state is wrapped in step-up for platform.read, which the backend cannot issue; /cli-config has no redirect; the config docs still say btw.cli_providers lives on the same page as the agents.

Comment thread dashboard/src/api/v1/codingCli.ts Outdated
* the profile has been read, and an empty string when the operator has not
* typed one. Only a switch writes it anywhere, and only into the CLI's own
* file -- the profile keeps the list itself, which is why the page can read a
* key back and the API cannot.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is not true of the profile endpoint this page actually reads.

configProfileApi.get runs _redact_sensitive_config, so btw.cli_providers[].api_key comes back as __ASTRBOT_REDACTED__, not the credential. The coding-CLI GET never returns api_key either. The editor therefore cannot "read a key back"; it is holding the redaction placeholder.

That is what makes the save/duplicate path below dangerous: a new row that copies api_key from an existing card will POST the placeholder as a real value. _restore_redacted_sensitive_config only replaces the placeholder when it is still sitting on the same list index as the stored entry.

Comment on lines +177 to +198
set: (next) => {
const config = configData.value as Record<string, unknown>;
const btw = asRecord(config.btw) ?? {};
const byId = new Map(
rawProviders(configData.value).map((entry) => [
text(entry.id).trim(),
entry,
]),
);
btw.cli_providers = next.map((provider) => {
const entry: Record<string, unknown> = {
...(byId.get(provider.id) ?? {}),
id: provider.id,
name: provider.name,
base_url: provider.base_url,
model: provider.model,
note: provider.note,
};
if (provider.cli) entry.cli = provider.cli;
else delete entry.cli;
if (provider.api_key) entry.api_key = provider.api_key;
else delete entry.api_key;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This setter rebuilds cli_providers from the child's emit and does not speak the config redaction protocol.

configProfileApi.get('default') returns api_key: "__ASTRBOT_REDACTED__". The getter copies that string through. Then:

  • if (provider.api_key) entry.api_key = provider.api_key writes the placeholder back for existing rows (ok only while the list order is unchanged — restore is by index, not by id).
  • else delete entry.api_key drops the key. Restore iterates posted keys, so a missing api_key is a real deletion, not "leave it alone" as the comment above claims.
  • A new row (add / duplicate) is past len(current). Restore breaks and the placeholder is stored as the credential. A later switch writes __ASTRBOT_REDACTED__ into ~/.claude/settings.json / ~/.codex/auth.json.

Merge by id against the loaded snapshot: keep the placeholder on the original id, never copy it onto a new id, and do not delete api_key when the child sent an empty field.

dashboard/tests/thirdPartyAgentsPage.vitest.ts feeds the page api_key: 'sk-secret', which is not what GET returns, so this never fails in CI.

Comment on lines +594 to +605
function duplicate(cli: CodingCliKind, provider: CodingCliProvider) {
const id = nextId(`${provider.id}-copy`, cli);
const source = find(provider.id);
commit([
...providers.value,
{
...provider,
id,
name: `${provider.name} (copy)`,
current: false,
cli: source?.cli ?? '',
api_key: source?.api_key ?? '',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duplicate copies source.api_key. After a profile GET that value is __ASTRBOT_REDACTED__, and the copy is a new id/index.

On save, _restore_redacted_sensitive_config restores placeholders only while idx < len(current). The original keeps its real key; the copy persists the placeholder as api_key. Switch then applies that string as the CLI credential.

Duplicate without a key (or with api_key: '' and has_api_key: false) and let the operator type a new one. Do not copy the redaction token.

Comment on lines +549 to +589
function confirmForm() {
const id = form.value.id.trim();
if (!id) {
formError.value = tm('thirdPartyAgentsPage.idRequired');
return;
}
if (!form.value.base_url.trim() && !form.value.api_key.trim()) {
formError.value = tm('thirdPartyAgentsPage.endpointOrKeyRequired');
return;
}
const scope = form.value.cli === 'claude_code' ? '' : form.value.cli;
const clash = providers.value.some(
(provider) => provider.id === id && (provider.cli ?? '') === scope,
);
const editingSelf =
formEditing.value &&
find(id) !== undefined &&
(find(id)?.cli ?? '') === scope;
if (clash && !editingSelf) {
formError.value = tm('thirdPartyAgentsPage.idTaken');
return;
}

const apiKey = form.value.api_key.trim();
const previous = find(id);
const entry: StoredCliProvider = {
id,
name: form.value.name.trim() || id,
base_url: form.value.base_url.trim(),
model: form.value.model.trim(),
note: form.value.note.trim(),
has_api_key: Boolean(apiKey) || Boolean(previous?.has_api_key),
current: false,
cli: scope,
api_key: apiKey || (previous?.api_key ?? ''),
};

const index = providers.value.findIndex((provider) => provider.id === id);
const next = [...providers.value];
if (index === -1) next.push(entry);
else next[index] = entry;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Three identity bugs in one submit:

  1. openForm does not keep the original id. Change the id while editing → findIndex misses → next.push(entry) and the old row stays. The previous key is also lost, because previous = find(newId).
  2. Clash is (id, cli), but find / findIndex / remove (line 616) are id-only. Two rows with the same id (Claude-add uses cli: '', Codex-add uses cli: 'codex', and nextId is computed per visible list) overwrite or delete each other.
  3. Backend normalize_cli_providers also dedupes by id globally and silently drops the second. UI uniqueness and switch lookup will disagree.

Keep originalId for the edit, key the list by (id, cli), and match the backend's id-only rule in the form (or change the backend to accept per-CLI ids — not both).

Comment on lines +506 to +518
// Through the step-up path even though a read is an ordinary permission: a
// session that has not proved itself yet is answered with a challenge, and
// without this the page would dead-end on a button that cannot elevate.
const response = await runMutationWithStepUp(
(stepUp) =>
codingCliApi.state({ headers: stepUp ? stepUpHeaders(stepUp) : {} }),
{
action: READ_ACTION,
resourceType: 'instance',
resourceId: RUNNING_SCOPE,
},
requestStepUp,
);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cannot do what the comment says.

READ_ACTION is platform.read. test_the_state_action_is_an_ordinary_read asserts it is not high-risk and requires_step_up is False. AuthorizationService.issue_step_up raises Invalid step-up request for any action that is not high-risk.

So: GET no longer challenges (that was the point of the first commit), and if a 401 with requires_step_up ever did appear, the dialog would call stepUp({ action: 'platform.read' }) and the backend would reject it. The vitest that mocks that 401 is testing a path the server will not produce.

Drop the step-up wrapper on state(), or introduce a dedicated coding_cli.config.read if this GET should stay off platform.read. Do not issue step-up against an action that cannot be stepped up.

每个代理的 `providers` 是 provider 预设列表,`active_provider` 选择生效的一项。预设的 `base_url`、`model` 会写进该 CLI 自己的配置层:Claude Code 用 `--settings` 指向数据目录下的 settings 文件,Codex 用 `--profile astrbot-btw-<代理 ID>` 叠加 `$CODEX_HOME/astrbot-btw-<代理 ID>.config.toml`。每个代理各用一份 Codex 配置层,两个代理同时运行时不会互相覆盖。密钥不落盘,只在拉起子进程时通过环境变量传入——Claude Code 读 `ANTHROPIC_AUTH_TOKEN`,Codex 由配置层的 `env_key` 指向按代理 ID 命名的变量——因此持久化的只有端点与模型。切换 provider 不会改写用户的全局 CLI 配置。预设既无 `base_url` 也无 `api_key` 时视为“官方登录”,不写任何配置层,代理沿用用户自己的登录。
每个代理还可以带一份 `providers` 预设列表与 `active_provider`:Dashboard 已不再编辑它们(代理用的 provider 改由 **更多功能 → 第三方agent配置** 页面切换该 CLI 自己的配置),但配置文件里手写的预设仍按下面的规则生效。预设的 `base_url`、`model` 会写进该 CLI 自己的配置层:Claude Code 用 `--settings` 指向数据目录下的 settings 文件,Codex 用 `--profile astrbot-btw-<代理 ID>` 叠加 `$CODEX_HOME/astrbot-btw-<代理 ID>.config.toml`。每个代理各用一份 Codex 配置层,两个代理同时运行时不会互相覆盖。密钥不落盘,只在拉起子进程时通过环境变量传入——Claude Code 读 `ANTHROPIC_AUTH_TOKEN`,Codex 由配置层的 `env_key` 指向按代理 ID 命名的变量——因此持久化的只有端点与模型。切换 provider 不会改写用户的全局 CLI 配置。预设既无 `base_url` 也无 `api_key` 时视为“官方登录”,不写任何配置层,代理沿用用户自己的登录。

同一个页面还维护 `btw.cli_providers`,那是另一份列表:每个 CLI 一份 provider 列表,由操作者手动切换。点「启用」会把选中的 provider 写进该 CLI 在本机的全局配置——Claude Code 写 `~/.claude/settings.json` 的 `env` 块,Codex 在 `~/.codex/config.toml` 顶部写一段受管区块、密钥存进 `~/.codex/auth.json`。首次写入前原文件会被完整备份一次,「取回」用该备份还原并清掉 AstrBot 写入的密钥。代理没有配置自己的 `providers` 预设时,委派任务读的就是这个文件:在这里切换 provider,任务与你手动启动的会话用的是同一份配置。切换改写了不属于 AstrBot 的文件,因此按 `coding_cli.config.write` 授权并要求 step-up。

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

「同一个页面还维护 btw.cli_providers」已经不成立:本 PR 把 agent 卡片留在 BTW 页,把 cli_providers 和切换挪到了 更多功能 → 第三方agent配置/third-party-agents)。请改成那个入口,英文页 docs/en/dev/astrbot-config.md 的 "The same page keeps btw.cli_providers" 是同一处。

另外 /cli-config 被删掉了,docs/zh/use/webui.md / docs/en/use/webui.md 需要旧→新路径映射(见 AGENTS.md),MainRoutes.ts 也没有从 /cli-config 做 redirect。

Each agent's `providers` is a list of provider presets, and `active_provider` selects the one in effect. A preset's `base_url` and `model` are written into that CLI's own config layer: Claude Code through `--settings` pointing at a settings file in the data directory, and Codex through `--profile astrbot-btw-<agent id>` layering `$CODEX_HOME/astrbot-btw-<agent id>.config.toml`. Each agent gets a layer of its own, so two agents running at once cannot overwrite each other's provider. The credential is never written to disk; it is passed in the child's environment for the length of the run, which Claude Code reads as `ANTHROPIC_AUTH_TOKEN` and which Codex names through the layer's `env_key`, a variable named for the agent. Switching providers therefore never rewrites the user's global CLI configuration, and only the endpoint and model persist. A preset with neither `base_url` nor `api_key` counts as an official login: no layer is written and the agent keeps the user's own session.
Each agent may also carry a `providers` list of presets and an `active_provider`. The Dashboard no longer edits them -- a CLI's provider is switched on the **More Features → Third-party Agent Config** page, which rewrites the CLI's own configuration -- but a preset written into the config file by hand still takes effect as described below. A preset's `base_url` and `model` are written into that CLI's own config layer: Claude Code through `--settings` pointing at a settings file in the data directory, and Codex through `--profile astrbot-btw-<agent id>` layering `$CODEX_HOME/astrbot-btw-<agent id>.config.toml`. Each agent gets a layer of its own, so two agents running at once cannot overwrite each other's provider. The credential is never written to disk; it is passed in the child's environment for the length of the run, which Claude Code reads as `ANTHROPIC_AUTH_TOKEN` and which Codex names through the layer's `env_key`, a variable named for the agent. Switching providers therefore never rewrites the user's global CLI configuration, and only the endpoint and model persist. A preset with neither `base_url` nor `api_key` counts as an official login: no layer is written and the agent keeps the user's own session.

The same page keeps `btw.cli_providers`, which is a different list: one provider list per CLI, switched by hand. **Switch** writes the selected provider into that CLI's own global configuration on this host -- Claude Code's `env` block in `~/.claude/settings.json`, and for Codex a managed block at the top of `~/.codex/config.toml` with the credential in `~/.codex/auth.json`. The file is backed up once before the first write, and **Take back** restores that copy and drops the credentials AstrBot stored. When an agent carries no `providers` preset of its own, a delegated run reads this same file: switching a provider here changes the provider a task runs against, as well as your own sessions. Because a switch rewrites a file AstrBot does not own, it is authorized as `coding_cli.config.write` and asks for step-up.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same stale sentence as the Chinese page: btw.cli_providers is no longer on the BTW Dual Loop page. It is More Features → Third-party Agent Config (/third-party-agents). Please fix both languages together.

path: '/cli-config',
component: () => import('@/views/CliConfigPage.vue'),
name: 'ThirdPartyAgents',
path: '/third-party-agents',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/cli-config is removed with no alias. Bookmarks and any note that pointed at #219's page 404.

Add a redirect (/cli-config/third-party-agents) and an old-to-new row in docs/zh/use/webui.md + docs/en/use/webui.md. AGENTS.md requires that mapping when a WebUI entry point is renamed.

id: 'gw',
name: 'Gateway',
base_url: 'https://gw.example',
api_key: 'sk-secret',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the payload GET /config/profiles/default returns. Secrets are __ASTRBOT_REDACTED__.

Please add cases that match production:

  1. Load with the placeholder, duplicate, save → the new row must not persist __ASTRBOT_REDACTED__ as api_key.
  2. Load with the placeholder, delete the first of two keyed rows, save → the remaining row must not restore the deleted row's key (restore is by index today).
  3. Switch/Enable on an id that exists only in the unsaved emit → should not hit the API / should explain that the list is not saved yet.

# state names a file path and what the CLI currently holds, never a credential,
# and the read route cannot answer a step-up challenge: gating it behind the
# high-risk action above meant the page could not be filled in at all.
READ_ACTION = "platform.read"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reusing platform.read unblocks the GET, but it is the IM/config-profile action, not a coding-CLI read. The GET still returns host file paths, whether a credential is stored, and the current endpoint/model from ~/.claude / ~/.codex.

Anyone who can platform.read (session and above) now sees that, without coding_cli.config.write and without step-up. If that is the intended bar, say so next to WRITE_ACTION. If not, add coding_cli.config.read as a normal (non-step-up) action instead of borrowing platform.read.

The frontend load() still tries to step-up this action; that call is invalid, see the comment on CodingCliProviders.vue.

@xero-team-bot

xero-team-bot Bot commented Sep 18, 2026

Copy link
Copy Markdown

⚠️ This PR conflicts with its base branch and needs a rebase.

git fetch origin cli-provider-switch
git rebase origin/cli-provider-switch
# after resolving the conflicts
git push --force-with-lease

The needs-rebase label is removed automatically once the conflicts are gone.
(Xero-Team/AstrBot · detected by xero-bot)

@xero-team-bot

xero-team-bot Bot commented Sep 18, 2026

Copy link
Copy Markdown

✅ Conflicts resolved; removing the needs-rebase label.

The third-party agent page had grown two halves that answer different
questions.  Which CLI carries out a delegated task, and how that CLI is
set up -- command, model, output cap, permission mode, sandbox, the extra
directory it may write to -- is part of configuring the work loop, so
that editor goes back to the BTW page, next to the loop and its boundary.
The settings were briefly given a page of their own for that reason; they
read better where the loop that uses them is configured.

What stays behind is the list each coding CLI is switched between: one
list per CLI and the switch that writes the chosen entry into the CLI's
own configuration.  The page no longer offers a profile selector, because
a switch resolves the provider in the running profile's list and nothing
else can be acted on from here.

The per-agent provider presets are gone.  They chose the endpoint a
delegated run was layered with, and the CLI's own configuration now
answers that: a provider is configured once, on the third-party agent
page, instead of once for every agent.  A profile that still carries a
preset would keep an invisible setting alive, so the editor drops the
stored fields the moment an entry is saved.  The backend still honors a
preset written into the configuration by hand.

The provider entry the page edits is now the shared `StoredCliProvider`,
which the API module owns alongside the type it extends, and the route
tests below the app cover the read the page needs as well as the switch.

AI-Generated: true
Generated-At: 2026-09-18T09:02:00Z
The profile reports a stored key as `__ASTRBOT_REDACTED__`, and the editor
read that marker into the entry as if it were the key.  Saving then
carried it wherever the entry moved: a rename put it in front of a
different provider's key, and a copy under a new id kept the marker
itself, which a switch would write into the CLI's own file as if it were
a credential.  An entry added from a section was given no scope at all,
so it appeared under both CLIs while the profile -- which keys its list by
id -- would silently drop one of the two on the next load.

The marker now means what it says: a key is stored, the field is empty.
It goes back only for the entry it came from, whose id the save still
carries; a copy is metadata only and starts with no key.  An entry added
from a section is scoped to that section, and an id another entry already
uses is refused across the whole list rather than per CLI.

The hint under the key field said the key lives in the CLI's own file,
which is where a switch puts it, not where it is kept.  It is kept in the
AstrBot profile, and the hint now says so.

AI-Generated: true
Generated-At: 2026-09-18T09:12:00Z
A card was keyed by its position, so moving an agent carried the slot's
state along with the number rather than with the agent that moved, and a
card could keep showing what the agent that used to be there was showing.

Nothing here holds a secret any more, so this is not the same fault it
would have been; it is still the wrong identity, and the place a panel's
DOM state is decided.

AI-Generated: true
Generated-At: 2026-09-18T09:12:30Z
@YUZHEthefool
YUZHEthefool merged commit 00c5795 into master Sep 18, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants