Let agents propose credential instructions and groups through approved writes

[sudoHG]

Closes #161

Summary

• Agents can propose optional usage instructions and groups through create_credential and modify_credential, including metadata-only modification, clearing instructions and moving to Ungrouped. Catalog entries expose visible groups.
• Freeze complete before/after metadata, normalize group lookup with NFC/case folding, bind both fields to approval, and commit new groups with the credential and operation receipt in one transaction. Every write still requires approval and separate system authentication, including Allow credentials and timed allowances; Hidden credentials retain their generic rejection.
• The approval card follows the operation: create shows proposed values, delete shows current values, and modify compares changed fields while showing unchanged fields once with an Unchanged / 不变 note. Instructions and groups fit their content up to 60 and 30 points, then scroll; new groups are marked and actions fit the 680-point budget in English and Simplified Chinese. Delete omits the frozen-content heading, reveal button and authentication note; create and modify retain them.
• ADR 0010 amends ADR 0002 and records batch group rename/deletion for follow-up Let agents organize credentials and groups in one approved batch #162. Add synthetic behavior tests and a CI-only real MCP approval screenshot scenario.

Both review follow-ups change presentation and tests only. The approval digest, BrokerCredentialWriteSummary, freeze and commit behavior are unchanged, and create/modify reveal state and authentication behavior are preserved. Component equality uses the existing material digest as well as the existing summaries, so replacing a value with another of the same length does not incorrectly display Unchanged. No UI string changes were needed in the second follow-up.

Diff stat

git diff --stat origin/main...HEAD: 26 files changed, 1166 insertions, 89 deletions, from merge base e7e77ecdc294f6aa86284e8b98215d4f04ebec77. Main advanced to 6811d1ea8992b24229b0ac3297fba5e2a9aabcde with release-packaging changes during review; the PR diff excludes those unrelated changes. Scope covers Broker types/catalog, Vault freeze/transaction, helper schema/parsing/guidance, approval presentation/localization, scoped tests, the screenshot export allowlist and ADR/features documentation. The second follow-up compared with f260ec4307707b11ed66981b7cd8f2ce565fd875 changes only the approval card and related App tests: two files, 120 insertions and 36 deletions.

Tests

Final candidate: b4e7d166bc6454c61346836538fd492d34899d71.

swift build --scratch-path .build-161-review2 --jobs 4
swift test --scratch-path .build-161-review2 --jobs 4 --filter 'FrozenWriteSummaryContentTests|ApprovalMetadataContentTests|ApprovalPromptContentTests|LocalizationUnificationTests'
python3 scripts/check_hygiene.py && python3 scripts/check_module_deps.py
git diff --cached --check

Final local verification passed (exit 0). The affected App selection passed 28 tests, 0 failed, 0 skipped: metadata/rendering 7, operation-aware presentation 7, prompt 6 and localization 8. The three new tests each cover English and Simplified Chinese: actual rendered height grows with short instructions and groups, reaches the existing caps and stays stable for overflow; delete removes the complete frozen-content block while create and modify retain its full measured height. These are offscreen unit layout checks, without presenting a window or capturing an image. The existing group-height test now allows short content to shrink while verifying the same cap.

The seven operation-aware tests still pass both language parameter cases (14 combinations), covering create, delete, metadata-only modify, value-only modify, clearing metadata, same-length value replacement and missing digests. The 680-point prompt height tests and the check that metadata rendering never invokes reveal authentication passed.

Earlier implementation verification remains applicable to unchanged Broker/Vault/helper code: the broad synthetic selection passed 83 tests at 03f1e7586eedfdddc40c9d5e08ad1fa5d81d6136; the affected Vault selection passed 17 tests at b7986c85066700f33f4727e9aa18e5274e93f89c; the affected App selection passed 18 tests at 25eefc9d41fdcb9ca429a165d4cad5f4d6c5b799, all with zero failures and skips. The commands were:

swift test --scratch-path .build-161 --jobs 4 --filter 'AgentCredentialMetadataWriteTests|HelperMCPMetadataTests|ApprovalMetadataContentTests|HelperMCPComponentTests|HelperMCPTests|BrokerCatalogTests|AgentTextWrite|AuditBrokerBoundaryTests|ReviewCatalogLimitTests|HumanTextCredentialTests|ReviewCredentialMutationTests|ApprovalPromptContentTests|ReviewLocalizationTests|LocalizationUnificationTests'
swift test --scratch-path .build-161 --jobs 4 --filter 'AgentCredentialMetadataWriteTests|AgentTextWriteCreationTests|AgentTextWriteConcurrencyTests'
swift test --scratch-path .build-161 --jobs 4 --filter 'ApprovalMetadataContentTests|ApprovalPromptContentTests|LocalizationUnificationTests'

These tests verify byte limits, unchanged material/private metadata, Hidden rejection, metadata approval binding, separate write authentication, transaction rollback/retry and equivalent-group reuse. No local full suite, desktop E2E, app launch or screenshot capture was run.

Final CI run 37763651577 is associated with the candidate SHA. build-and-test and basic-ui-flows both passed. Full Swift suite: 1054 executed, 1051 passed, 0 failed, 3 skipped. All 182 automation tests passed, with zero failures or skips, and every hygiene/module check passed. CI checkout b38bd3954ea5202015f4030a5cb1be7fa3b42f62 merges the candidate into main at 6811d1ea8992b24229b0ac3297fba5e2a9aabcde, including its release-packaging updates.

Compared every XCTest identity and outcome with the second-review baseline CI at f260ec4307707b11ed66981b7cd8f2ce565fd875 (1051 executed, 1048 passed, 3 skipped): all 1051 outcomes are unchanged, all three added tests passed, and there are no missing tests or new skips. Compared with the last green main CI baseline at e7e77ecdc294f6aa86284e8b98215d4f04ebec77 (1025 executed, 1022 passed, 3 skipped): all 1025 outcomes are unchanged after mapping BrokerCatalogTests.testCatalogExcludesHiddenCredentialsAndAllSensitiveMetadata to testCatalogExposesVisibleGroupsAndExcludesHiddenCredentialsValuesAndPrivateNotes, and all 29 added tests passed. Main's subsequent release-packaging commit changes no Swift source or test files.

The same three tests are skipped in both baselines and the final candidate:

• CodexCLIContractTests.testInstalledCodexMCPAddGetJSONContractUsesIsolatedConfiguration: the installed CLI executable/version contract is not configured.
• LegacyFileWriteTransactionTests.testAbruptFileCommitProcess: only its isolated crash subprocess invokes this entry.
• NativeBootstrapFixtureGenerationTests.testGenerateSyntheticNativeBootstrapFixtures: explicit fixture generation only.

Desktop acceptance: 15 required flows passed, 0 failed, 0 skipped. Screenshot suite: 4 passed, 0 failed, 0 skipped. Independently checked result JSON and raw test identities: ScreenshotE2ETests.testMetadataWriteApprovalScreen itself passed, including its create-layout label assertions, a real synthetic MCP write, screenshot capture, clicking Deny and the resulting denied outcome. Its result is verified separately from the job status because the optional screenshot step permits failure.

All 15 PNGs exported. Inspected screenshots/15-approval-credential-metadata.png in the final-head CI artifact: short instructions and the group use their content height with the fixed blank gaps removed; complete proposed text, the new-group marker, the retained create reveal section, Approve Creation and Deny are visible. The artifact expires on October 15 under the existing seven-day retention policy.

Checks

Hygiene has zero violations and module dependencies passed locally. Broker protocol remains version 1. Permission policy, runtime/delivery, the App editor/sidebar and client configurations are unchanged. Only synthetic values and isolated test stores are used. CI also runs the automation checks; the implementation's three synthetic screenshot-export checks passed for complete, missing and duplicate metadata screenshot manifests.

Removed the second follow-up's owned Swift build directory (1,039,843,937 bytes), SwiftPM metadata directory, consumed local/CI logs and inspected CI download (7,843,659 bytes). Root ownership and symlink checks passed; internal SwiftPM links resolved within the owned directory, and lsof showed no open files before removal. No task build/test process or build directory remains, and the worktree is clean. Data volume at cleanup: 82 GiB available. One shared testmanagerd service remains; unrelated services and other tasks' directories were untouched. The open PR's branch/worktree remain for review.

Deviations and questions

The coordinator approved the implementation's additional FrozenAgentTextWrite.swift, VaultStore+AgentWrites.swift, ScreenshotE2ETests.swift, metadata-screenshot branch in Tests/AskKeyTestSupport/E2EBrokerScenario.swift and screenshot export entry in scripts/e2e-report.py; #161 Scope was updated. The App save path already uses the 4 KiB UTF-8 instruction limit, so no limit deviation was required. Group spelling/existence changes while approval is pending fail closed. #162 is documented only; no batch group-management tool is implemented here. The two display follow-ups address all coordinator review points without changing approved write contents or authorization behavior.

[luoji-bot]

Review: changes requested before merge (display only).

The approval card applies one Before/After template to every operation. On a create there is no "before", so the card shows empty "Before" labels and Before rows reading None / Ungrouped; on a delete the "After" rows are meaningless. Make the card follow the operation:

1. Create: show only what will be written: components, usage instructions, and group with the existing new-group marker. No "Before" rows or labels.
2. Delete: show only the current components, instructions and group that will be removed. No "After" rows or labels.
3. Modify: show Before → After only for fields that change. A field that doesn't change appears once with its current value and an "Unchanged" note (Simplified Chinese: 不变).
4. Apply the same rule to the existing component summary rows at the top of the card, not only the new metadata section.
5. Display only: the approval digest, summary contents and commit behavior stay unchanged.
6. Tests: cover the create, delete, metadata-only modify and value-only modify layouts in English and Simplified Chinese. Keep the 680-point height test. The CI metadata screenshot test must pass on the final head; report its own result, because the screenshot step is continue-on-error.

This replaces item 7 in #162, which I am removing there.

[luoji-bot]

Follow-up review (display only), requested by the maintainer before merge:

1. No blank space under short metadata. The usage-instructions and group regions use fixed heights (60 and 30 points), so short instructions or a single group line leave an empty gap, visible on create and delete. Size each region to its content, capped at the current maximum height, and scroll only beyond the cap. Long text must still keep the Approve/Deny actions within the 680-point budget.
2. No "Frozen Content to Write" on delete. A delete writes nothing, and its reveal material is only the credential name. Hide that section, including "Authenticate and View" and its note, for delete approvals. Keep it unchanged for create and modify.
3. Tests: cover short and long instructions (region height follows content up to the cap) and the absence of the frozen-content section on delete, in English and Simplified Chinese. Keep the 680-point test, and require the CI metadata screenshot test itself to pass.