Skip to content

refactor(settings): derive the permission vocabulary from one declaration - #2625

Merged
thymikee merged 2 commits into
mainfrom
t3code/implement-issue-2614
Sep 15, 2026
Merged

thymikee merged 2 commits into
mainfrom
t3code/implement-issue-2614

Conversation

@thymikee

@thymikee thymikee commented Sep 15, 2026

Copy link
Copy Markdown
Member

Summary

packages/contracts/src/settings.ts declares the settings permission vocabulary once — actions, modes, app-scoped targets, macOS targets — and the two parsers, the permission fragments of settings help and its invalid-args message, the public client permission types (via import type, so AppError stays off the client's runtime path) and the CLI's membership sets are built from it.

Deleted: two PermissionTarget unions, a module-local target array, 16 parser comparisons, two hand-written usage fragments, the CLI's 19 literal permission names, a duplicated PermissionState alias, and the macOS fragment restated in cliDetail. Accepted names, case and whitespace normalization, error strings, help ordering and daemon positionals are unchanged; the mobile parser still refuses the macOS-only names, so nothing widened. Actions and modes are derived too — same duplication class, same owned files. The macOS <grant|reset> stays a literal: which actions macOS serves is a support fact owned by platform-apple.

Part of #2614. #2363 is still open, so this adds no all; when it lands, all is one name in MOBILE_PERMISSION_TARGETS plus the fixed test literals.

Validation

3aa6b46fa0: pnpm check:affected --run green — 3017 tests, with format, lint, typecheck, layering, fallow and build. check:production-exports and check:command-docs report nothing in the changed files.

Characterization landed first, from fixed literals duplicated per test file so one edit cannot silence every witness. Issue replay: adding face-time to MOBILE_PERMISSION_TARGETS alone moved the parser, CLI membership, help fragment, cliDetail and the client type, and the fixed pins then failed (three runtime, two toEqualTypeOf) rather than accepting it silently. Removed before the commit. A planted value import of ./settings.ts in client-settings.ts fails its canary.

Coverage, Integration and the device replay lanes stay CI-authoritative on this head; no device evidence is claimed, as no platform or runner code changed.

Rebased onto main, which is itself red at 2cafab3ad0: installArgs in snapshot-helper-install.test.ts:29 no longer exists on AndroidSnapshotHelperManifest (#2619 copied the fixture after #2618 deleted the field). The second commit drops that stale field — nothing parses it, and the leniency case stays covered in snapshot-helper.test.ts:913. It is outside the #2614 family and can be split out; without it the typecheck gate here cannot run.

@github-actions

github-actions Bot commented Sep 15, 2026

Copy link
Copy Markdown

Size Report

Metric Base Current Diff
Installed (including dependencies) 4.56 MB 4.56 MB +711 B
Package (unpacked) 4.56 MB 4.56 MB +711 B
Package (download) 1.35 MB 1.35 MB +278 B

Startup median (7 runs, lower is better):

Scenario Base Current Diff
CLI --version 29.5 ms 28.2 ms -1.4 ms
CLI --help 82.4 ms 80.9 ms -1.6 ms

@thymikee

Copy link
Copy Markdown
Member Author

Reviewed at ac6fc28. The code looks good: the permission vocabulary comes from one declaration, and the parsers, help fragments and client types derive from it without changing accepted names or errors. Smoke Tests runs 01-settings.ad on real Android and iOS targets, and 3 of 4 matrix jobs were green at review time.

Typecheck & Package fails with TS2353 on installArgs in packages/platform-android/src/__tests__/snapshot-helper-install.test.ts. This PR does not touch that file, and main fails the same way at 2cafab3 because #2618 removed the field while that test still sets it, so the failure is unrelated to this PR. A rebase will not clear it until main is fixed.

@thymikee thymikee added the ready-for-human Valid work that needs human implementation, judgment, or maintainer merge label Sep 15, 2026
…tion

packages/contracts/src/settings.ts now declares the settings permission
actions, modes, app-scoped targets and macOS targets once, and the two
parsers, the settings help fragments and its invalid-args message, the
public client permission types, and the CLI's membership sets are built
from those collections.

Accepted names, normalization, error strings, help ordering and daemon
positionals are unchanged; no surface widened what it accepts. The macOS
action list stays a literal because it is a support fact owned by
platform-apple.

Part of #2614.
#2619 copied the snapshot helper manifest fixture after #2618 deleted
`installArgs` from AndroidSnapshotHelperManifest, so the new fixture failed
typecheck on main: the field is parsed by no one and the leniency case is
already covered in snapshot-helper.test.ts.

Repair for the breakage on main (2cafab3), unrelated to the settings
vocabulary refactor it blocks here.
@thymikee
thymikee force-pushed the t3code/implement-issue-2614 branch 2 times, most recently from ac6fc28 to 3aa6b46 Compare September 15, 2026 09:52
@thymikee
thymikee merged commit aba54b4 into main Sep 15, 2026
17 of 18 checks passed
@thymikee
thymikee deleted the t3code/implement-issue-2614 branch September 15, 2026 10:06
@github-actions

Copy link
Copy Markdown
PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-09-15 10:06 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-human Valid work that needs human implementation, judgment, or maintainer merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant