Skip to content

Make agent approval cards readable at a glance - #175

Merged
sudoHG merged 3 commits into
mainfrom
sudoHG/173-readable-approval-cards
Oct 9, 2026
Merged

sudoHG merged 3 commits into
mainfrom
sudoHG/173-readable-approval-cards

Conversation

@sudoHG

@sudoHG sudoHG commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Closes #173
Closes #174

Summary

Approval cards name the requester and the object in the title, show what will happen in fixed, plain-language sections, and state each button's consequence. Read cards show the full command (scrolling after 4 lines), a heading with the item count and arrow rows for what the command receives, the unverified purpose, and the scope of the timed allowance. Write cards show the purpose, a change summary that names what changes, item rows with exact status tags and masked values, the instructions with a word-level diff, and the group; create and delete state their consequence under the primary button. Organize steps read as sentences, with No change and Merge tags and hidden counts. The card body scrolls under a 640-point cap with a persistent scroll bar and an overflow line that names the hidden sections, so the actions and footer stay visible. The pending list uses the same verbs.

For #174, the App-side write summary carries one vault-keyed value digest per component, so a modify card tags each item exactly. This is risk:security: approval digests, retransmission, consumption, commit and every Broker response are unchanged (see Item 1 below).

Diff stat

git diff --stat origin/main: 27 files changed, 3163 insertions(+), 1733 deletions(-)

  • Sources/AskKeyAppKit/App + Views/Credentials/PendingRequestPresentation.swift: 12 files, +1385 / −423 (helpers next to the cards: ApprovalCopy.swift, ApprovalScrollArea.swift, ApprovalCardComponents.swift, ApprovalTextDiff.swift; largest card file 264 lines)
  • Localizable.xcstrings: +801 / −787
  • Sources/AskKeyBroker/CredentialComponents.swift, Sources/AskKeyVault/Vault+AgentWriteFreeze.swift: 2 files, +22 / −2 (Show which components change in a modify approval #174)
  • Tests: 12 files, +955 / −521

Second review (41ecea4..9670fa8): 22 files changed, 1092 insertions(+), 573 deletions(-).

Second review changes (9670fa8)

  1. Exact per-item tags (Show which components change in a modify approval #174, risk:security). BrokerCredentialComponentSummary gains valueDigest: String?, documented as App-side only. Vault+AgentWriteFreeze.swift componentSummary fills it with HMAC-SHA256 over a length-prefixed encoding of the component value (text, or filename plus bytes). The HMAC key is derived from the vault key with HKDF, using info AskKey approval component value digest v1. Without the vault key a digest cannot confirm a guessed value, and it is stable across freezes. The summary type is not Codable and is returned only by frozenAgentWriteSummary to the App. The approval digest still hashes the aggregate beforeDigest/afterDigest, instructions, group and createsGroup; nothing in retransmission, consumption or commit reads the new field. Cards tag items as Unchanged, Replaced, New, Changed (delivery or masking only, same value) or Removed; May be replaced is gone, and only new or replaced values are listed. A summary without digests falls back to Replaced, never Unchanged.
  2. Instruction diff: removed words in red with strikethrough, added words in green, plus 删掉了:「…」 / Removed: “…”. It uses a weighted LCS (words over punctuation and spaces) after trimming the common start and end; Chinese characters count as words.
  3. Short create cards fit: values are merged into 凭证内容(N 项,值由 {请求方} 提供) / Items (N, values provided by {Requester}) with masked dots at the row end and one 验证后查看 / Authenticate to View on the header. The create and delete consequences sit under the primary button. Overflow lines name hidden sections (下面还有:分组、详细信息 / Below: Group, Details) and still count steps on organize cards. Read cards shrink the icon to 40 points before their body scrolls. The one-item and two-item create cards (06/07) fit without scrolling in both languages.
  4. Read rows: 批准后这个命令会拿到「X」里的 N 项 / If you allow, this command receives N items from “X”, then → 环境变量 X / → Environment variable X and → 临时文件(路径在 X,最多 5 分钟后删除).
  5. Copy:
    • Change summaries name items and sections (会改动:TOKEN 的值 · 不变:给 Agent 的使用说明、分组). The new timed scope also appears under the timed retry. The overwrite line sits under each replaced row.
    • No "delivered" / 交付 wording remains: I checked all 117 keys used by the cards.
    • 「X」的值 / "the value of “X”" stays on one line.
    • English sublines are capitalised.
    • The delete button reads Move to Recycle Bin, the App's existing key.
    • New footer (English uses the sidebar's "Pending requests").
    • The merge step starts with what the requester asked for. The rename detail says 改名时组里有 N 个凭证.
    • The section-level 已修改 tag on items is dropped.

Tests

Head 9670fa8, base 425e29e. Task build directory, deleted after use.

  • swift build: Build complete.
  • Vault, swift test --filter "AgentWriteComponentDigestTests|AgentCredentialMetadataWriteTests|AgentTextWriteCreationTests|AgentTextWriteApprovalLifecycleTests|AgentTextWriteConcurrencyTests|AgentTextWriteBrokerSocketTests": 26 passed, 0 failed (digest 2, metadata 12, lifecycle 6, creation 4, socket 1, concurrency 1). In AgentWriteComponentDigestTests, one same-length component of a three-item bundle is rotated: only its digest changes, and the others' digests and all sizes and deliveries are equal. The digests are keyed (not the plain SHA-256 of the value), stable across freezes, unaffected by a delivery change, and absent from the encoded Broker reply.
  • App, the 11 affected suites (ApprovalPromptContentTests|FrozenWriteSummaryContentTests|OrganizationApprovalContentTests|ApprovalCardLayoutTests|WorkspaceInteractionTests|LocalizationUnificationTests|ScreenPresentationTests|Batch4SettingsLanguageTests|LocalizationRemediationTests|WorkspacePrototypeContractTests|AppLanguageCatalogTests): 78 passed, 0 failed. FrozenWriteSummaryContentTests.testVaultRotationOfOneSameLengthItemShowsReplacedAndUnchanged builds the summary with a real Vault and asserts Unchanged for USER and Replaced for TOKEN, with only TOKEN listed. ApprovalCardLayoutTests.testShortCreateCardsFitWithoutScrolling covers cards 06/07 in both languages.
  • swift test --filter AskKeyAppTests: 371 passed, 0 failed, 0 skipped (main: 368 test methods in this target).
  • Tests/AskKeyE2ETests/*.swift: type-checked with swiftc -typecheck (exit 0). Not run locally per AGENTS.md.
  • CI on 9670fa8, run 37881786563:
    • build-and-test: green. swift test ran 1097 tests, 3 skipped, 0 failures; main 425e29e has 1092, 3 skipped. Automation: 183 ran. Hygiene and module checks passed.
    • basic-ui-flows: green. Required flows passed 15, failed 0, skipped 0. The screenshot round (ScreenshotE2ETests) passed 5, failed 0, skipped 0, and exported 17 screenshots.
    • Inspected approval screenshots 12–17 from the artifact:
      • 12–14 (read): the "receives 1 item from “Staging API”" heading with its → Environment variable row, the new timed scope, the cancelled note without delivery wording, and the "Authenticate and Allow Once" retry.
      • 15 (create): fits without scrolling. It shows Items with the provider, the masked value, the header "Authenticate to View", and the consequence under Create Credential.
      • 16–17 (organize): the merge step opens with what the requester asked for, and the rename detail reads "When it's renamed…". The overflow line shows "1 more step — scroll to see it" before scrolling and "Reached the end" after.
      • On every card the footer and actions are fully visible.

Checks

  • python3 scripts/check_hygiene.py: Hygiene rules passed: size=0, test-support=0, debug=0, local-path=0, non-ascii-name=0, multica=0, cjk=0.
  • python3 scripts/check_module_deps.py: Module dependency check passed.

Deviations and questions

  1. Data limits (Make agent approval cards readable at a glance #173): read cards for multi-item credentials still describe their items without names, because the mapping is not opened before approval. AppDelegate+Approval.swift is untouched.
  2. 640-point cap (within the 680-point limit): at 680, the footer of a full-height card slid behind the Dock on the 768-point CI screen.
  3. One scrolling body instead of separately capped regions; the command keeps its own 4-line cap.
  4. Names wrap as a whole using U+2060 word joiners and no-break spaces; the E2E checks build the same form.
  5. Catalog keys: the string-symbol generator treats keys that differ only by case or word order as the same symbol, so some keys are prefixed and listed as aliases in LocalizationUnificationTests. These are Quoted name: %@, Sentence separator, New group tag, Credential value: %@ (the value of %@ / %@的值), Removed phrases: %@ (Removed: %@) and Change summary: instructions (instructions for agents).
  6. Copy not spelled out in the review:
    • Shorter English for two lines, so the short create cards keep a margin; the Chinese is unchanged. They are "Saved as is if you approve. Viewing needs another authentication; it doesn't approve." and "Its agent permission will be Ask every time, so agents need your approval for each use."
    • Summary phrases for added, removed and delivery-only items: 新增 X / X added, 移除 X / X removed, X 交给程序的方式 / how X is given to programs.
    • Unchanged items are named in full in the summary; nothing is cut.
    • English footer: "Expires in mm:ss and nothing is handed over · Esc to hide; decide in Pending requests before it expires".
  7. docs/adr/0010-agent-credential-metadata-writes.md still names the old Approve Organization button. Docs are outside this Scope.
  8. Local load: I briefly ran one local test build while two other agents' xcodebuild tests were running (three concurrent). Every later build waited until at most one other was running.

sudoHG added 2 commits October 9, 2026 10:06
Titles name the object (credential, value or groups) and the requester.
Read cards show the full command, what it receives and the unverified
purpose; write cards use fixed sections with a change summary, status
tags, plain-language item rows and a value box only for new or replaced
values; organize steps read as sentences with no-change and merge tags.

The card body scrolls under a 680-point cap with a persistent scroll bar
and an overflow line, so actions and the footer always stay visible.
Buttons and the footer state each decision's consequence, the pending
list uses the same object-naming verbs, and the catalog carries English
and Simplified Chinese copy.

Display and copy only: the Broker, Vault, approval digests and summaries
are unchanged.
At the 680-point cap the panel's top sits under the menu bar and the
footer slid behind the Dock on the 768-point CI screen. A 640-point cap
keeps the footer visible while staying within the 680-point limit.
@luoji-bot

luoji-bot Bot commented Oct 9, 2026

Copy link
Copy Markdown

Second review, from an image-only re-review of 16 cards (English and Simplified Chinese) rendered from 41ecea4. Five of the six first-level findings are resolved. Changes requested before merge:

  1. Exact per-item tags (folds in Show which components change in a modify approval #174). Stop showing 可能已替换 / May be replaced. It currently tags untouched items, such as API_KEY when only RECOVERY_CODE is added, and lists them under "new values from Claude Code". Add an App-side value digest per component to BrokerCredentialComponentSummary, computed in Vault+AgentWriteFreeze.swift componentSummary. It must never appear in a Broker response. The card then tags each item exactly as 不变 or 替换, and the value box lists only new or replaced items. Add a Vault test where one same-length component of a bundle is rotated: that item is Replaced and the others are Unchanged. This PR closes Show which components change in a modify approval #174 as well.
  2. Instruction diff. In 给 Agent 的使用说明, highlight word-level changes: removed text in red with strikethrough, added text in green. When something is removed, add a line 删掉了:… / Removed: … with the removed sentence or phrase.
  3. Short create cards fit without scrolling.
    • Merge 要保存的值 into 凭证内容: one row per item, masked dots at the row end, and one 验证后查看 / Authenticate to View on the section header. Its heading becomes 凭证内容(N 项,值由 {请求方} 提供) / Items (N, values provided by {Requester}).
    • Move the 批准后 consequence below the primary button, outside the scroll area, styled like the 30-minute scope line. The same applies to delete.
    • Scroll hints name the hidden sections: 下面还有:批准后、详细信息 / Below: After you approve, Details.
    • Shrink the icon on read cards when they overflow.
    • The cards in 06 and 07 (one or two items, short instructions, a group) must fit without scrolling.
  4. Read card rows drop the repeated credential name. Use the heading 批准后这个命令会拿到「X」里的 N 项 / If you allow, this command receives N items from “X”, with rows such as → 环境变量 DEPLOY_HOST and → 临时文件(路径在 SSH_KEY_FILE,最多 5 分钟后删除).
  5. Copy:
    • Change summaries name what changes: 会改动:API_KEY 的值 · 不变:给 Agent 的使用说明、分组. Never write "凭证内容" or "items" in summaries.
    • Timed scope: 30 分钟内,你这个 Mac 账户下的任何 Agent 或命令读取这个凭证都不再询问;修改或删除它仍要你批准。 / For 30 minutes, any agent or command in your Mac account can read this credential without asking. Changing or deleting it still needs your approval. Keep this line under the timed retry button too.
    • Overwrite line: 批准后旧值会被覆盖,无法找回。 / If you approve, the old value is overwritten and can't be recovered. It goes directly under each replaced row.
    • Remove the remaining 交付 / delivered wording, for example in the cancelled-authentication note.
    • Keep the credential name together with 的值 when a title wraps.
    • English sublines start with a capital letter.
    • Use the App's own name for the bin in both languages: 回收站 / Recycle Bin, so the button reads Move to Recycle Bin.
    • Footer: mm:ss 后自动作废,不会交出任何东西 · Esc 先收起,过期前可在「待处理请求」里决定, with English matching the sidebar's exact item name.
    • Merge step: start with Claude Code 请求的是把「Old」改名为「Archive」;「Archive」已存在且对它隐藏,所以实际会合并。 Then give the result.
    • Rename count: 改名时组里有 N 个凭证…
    • Drop the section-level 已修改 tag when row-level tags are present.
  6. Not now: summarizing many similar organize steps (follow-up).

Scope extension for item 1: Sources/AskKeyBroker/CredentialComponents.swift (summary struct), Sources/AskKeyVault/Vault+AgentWriteFreeze.swift (summary construction only, no change to approval digests or commit), and Vault tests. Item 1 is risk:security.

Give each component of the App-side write summary a vault-keyed value
digest, computed when the write is frozen. The approval digest,
retransmission, consumption and commit are unchanged, and the digest
never appears in a Broker response. Modify cards now tag every item as
Unchanged, Replaced, New, Changed or Removed and list only new or
replaced values.

Instruction edits are highlighted word by word with a Removed line.
Values merge into the items section, consequences sit under the primary
button, overflow lines name the hidden sections, and read cards shrink
their icon before scrolling, so short create cards fit without
scrolling. Read rows no longer repeat the credential name, and the
summary, timed-scope, overwrite, footer, bin and merge copy follow the
second review.
@sudoHG
sudoHG merged commit 57c614a into main Oct 9, 2026
2 checks passed
@sudoHG
sudoHG deleted the sudoHG/173-readable-approval-cards branch October 9, 2026 05:35
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.

Show which components change in a modify approval Make agent approval cards readable at a glance

1 participant