From c5b84fb5f4648d7123d045f61838dd64cf8882d8 Mon Sep 17 00:00:00 2001 From: Devvyn <22340871+opticon454@users.noreply.github.com> Date: Wed, 2 Sep 2026 09:35:55 +0800 Subject: [PATCH] docs(cli-registry): annotate overlays.credStore as declared-for-later MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review item 4 named THREE live tables duplicating registry data. Two are now read from the entry (`defaultRemoteCommandForMode`, `defaultDockerCommandForMode`); the third, `resolveDockerCredentialArtifacts`, is not — and it was left neither wired nor annotated, which is the state that item explicitly rules out. It is not wired because the shape cannot express the live table: `credStore` is ONE store per CLI, and `CRED_STORES` needs two for gemini (`.gemini` for the CLI's own auth plus `.config/gcloud` for Vertex), while deepseek's entry declares none at all even though `.dsh` is seeded. Wiring it means making the field an array and correcting those two entries — a change to credential seeding, which is at once the worst thing in that file to get wrong and the least covered by tests, since every docker IO path is no-op'd under vitest. It belongs in its own change, measured against a real container. So it is annotated instead, at the field, in the type's declared-for-later header, in docs/cli-registry.md, and in the pinned DECLARED_FOR_LATER list — the last of which means wiring it later makes a test fail rather than leaving a stale comment behind. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WQkoi1cNegqVwZHgzx5SbJ --- docs/cli-registry.md | 2 ++ src/config/cli-registry/types.ts | 15 ++++++++++++++- test/cli-registry-no-id-branching.test.ts | 3 +++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/docs/cli-registry.md b/docs/cli-registry.md index a4d29161..2717cf86 100644 --- a/docs/cli-registry.md +++ b/docs/cli-registry.md @@ -103,6 +103,8 @@ This matters because it is invisible when it is wrong. `capabilities.privilegedP Treat those values as **transcribed, not authoritative** — nothing enforces that `echo.policy` matches `_updateLocalEchoState`'s fallthrough, or that `accent` matches the gradient CSS paints, so re-measure before wiring one up. A field that is both wrong and unread is worse than an absent one, because the next reader trusts it; `test/cli-registry-no-id-branching.test.ts` pins the list so it cannot quietly grow, and wiring one up makes its line there fail, which is the direction you want. +`overlays.credStore` is in the same category, for a sharper reason: the Docker credential-seeding path still reads its own `CRED_STORES` table, because this shape allows ONE store per CLI and the live table needs two for gemini (`.gemini` for the CLI's own auth plus `.config/gcloud` for Vertex), while deepseek declares none here even though `.dsh` is seeded. Wiring it means making the field an array and correcting those two entries — a change to credential seeding, which is simultaneously the worst thing here to get wrong and the least covered by tests, since every docker IO path is no-op'd under vitest. + Everything else in the interface is live, including `overlays.remote` / `overlays.docker`, which back `defaultRemoteCommandForMode()` and `defaultDockerCommandForMode()` directly. Those two used to be hardcoded `Record<…CommandMode, string>` tables duplicating the registry with nothing keeping the two in step; `test/location-overlay-commands.test.ts` pins every resulting command as a literal string. ## Resolve at call time, never at import diff --git a/src/config/cli-registry/types.ts b/src/config/cli-registry/types.ts index 4eb603be..deb2305b 100644 --- a/src/config/cli-registry/types.ts +++ b/src/config/cli-registry/types.ts @@ -443,6 +443,19 @@ export interface CliOverlays { */ remote?: { command?: string } | { disabled: true }; docker?: { command?: string } | { disabled: true }; + /** + * ⚠️ DECLARED-FOR-LATER, unlike `remote`/`docker` above, which are live. + * + * The Docker credential-seeding path still reads its own `CRED_STORES` table in + * `docker-hosts.ts`, because this shape cannot yet express that table: it allows ONE store + * per CLI, and the live table needs two for gemini (`.gemini` for the CLI's own auth plus + * `.config/gcloud` for Vertex), while deepseek's entry here declares none at all even + * though `.dsh` is seeded. Wiring it therefore means making this an ARRAY and correcting + * those two entries — a change to credential seeding, which is both the highest-consequence + * thing in this file to get wrong and the least covered by tests, since every docker IO + * path is no-op'd under vitest. It belongs in its own change, measured against a real + * container. + */ credStore?: CliCredStore; } @@ -453,7 +466,7 @@ export interface CliOverlays { /** * ⚠️ DECLARED-FOR-LATER: fields no code reads yet. * - * `shortBadge`, `accent`, `capabilities.echo`, `capabilities.wheelForward`, + * `shortBadge`, `accent`, `overlays.credStore`, `capabilities.echo`, `capabilities.wheelForward`, * `capabilities.keyboardAccessory` and `capabilities.maxFrameBytes` all describe FRONTEND * behaviour, and the frontend is deliberately untouched by the change that introduced this * registry — `app.js`, `terminal-ui.js`, `styles.css` and friends keep their own diff --git a/test/cli-registry-no-id-branching.test.ts b/test/cli-registry-no-id-branching.test.ts index 5bde799f..06cad742 100644 --- a/test/cli-registry-no-id-branching.test.ts +++ b/test/cli-registry-no-id-branching.test.ts @@ -315,6 +315,9 @@ describe('declared-for-later fields', () => { 'capabilities.wheelForward', 'capabilities.keyboardAccessory', 'capabilities.maxFrameBytes', + // The Docker credential-seeding path still reads its own CRED_STORES table: this shape + // allows ONE store per CLI and the live table needs two for gemini. See CliOverlays. + 'overlays.credStore', ]; /** Read every `.ts` under src/, minus the registry itself (which of course names them). */