fix(ui): drop the caption from the /permission error text too
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.
This commit is contained in:
creatixchu
+2
-2
@@ -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
|
||||
@@ -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 `<Command> <noun>:` 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.
|
||||
@@ -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 上执行，它能无密钥地抵达一条真实的落定命令行。
|
||||
@@ -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)
|
||||
|
||||
@@ -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 } })
|
||||
|
||||
@@ -211,16 +211,16 @@ export class PermissionService extends Service {
|
||||
name: 'permission',
|
||||
description: 'Switch the permission preset (sandbox mode + approval policy)',
|
||||
input: { hint: '<preset>' },
|
||||
// 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}` }
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user