From f4111a370160b890faede3d3b49eeea649b24da5 Mon Sep 17 00:00:00 2001 From: creatixchu Date: Thu, 30 Jul 2026 18:07:35 +0800 Subject: [PATCH] fix(ui): drop the caption from the /permission error text too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review round: the no-caption rule the Agent Note states applied only to the success texts, leaving `permission · unknown permission preset "bogus" (…)`. The error text now reads `unknown preset "bogus" (…)` and its exact wording is pinned; the fixture mirror drops `JSON.stringify` for the host's own quoting so the two cannot drift on a quoted argument. The Note now states the line it draws: the rule bans a caption for the command's own value, not the vocabulary, so `/plan`'s `Plan mode off.` and `/goal`'s `Goal cleared.` conform as written — recorded with the broader name-ban as a rejected alternative. The web row assertions are scoped to the row so unrelated page text reading `permission` cannot satisfy them. --- .../2026-07-30-command-row-copy-contract.i18n.yaml | 4 ++-- .../architecture/2026-07-30-command-row-copy-contract.md | 8 ++++++-- .../2026-07-30-command-row-copy-contract.zh.md | 8 ++++++-- apps/web/tests/seeded-history.e2e.ts | 9 ++++++--- packages/client/connection/src/client/fixture.ts | 2 +- packages/ui/permission/src/index.ts | 8 ++++---- packages/ui/permission/tests/projection.spec.ts | 8 +++++++- 7 files changed, 32 insertions(+), 15 deletions(-) diff --git a/.agents/notes/implemented/architecture/2026-07-30-command-row-copy-contract.i18n.yaml b/.agents/notes/implemented/architecture/2026-07-30-command-row-copy-contract.i18n.yaml index f78521f13b..330a279b35 100644 --- a/.agents/notes/implemented/architecture/2026-07-30-command-row-copy-contract.i18n.yaml +++ b/.agents/notes/implemented/architecture/2026-07-30-command-row-copy-contract.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/architecture/2026-07-30-command-row-copy-contract.md -2026-07-30-command-row-copy-contract.md: 984f2a94a4aaa23b3892f1f62a5374ab74b38723 -2026-07-30-command-row-copy-contract.zh.md: 3dd99927a346c29724a67193bb02d87e0b0c7e5c +2026-07-30-command-row-copy-contract.md: f6d5199389b3907780c501894e2861e6add85e77 +2026-07-30-command-row-copy-contract.zh.md: 4afaf31640c07e88765060681739f262f322769e diff --git a/.agents/notes/implemented/architecture/2026-07-30-command-row-copy-contract.md b/.agents/notes/implemented/architecture/2026-07-30-command-row-copy-contract.md index 984f2a94a4..f6d5199389 100644 --- a/.agents/notes/implemented/architecture/2026-07-30-command-row-copy-contract.md +++ b/.agents/notes/implemented/architecture/2026-07-30-command-row-copy-contract.md @@ -14,7 +14,9 @@ The row's two halves have disjoint jobs, and each side is written to its own hal The row title is the bare command name — no `/`, no arguments. The `/` belongs to the composer's input grammar, not to a settled record, and the argument is not the row's to report: the summary already says what the command did. `GenericCommandCard` keeps the `命令` fallback for a cross-window node whose `command/run` page fell out of the client's window. -A command handler's settlement `text` therefore never repeats the command's own name, because the surface that renders it has already said it. `/permission` returns `preset workspace-write` and, bare, `current preset workspace-write (available: …)`. Read as a row this is `permission · preset workspace-write`; read as a standalone line — the TUI appends the same text as a notice — it still states which preset now applies. +A command handler's settlement `text` therefore never labels its value with the command's own name, because the surface that renders it has already said it. `/permission` returns `preset workspace-write`, bare `current preset workspace-write (available: …)`, and for a bad argument `unknown preset "bogus" (available: …)`. Read as a row this is `permission · preset workspace-write`; read as a standalone line — the TUI appends the same text as a notice — it still states which preset now applies. + +The rule bans the *label*, not the vocabulary. `Permission preset: workspace-write.` lost because `Permission preset:` is a caption for a value whose caption is already the title. A domain noun that happens to contain the command's name is not a caption and stays: `/plan` keeps `Plan mode off.` and `Plan mode on. Use /plan off to leave.` (`plan · Plan mode off.` names the mode, and the tail is an instruction, not an echo), and `/goal` keeps `Goal cleared.`. A handler that finds itself writing ` :` in front of its own value is the case this rule catches. The log is unchanged: `command/run` keeps the structured `name`/`args` split, so a richer registered command row can still render arguments from the same node without a second data channel. @@ -26,6 +28,8 @@ The log is unchanged: `command/run` keeps the structured `name`/`args` split, so **Have the row strip a leading command name from the settlement text.** Presentation would silently rewrite handler-authored text, and every handler that phrased its outcome differently would defeat the heuristic. +**Ban the command's name from its settlement text outright, rewriting `/plan` and `/goal` to match.** The broader ban costs more than it buys: `Plan mode off.` and `Goal cleared.` are the clearest sentences those outcomes have, in the row and as standalone TUI notices both, and the shortenings that satisfy a name ban (`off.`, `cleared.`) read as fragments. Captions are the redundancy worth removing. + ## Consequences -Every command row gets shorter, and the rule scales: a new command's author writes its outcome without knowing which surface renders it, and no surface has to de-duplicate. The cost is that the dispatched arguments leave the collapsed row — while a command is still executing the row shows only its name and `执行中…` — and that the no-repetition rule is a convention the reviewer enforces, not a gate. The `/permission` texts are pinned by the permission package's command tests, and the assembled row copy by the [seeded-history](../../../../apps/web/tests/snapshots/seeded-history/command-row.expected.md) web golden, which reaches a real settled command row keylessly because `/permission` runs entirely on the host. +Every command row gets shorter, and the rule scales: a new command's author writes its outcome without knowing which surface renders it, and no surface has to de-duplicate. The cost is that the dispatched arguments leave the collapsed row — while a command is still executing the row shows only its name and `执行中…` — and that the no-caption rule is a convention the reviewer enforces, not a gate. The `/permission` texts are pinned by the permission package's command tests, and the assembled row copy by the [seeded-history](../../../../apps/web/tests/snapshots/seeded-history/command-row.expected.md) web golden, which reaches a real settled command row keylessly because `/permission` runs entirely on the host. diff --git a/.agents/notes/implemented/architecture/2026-07-30-command-row-copy-contract.zh.md b/.agents/notes/implemented/architecture/2026-07-30-command-row-copy-contract.zh.md index 3dd99927a3..4afaf31640 100644 --- a/.agents/notes/implemented/architecture/2026-07-30-command-row-copy-contract.zh.md +++ b/.agents/notes/implemented/architecture/2026-07-30-command-row-copy-contract.zh.md @@ -14,7 +14,9 @@ Web 命令行由一对落库的[命令生命周期事件](../../proposed/archite 行标题就是裸命令名——没有 `/`，也没有参数。`/` 属于编辑器的输入语法，不属于一条已落定的记录；参数也不该由这一行来报告：摘要已经说清了这条命令做了什么。对于 `command/run` 那一页已滑出客户端窗口的跨窗口节点，`GenericCommandCard` 仍保留 `命令` 兜底标题。 -因此，命令 handler 的落定 `text` 绝不重复命令自身的名字——渲染它的界面已经说过一次了。`/permission` 返回 `preset workspace-write`，裸调用时返回 `current preset workspace-write (available: …)`。作为一行读是 `permission · preset workspace-write`；作为独立一句读——TUI 把同一段 text 作为通知追加——它依然说明了当下生效的是哪个预设。 +因此，命令 handler 的落定 `text` 绝不用命令自身的名字给自己的值加标签——渲染它的界面已经说过一次了。`/permission` 返回 `preset workspace-write`，裸调用时返回 `current preset workspace-write (available: …)`，参数非法时返回 `unknown preset "bogus" (available: …)`。作为一行读是 `permission · preset workspace-write`；作为独立一句读——TUI 把同一段 text 作为通知追加——它依然说明了当下生效的是哪个预设。 + +这条规则禁的是*标签*，不是用词。`Permission preset: workspace-write.` 之所以出局，是因为 `Permission preset:` 是给一个值加的题头，而这个题头正是标题本身。恰好含有命令名的领域名词不是题头，因此保留：`/plan` 仍返回 `Plan mode off.` 与 `Plan mode on. Use /plan off to leave.`（`plan · Plan mode off.` 说的是那个模式，句尾是一条指引，不是回声），`/goal` 仍返回 `Goal cleared.`。真正被这条规则拦下的，是 handler 在自己的值前面写出 `<命令名> <名词>：` 的那一类。 日志本身未变：`command/run` 保留结构化的 `name`／`args` 拆分，因此更丰富的已注册命令行仍可从同一个节点渲染参数，无需第二条数据通道。 @@ -26,6 +28,8 @@ Web 命令行由一对落库的[命令生命周期事件](../../proposed/archite **由这一行从落定文案里剥掉开头的命令名。** 呈现层会悄悄改写 handler 写就的文案，而任何换一种措辞表达结果的 handler 都会让这套启发式失效。 +**彻底禁止命令名出现在自己的落定文案里，并把 `/plan`、`/goal` 一并改写。** 这种更宽的禁令代价大于收益：无论在行上还是作为独立的 TUI 通知，`Plan mode off.` 与 `Goal cleared.` 都是这些结果最清楚的句子，而满足"禁名字"所需的缩写（`off.`、`cleared.`）读起来只是残句。值得去掉的冗余是题头。 + ## Consequences -每一条命令行都变短了，而且这条规则可扩展：新命令的作者写结果时无需知道由哪个界面渲染，任何界面也都不必再去重。代价是分派参数离开了折叠行——命令仍在执行时，行上只有名字和 `执行中…`——以及"不重复"这条规则是靠评审执行的约定，而非门禁。`/permission` 的文案由 permission 包的命令测试钉住，装配后的行文案由 [seeded-history](../../../../apps/web/tests/snapshots/seeded-history/command-row.expected.md) web 预期输出钉住：因为 `/permission` 完全在 host 上执行，它能无密钥地抵达一条真实的落定命令行。 +每一条命令行都变短了，而且这条规则可扩展：新命令的作者写结果时无需知道由哪个界面渲染，任何界面也都不必再去重。代价是分派参数离开了折叠行——命令仍在执行时，行上只有名字和 `执行中…`——以及"不加题头"这条规则是靠评审执行的约定，而非门禁。`/permission` 的文案由 permission 包的命令测试钉住，装配后的行文案由 [seeded-history](../../../../apps/web/tests/snapshots/seeded-history/command-row.expected.md) web 预期输出钉住：因为 `/permission` 完全在 host 上执行，它能无密钥地抵达一条真实的落定命令行。 diff --git a/apps/web/tests/seeded-history.e2e.ts b/apps/web/tests/seeded-history.e2e.ts index 39d450f55c..44e0c67ea5 100644 --- a/apps/web/tests/seeded-history.e2e.ts +++ b/apps/web/tests/seeded-history.e2e.ts @@ -163,9 +163,12 @@ describe('web e2e: seeded history renders through cold resume', () => { await page.getByRole('button', { name: 'Access mode, current: Danger Full Access' }).click() await page.getByRole('menuitem', { name: 'Workspace Write' }).click() await page.getByRole('button', { name: 'Access mode, current: Workspace Write' }).waitFor({ timeout: 10_000 }) - await expect.poll(() => page.getByText('preset workspace-write', { exact: true }).count(), { timeout: 10_000 }).toBe(1) - expect(await page.getByText('permission', { exact: true }).count()).toBe(1) - expect(await page.getByText('/permission workspace-write', { exact: true }).count()).toBe(0) + // Scoped to the row itself, so unrelated page text that happens to read + // `permission` (a future resident slash menu) cannot satisfy or break it. + const row = page.locator('[data-variant="others"]').filter({ hasText: 'preset workspace-write' }) + await expect.poll(() => row.count(), { timeout: 10_000 }).toBe(1) + expect(await row.getByText('permission', { exact: true }).count()).toBe(1) + expect(await row.getByText('/permission workspace-write', { exact: true }).count()).toBe(0) const snapshot = (await captureStableAria(page, '[class*="centerCol"]', scaffold.workspaceCwd)) .split(SEED_ID).join('{{seededId}}') await compareOrRefreshGolden(COMMAND_ROW_EXPECTED, snapshot, MODE) diff --git a/packages/client/connection/src/client/fixture.ts b/packages/client/connection/src/client/fixture.ts index 2278e4545f..7a1e91989e 100644 --- a/packages/client/connection/src/client/fixture.ts +++ b/packages/client/connection/src/client/fixture.ts @@ -1333,7 +1333,7 @@ export function createFixtureApi(options: FixtureOptions = {}): ApiProxy { const current = permissionSelectOf(logOf(id)).currentValue append(id, { type: 'command/done', data: { commandId, kind: 'success', text: `current preset ${current} (available: ${Object.keys(PERMISSION_PRESETS).join(', ')})` } }) } else if (spec === undefined) { - append(id, { type: 'command/done', data: { commandId, kind: 'error', text: `unknown permission preset ${JSON.stringify(preset)} (available: ${Object.keys(PERMISSION_PRESETS).join(', ')})` } }) + append(id, { type: 'command/done', data: { commandId, kind: 'error', text: `unknown preset "${preset}" (available: ${Object.keys(PERMISSION_PRESETS).join(', ')})` } }) } else { if (permissionSelectOf(logOf(id)).currentValue !== preset) append(id, { type: 'permission/preset', data: { preset } }) append(id, { type: 'sandbox/mode', data: { mode: spec.sandbox } }) diff --git a/packages/ui/permission/src/index.ts b/packages/ui/permission/src/index.ts index b9f85ff60d..5c18a2b560 100644 --- a/packages/ui/permission/src/index.ts +++ b/packages/ui/permission/src/index.ts @@ -211,16 +211,16 @@ export class PermissionService extends Service { name: 'permission', description: 'Switch the permission preset (sandbox mode + approval policy)', input: { hint: '' }, - // The settlement text never repeats the command's own name: a surface - // that renders `name · text` (the web command row) would otherwise - // read `permission · Permission preset: workspace-write.` + // No settlement text labels its value with this command's own name: a + // surface that renders `name · text` (the web command row) would + // otherwise read `permission · Permission preset: workspace-write.` handler: ({ agent, rawInput }) => { const name = rawInput.trim() if (name === '') { return { kind: 'success', text: `current preset ${this.current(agent.session.events)} (available: ${this.names.join(', ')})` } } if (!this.names.includes(name)) { - return { kind: 'error', text: `unknown permission preset "${name}" (available: ${this.names.join(', ')})` } + return { kind: 'error', text: `unknown preset "${name}" (available: ${this.names.join(', ')})` } } this.set(agent.session, name) return { kind: 'success', text: `preset ${name}` } diff --git a/packages/ui/permission/tests/projection.spec.ts b/packages/ui/permission/tests/projection.spec.ts index 0b057ce91c..1649fe7077 100644 --- a/packages/ui/permission/tests/projection.spec.ts +++ b/packages/ui/permission/tests/projection.spec.ts @@ -110,7 +110,13 @@ describe('/permission command', () => { const { ctx, session } = await harness() const agent = await agentFor(ctx, session) const execution = await ctx.commands.execute(agent, '/permission yolo', new AbortController().signal) - expect(execution?.result).toMatchObject({ kind: 'error' }) + // The error text carries the same no-self-labelling rule as the success + // texts: `permission · unknown preset "yolo" (…)`, not `unknown permission + // preset`, which the row's own title already says. + expect(execution?.result).toEqual({ + kind: 'error', + text: 'unknown preset "yolo" (available: workspace-write, danger-full-access)', + }) expect(session.events.filter(event => event.type !== 'command/run' && event.type !== 'command/done')).toHaveLength(0) }) })