Skip to content

DocumentDB Local with Quick Start helpers - #798

Merged
Tomasz Naumowicz (tnaum-ms) merged 176 commits into
release/0.10.0from
feature/local-quickstart
Aug 6, 2026
Merged

DocumentDB Local with Quick Start helpers#798
Tomasz Naumowicz (tnaum-ms) merged 176 commits into
release/0.10.0from
feature/local-quickstart

Conversation

@tnaum-ms

@tnaum-ms Tomasz Naumowicz (tnaum-ms) commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Add a guided DocumentDB Local Quick Start flow that goes from a Docker-ready machine to an open local connection without leaving VS Code.
  • Add managed instance lifecycle actions and status in the Connections view, including start, stop, restart, delete, recovery, and existing-instance safeguards.
  • Add the Quick Start webview and extension-host services for provisioning, readiness checks, durable state, credential handling, telemetry, and actionable errors.
  • Preserve manually added localhost connections alongside regular connections and detect DocumentDB Local instances added by hand.

Release integration

  • Targets release/0.10.0.
  • Merged the latest release/0.10.0 changes in commit 03125338.
  • Resolved the router conflict by retaining the Quick Start router while adopting the relocated Collection Index router and the release branch toast-procedure split.

Validation

  • npm run l10n
  • npm run prettier-fix
  • npm run lint
  • Jest: 3,396 tests passed
  • npm run build

Signed-off-by: Guanzhou Song <guanzhousong@microsoft.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Guanzhou Song <guanzhousong@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Guanzhou Song <guanzhousong@microsoft.com>
- Add local-quickstart-v2.md: simplified design that removes the dedicated
  local connection subtree, uses a webview for container creation, gates
  TLS exceptions in the regular connection wizard, and migrates legacy
  emulator connections on first launch.
- Mark local-quickstart.md as iteration 1 with a forward reference.
…decision note

- terminal-first create progress for v1.0 (staged in-webview card deferred to v1.1)
- adopt @microsoft/vscode-container-client (Docker+Podman) as the runtime layer
- port fallback only for the default port; read the bound port from docker inspect
- distinguish create-vs-start failure; pre-check connection name AND container name
- scope prereq checks v1.0/v1.1; add section 18.1 PostgreSQL source-benchmark learnings
- add docs/ai-and-plans/PRs/653-local-quickstart-design/description.md decision note

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-agent review

Adds a demo-focused proof-of-concept plan for Local Quick Start under

docs/ai-and-plans/PRs/local-quickstart-poc/:

- description.md: POC goal, scope (focus vs leave-out), deliberate deviations, reuse strategy, real-world image findings

- poc-implementation-plan.md: process contract, architecture map, 14 decisions, 9 work items (Core WI-0..6 + Stretch WI-7..8), risks, demo script

- review-and-resolutions.md: two rounds of 5-agent review (design/manager/impl/scope/risk) with every finding and resolution; consensus reached at rev. 3

No shipping code; planning only.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add @microsoft/vscode-container-client dependency (validates OPEN-3: post-image args supported via runContainer command[])

- ContainerRuntime: isDockerReady/isPortFree/pullImage/createAndRunContainer/inspect/start/stop/remove/listByLabel/followLogs

- Masked, line-buffered OutputChannel writes so the generated password never leaks (D14), incl. split-chunk safety

- quickStartTypes: image/port/label constants, InstanceState, StageEvent, InstanceMetadata

- Unit tests for masking (8 passing)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Standalone service (documented deviation from D13 Task-composition: Task is single-use + numeric-notification progress, ill-fitting the in-webview stage checklist + Retry; D13 permits standalone)

- provision() async generator yields StageEvents (checking/pulling/creating/starting/waiting/done); per-attempt AbortSignal; cleanup in finally (pull-cancel removes nothing; create/start-cancel removes by id, D12)

- wire-protocol readiness probe via connectToClient (TLS-allow-invalid), 180s + backoff (D7)

- credentials: URL-safe alphabet, percent-encoded conn string (D6); stored in SecretStorage

- activation reconcile(): adopt labelled container if creds stored, else clean-remove

- unit tests for credentials/conn-string (16 passing total)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- localQuickStartController (extends WebviewControllerBase; closePanel via this.panel.dispose, not this.dispose)

- localQuickStartRouter: getDockerStatus/getStatus queries, startQuickStart subscription (mirrors ctx.signal to cancel provisioning + docker), closePanel mutation; imports tRPC primitives from ./trpc

- mounted localQuickStart in appRouter; registered LocalQuickStart in WebviewRegistry (auto-bundled)

- openLocalQuickStart command + registration; attach QuickStartService (reconcile + dispose) at activation

- minimal React entry calling getDockerStatus (full UI in WI-3/WI-4)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Review: 4 metric cards (Docker/Port/Data=Ephemeral(POC)/Security) + What-we-do summary + Start

- Docker-not-ready variant with Retry (opt-in; never auto-starts Docker)

- Progress: lightweight staged checklist (D3) driven by startQuickStart subscription + elapsed timer + spinner; Cancel unsubscribes

- Failure: inline error + Retry; View Docker output reveals the channel

- Success: brief card then auto-close via closePanel (panel.dispose) -> tree handoff

- added showOutput mutation; moved DockerStatusResult to pure types module (no router runtime in webview bundle)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-5, WI-6)

- LocalQuickStartItem root node: rocket empty-state row (opens webview) when NotInstalled; inline DocumentDBClusterItem when Running/Stopped (expand to browse); Provisioning/Error rows

- QuickStartClusterItem subclass stamps a static 'Running . localhost:port' description (D2 design review)

- QuickStartService pre-populates CredentialCache (setAuthCredentials) on success + reconcile so the inline item browses without a re-prompt

- ConnectionsBranchDataProvider: render the node UNCONDITIONALLY incl. the zero-connections empty state (mandated option A); prepend in normal path; fix savedConnections telemetry count

- refresh the tree on QuickStartService status change

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- ClustersExtension: refresh Connections tree on QuickStartService status change (WI-6 wiring)

- prettier formatting; eslint-disable for initial-load setState-in-effect (matches DocumentView)

- record standalone-service deviation from D13 + -dt/customOptions note in the plan

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Functional/security/tree/webview/design review fixes:

- D14: followLogs now line-buffered via MaskingLineBuffer (split-secret safety)

- stop followLogs on success: cts.cancel() in finally (was leaking docker logs -f)

- orphan sweep if cancelled during create window (createAttempted + label sweep)

- clear stale ClustersClient on fresh provision so re-run browses with new creds

- webview: emit terminal error when busy + onComplete handler (no stuck 'provisioning'); unsubscribe-before-resubscribe + null-on-terminal (no double-click leak); review-screen Cancel button

- readiness probe: direct MongoClient w/ 3s serverSelectionTimeout (Cancel responsive); remove redundant -t (detached already adds --tty)

- optional sample-doc seed so the tree isn't empty to browse

- Learn more... row; savedConnections telemetry counts real connections/folders

Gates: lint, jest 2055, build, webview bundle all pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…utions

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… Windows

Root cause: ShellStreamCommandRunnerFactory without a shellProvider drops each arg's quoting metadata and sets windowsVerbatimArguments on Windows, splitting Go-template args like '--format {{json .}}' on the space. This made 'docker info' (and inspect/list) fail, so isDockerReady reported the daemon unreachable even when Docker was running.

Fix: provide a platform shell provider (Cmd on Windows, Bash elsewhere) to every runner; switch makeRunner to strict:false (non-zero exit still rejects; harmless stderr warnings don't). Add @microsoft/vscode-processutils dependency.

Verified live on Windows: info, run (with credential args + labels), inspect (bound port), listByLabel, remove all succeed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…s shell-provider)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adopt the image's native sample-data mechanism (documentdb-local --init-data true / INIT_DATA=true) instead of a bespoke driver-side seed — matches design 8.4 (use the image's standard init-script convention; portable, identical to running the image by hand).

- pass '--init-data true' as post-image args alongside --username/--password

- loads the rich sampledb (users/products/orders/analytics) vs the prior single doc

- remove seedSampleData() driver hack

- add capped, non-fatal waitForSampleData() so the tree reliably shows data on first expand (sample data loads just after gateway readiness)

Verified live on Windows: --init-data true populates sampledb.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…shell provider)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Missing badge

Turns the provision-and-browse POC into a managed instance (design §6.2/§11).

- States: add Starting/Stopping; Stopped now reachable; Missing badge (§6.1) when metadata exists but Docker has no container

- Service: start/stop/restart/deleteContainer/refreshLiveState; all destructive ops verify the quickstart label before acting (§9/§13.1); lifecycleBusy guard

- Tree: state-aware rows with a Quick-Start-specific contextValue (treeItem_quickStartInstance + state token) so the instance shows Quick Start actions, not generic cluster menus; live-refresh on expand (cheap multi-window/external-change freshness, §12)

- Commands: Start/Stop/Restart/Delete Container/Copy Connection String/Copy Password/View Logs (inline + context menu, gated by state)

- Delete uses a one-line modal confirm (§11); password masked in View Logs (D14)

Verified live on Windows: run/stop(exited)/start(running)/remove + label check. Gates: lint, jest 2055, build, webpack-prod all pass.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ocker exec)

Baked '--init-data true' re-ran the image's sample-data init on every docker start, hit a duplicate-key error, and crashed the container (set -e); start() also inspected too early and falsely reported Running. Now run with only credentials and seed the built-in sampledb once after readiness via the image's native init script (docker exec), and require the container to stay running (confirmStaysRunning) before declaring Running. Verified live: stop->start stays running and sample data persists.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ased-cryptographic-random)

Use rejection sampling so every URL-safe alphabet character is equally likely; a plain byte % 62 over-represented the first 8 characters. Flagged high-severity by CodeQL on PR #756.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tance, ownership-bounded)

Production v1 design decision for the multiple-instance / existing-container edge case raised by German Eichberger: Quick Start manages exactly one labelled instance it created; users attach their own containers via the regular wizard; multi-instance/adopt/auto-discovery deferred to v1.2 (labels keep the model forward-compatible). Identifies unlabelled name/port collision safety as the one concrete v1 hardening item. No code changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… Docker diagnosis

P0: persistent named volume at /data (data survives recreate; Missing->recreate reuses stored creds+volume; idempotent sample seeding; explicit Delete drops the volume). Credentials now pass via a temp --env-file (mode 600, deleted in finally) as USERNAME/PASSWORD instead of CLI args (verified the image authenticates via env). Port-conflict fallback band [10260,10360) with bound-port readback. P1: DockerReadiness gains host arch/platformSupported; Docker-not-ready view rebuilt into per-check cards + Start Docker Desktop action + Install/Troubleshooting links; review Data card now 'Persistent volume'. Verified live + gates (lint/jest 2055/build/webpack/l10n).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
P1-3: success card adds Open Connection (focuses Connections view) + Copy Connection String, with a longer auto-close so the buttons are usable. P3-2: start/stop/restart re-check live Docker state via liveStateGuard before acting and inform the user if another window already changed it (design §12). P3-1: documentDB.quickstart.provision telemetry (result/reused/portFallback/provisionMs), getDockerStatus reports dockerReadiness+platformSupported, lifecycle commands tag action; no names/ports/creds sent (§14). Gates: l10n/prettier/lint/jest 2055/build/webpack-prod.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…plicit flag (M4, I2-2)

The Configure step relabelled its settings when stored credentials existed, but
the primary button still read 'Start DocumentDB Local' and the footer still said
'Nothing else on your machine is changed' - while provision() force-removed the
running container first. Opening Quick Start out of curiosity destroyed a
running instance and the copy said the opposite.

Configure now ASKS, above the settings table:
  - Use existing data - recreate the container and keep your documents
  - Start fresh - erases all data in DocumentDB Local  (never pre-selected)

The answer is sent to provision() as an explicit AdvancedQuickStartOptions
.startFresh flag instead of being inferred from getReusableCredentials(), and
the primary label plus the footer note follow the selection. Per I2-Q4 there is
no extra confirmation dialog: the destructive option states the data loss in its
own label.

The RR4 / 5.2 volume-wipe gate is preserved - 'Start fresh' is now the only path
that may drop a volume, and it is an explicit user choice. Resolves N1 by
construction: there is no inferred value left to go stale.

DockerStatusResult.willReuse / willReuseExistingInstance() are renamed to
canReuseExistingData - the flag now says what is POSSIBLE, not what will happen.
…2-3)

Reaching the wizard while an instance exists should not normally be possible -
the tree does not link to it in that state - but the command palette, a stale
panel and cross-window races all still get there, and a healthy running instance
could be walked straight into a destructive recreate.

The Configure step (the screen where the decision is actually made, not the
Introduction step) now shows a MessageBar with three variants:

  Healthy (Running)   info    - primary disabled, offers Open Connection / Close
  Stopped             info    - primary disabled, offers Start / Close
  CredentialsMissing  warning - primary enabled, forced onto Start fresh

The guard checks the instance is usable FROM THE CONNECTIONS VIEW, not merely
that a container is running. The service's hard refusal for the credential-
unavailable case (which left the user hunting for a separate Delete Container)
becomes an explicit, warned Start-fresh path inside the wizard; the RR4 volume
gate is unchanged because that is still an explicit choice.

Adds a startInstance mutation to the router for the Stopped variant.
…t rows (I2-4, N3)

The state_error row showed status.errorMessage as its description with NO
command - a passive dead end - and the NotInstalled branch pushed a second,
message-only ${id}/error child (N3).

The tree now renders recovery ACTIONS instead of error text, using the canonical
pattern the rest of the codebase uses: createRetryNode plus companions for the
setup log and, once a container exists, Delete Container. The message-only child
is gone.

Per I2-Q2 the retry node performs a REAL retry - for a provisioning failure that
means reopening the wizard - while a passive refresh reuses the cached error
children. LocalQuickStartItem therefore implements TreeElementWithRetryChildren
so BaseExtendedTreeDataProvider caches the failed state.

Per I2-Q5 this applies to genuine failures only: Missing and CredentialsMissing
keep their own actionable rows and are not classified as cached error states.
…s fixed elsewhere (I2-17)

failedChildrenCache freezes a node's children once it is classified as failed
and returns them without re-fetching. If the user then fixed the underlying
problem outside the tree - typically in the Quick Start webview - the row kept
rendering the stale error node until a manual collapse/expand.

QuickStartService.onDidChangeStatus now resets the Quick Start subtree's error
state before refreshing, mirroring AtlasDiscoveryProvider.onDidChangeSession.

Raised while answering I2-Q3 as the direct consequence of accepting modals from
getChildren().
Annotates every Iteration 2 item with what was done, why, and its commit;
records the three places the implementation departed from the plan (I2-3 moved
to the Configure step, I2-5 needed a cooldown as its loop breaker, willReuse
renamed to canReuseExistingData).

I2-8 is the only item not shipped - promoted to Iteration 3 as I3-1, blocked on
I3-Q1 (permission to rewrite the TDD: persistence assertions).
@tnaum-ms

Copy link
Copy Markdown
Collaborator Author

Iteration 2 of the code review — 9 items shipped, 1 deferred

Worklist: docs/ai-and-plans/PRs/798-local-quickstart/code-review-2026-08-04.md §11.4. One dedicated commit per item; each item's full write-up lives in that chapter next to the decision it implements.

# Commit Outcome
I2-1 ec0a77cb ✅ H5 regression test
I2-2 f473b02c ✅ Recreate-vs-fresh choice + explicit startFresh flag
I2-3 5e2c9314 ✅ Existing-instance guard — on the Configure step
I2-4 a684ce95 ✅ Actionable error nodes (closes N3)
I2-5 1a0f3ab9 ✅ Tree renders from cache instead of blocking on Docker
I2-6 bef2128f suggestPort() skipped on polled readiness calls
I2-7 7ae61fd8 ✅ Single-instance intent recorded at the multi-instance seams
I2-8 ⏸ Deferred to Iteration 3 — see the bottom of this comment
I2-9 ✅ Verified — L2 closed, no code needed
I2-10 81f062f8 ✅ Password stripped from the tree model — answered in-thread
I2-17 bca46b67 ✅ Cached tree error state cleared when the failure is fixed elsewhere

I2-2 — f473b02c Ask before recreating, and take the answer as an explicit flag (M4)

When stored credentials existed, the Configure step relabelled its settings — but the primary button still read Start DocumentDB Local, the footer still said "Nothing else on your machine is changed", and provision() force-removed the running container first. Opening Quick Start out of curiosity destroyed a running instance while the copy said the opposite.

Configure now asks, above the settings table:

  • Use existing data — recreate the container and keep your documents
  • Start fresh — erases all data in DocumentDB Local (never pre-selected)

The answer travels as AdvancedQuickStartOptions.startFresh instead of being inferred from getReusableCredentials(), and the primary label plus the footer note follow the selection. Per the agreed Q4 answer there is no extra confirmation dialog — the destructive option states the data loss in its own label, and a modal on top would prompt twice for a decision just made deliberately.

The RR4 / §5.2 volume-wipe gate is preserved: "Start fresh" is now the only path that may drop a volume, and it is an explicit user choice. This also resolves N1 by construction — there is no inferred value left to go stale.

willReuse / willReuseExistingInstance() are renamed to canReuseExistingData: the flag now names a capability rather than an outcome the service decides.

I2-3 — 5e2c9314 Guard the wizard when an instance already exists

Reaching the wizard while an instance exists should not normally be possible — the tree does not link to it in that state — but the command palette, a stale panel and cross-window races all still get there, and a healthy running instance could be walked straight into a destructive recreate.

Placement changed from the plan: the guard is on the Configure step, not Introduction. That is where the destructive decision is actually made and where the guard sits next to the "Start fresh" radio it forces; on Introduction it would be a speed bump the user clicks past before reaching the choice it guards.

Variant MessageBar Primary
Healthy (Running) info disabled — offers Open Connection / Close
Stopped info disabled — offers Start / Close
CredentialsMissing warning enabled, forced onto Start fresh

Missing is deliberately not guarded — recreating is exactly what that state asks for. The service's old hard refusal for the credential-unavailable case (which left the user hunting for a separate Delete Container) becomes an explicit, warned Start-fresh path inside the wizard.

I2-4 — a684ce95 Actionable error nodes (§9.2 Q4, N3)

Two dead ends removed. The state_error row showed status.errorMessage as its description with no command, and the NotInstalled branch pushed a second, message-only ${id}/error child (N3).

The tree now renders recovery actions, not error text: createRetryNode — pointed at the wizard, because for a provisioning failure that is the operation being retried — plus View setup log and, once a container exists, Delete Container. The message-only child is gone.

LocalQuickStartItem now implements TreeElementWithRetryChildren, which is what makes BaseExtendedTreeDataProvider cache the failed children. That is the mechanism behind the passive-vs-real retry distinction: a parent/whole-view refresh reuses the cached error children and never re-runs the failing operation (so no second modal), while clicking retry resets the cache and genuinely retries. Per the Q5 answer, no detectErrorState hook was added — Missing and CredentialsMissing are service states with their own actionable rows, not fetch failures.

I2-5 — 1a0f3ab9 Render the tree row from cache instead of blocking on Docker (M6)

getChildren() awaited refreshLiveState(), so every Connections-view refresh — connection add/remove/rename, folder ops, discovery refresh, ext.state transitions — paid a process spawn plus a Docker round-trip, for every user who has ever provisioned an instance.

The row now renders immediately from the last known state and the probe runs in the background via refreshLiveStateInBackground(). Rows a probe can actually change (Running / Stopped / Missing) show a Refreshing… hint while one is in flight, and onDidChangeStatus redraws them.

One thing worth a reviewer's eye: clearing that hint requires firing the status event even when nothing changed, and that event re-enters getChildren(). The 5 s BACKGROUND_REFRESH_COOLDOWN_MS is therefore load-bearing, not an optimisation — it is what stops the completion event arming the next probe and rebuilding H1 in a new shape. A regression test asserts one docker inspect for a burst of four calls.

I2-6 — bef2128f Skip suggestPort() on polled readiness calls (M6-b)

getDockerStatus ran suggestPort() on every call, including the readiness poll that re-runs on a 1–5 s backoff for up to 90 s; suggestPort() binds a probe socket per candidate and walks up to QUICK_START_PORT_SCAN_LIMIT ports. DockerStatusResult.suggestedPort is now optional and omitted when input.polled is set; the webview keeps its previous suggestion, so the Configure field is never blanked by a poll.

I2-7 — 7ae61fd8 Record the single-instance scope in code

The service layer is already multi-instance-shaped while the tree and webview assume exactly one instance, and a reader could not tell whether those seams were load-bearing or aspirational. Notes added at the instances map, reservedPorts(), listStatuses() and the operationId nonce — the last flagged as load-bearing today for concurrent windows (H3/H4), which was precisely the distinction that was unclear. Documentation-in-code only.

I2-1 — ec0a77cb Pin the H5 contract

The suite stayed green through H5 because IContainerRuntime, ext.secretStorage and the tree provider are all mocked. The new tests pin the contract that closed it: with an empty CredentialCache — the post-reload state that made H5 reproducible — the node still lists databases, QuickStartService is what supplies them, and the cache ends up populated as a side effect rather than a precondition. Also covers the no-stored-secret path (no client, error-recovery children, no throw out of getChildren()).

I2-17 — bca46b67 Clear the cached tree error state when the failure is fixed elsewhere

failedChildrenCache freezes a node's children once classified as failed. If the user then fixed the problem outside the tree — typically in the Quick Start webview — the row kept rendering the stale error node until a manual collapse/expand. onDidChangeStatus now resets the Quick Start subtree's error state before refreshing, mirroring the AtlasDiscoveryProvider.onDidChangeSession precedent.

I2-9 — L2 closed by verification

suggestPort() returns the instance's own recorded port when it is still free (already covered by "prefers the instance own recorded port so a recreate keeps its address"), and the Configure Address row renders it while portTouchedRef stops a suggestion from clobbering a typed value. No code needed.

I2-10 — 81f062f8 — answered in the review thread

Answered where it was raised: #discussion_r3714252974.


Not shipped: I2-8 (credential store consolidation) → Iteration 3

Approved, then deferred on two gates confirmed while scoping it:

  1. TDD: contract tests. QuickStartProvisionDurability.test.ts asserts the persistence contract against the raw keyssecretKey(DEFAULT_ALIAS) written before the readiness probe and cleared on a failed attempt, readRegistry(globalState) showing provisioning → ready → missing. This item relocates exactly those keys. .github/copilot-instructions.md requires a maintainer decision before touching a TDD: suite.
  2. Data safety. The migration must carry the secrets and the registry including the live lease fields (operationId, leaseAt), and must complete before reconcile() — the R1 ordering migrateLegacyQuickStartKeys already relies on. A mistake there is a silent volume wipe.

It is hygiene, not a fix — H5 was closed without it — and nothing else depends on it. Tracked as I3-1, blocked on I3-Q1 in §11.5.


Verification: npm run l10nnpm run prettier-fixnpm run lint (clean) → npx jest --no-coverage (204 suites / 3355 tests, all passing; +1 suite and +9 tests from this iteration) → npm run build (clean).

…vice (I3-1)

The managed instance's state was split across three ad-hoc stores: the
connection string in a raw SecretStorage key, the image ref in globalState, and
the instance list in a hand-rolled 'documentdb.quickstart.registry' blob with
its own write lock. provision() had to hand-roll a two-phase commit across them.

It now lives in StorageService under 'local-quickstart' / workspace 'instances',
one item per alias: the record in properties, the connection string in secrets.
A state change is a single push(). This follows the service-kubernetes
sourceStore and service-atlas-mongodb credential-store precedents.

The feature has not shipped, so there is no migration and no legacy read
fallbacks: quickStartRegistry.ts, migrateLegacyQuickStartKeys, secretKey(),
imageRefKey() and the LEGACY_* keys are all deleted, and the activation path
loses its pre-reconcile migration step.

Found and fixed on the way: StorageService.push() writes a secret but never
clears one, so the H3 restore path (a discarded provision must not leave its
credentials behind) needed an explicit delete-then-push. The existing coverage
missed it because it failed at 'docker run', before the early credential write
- two tests now cancel during the readiness wait, which is the real path.

New quickStartStore.test.ts covers the record/credential round-trip, the
create-on-first-write behaviour, the clear path and the in-lock conditional
delete the lease release depends on.
Records I3-1 as implemented and corrects the Iteration 2 deferral rationale:
the TDD: blocker did not exist (it came from a scoping summary, not the tree),
and the migration gate was removed rather than met - the feature has not
shipped, so there is no migration and no legacy fallbacks.

Also documents the StorageService.push() secret-clearing bug found on the way,
which matters to any other feature built on that storage.
@tnaum-ms

Copy link
Copy Markdown
Collaborator Author

Iteration 3 — b8e25fc3 Quick Start durable state moves to StorageService (I3-1)

The last outstanding review item. The managed instance's state was split across three ad-hoc stores — the connection string in a raw SecretStorage key, the image ref in globalState, and the instance list in a hand-rolled documentdb.quickstart.registry blob with its own write lock — which is why provision() had to hand-roll a two-phase commit across them (write the secret early, remember the previous value, restore it in finally).

It now lives in StorageService.get('local-quickstart'), workspace instances, one item per alias: the record in properties, the connection string in secrets. A state change is a single push(). Same idiom as the service-kubernetes sourceStore and the service-atlas-mongodb credential store, so the codebase has one storage pattern instead of a per-feature one.

Net −500 lines. quickStartRegistry.ts, migrateLegacyQuickStartKeys, secretKey(), imageRefKey(), the LEGACY_* constants and the pre-reconcile() migration step in ClustersExtension are all deleted.

Two things worth flagging

1. The blocker I recorded for this item did not exist. I deferred it out of Iteration 2 partly because a scoping summary said QuickStartProvisionDurability.test.ts contained a TDD:-prefixed contract suite, which repo policy says must not be rewritten without a maintainer decision. It does not — grep -rl "TDD:" src/ returns only query-language and playground files, none under src/services/localQuickStart/. The affected tests are ordinary it(...) cases. My mistake was recording a blocker from a summary of the tree rather than from the tree; the permission was granted anyway, so it cost only the deferral.

2. StorageService.push() never clears a secret. It writes the secret key when item.secrets is non-empty and simply doesn't touch it otherwise — so clearing credentials by pushing an item without them leaves the old secret readable. That silently breaks the H3 restore contract: a discarded provision would leave its credentials behind, and the next run would decide reusing from credentials no volume was ever initialized with. updateInstance now deletes the item before re-pushing when the credentials are being cleared (safe inside the lock; a crash between the two leaves no record, which is the harmless direction).

This is a property of the shared storage service, not of Quick Start — worth knowing for anything else built on it.

The existing coverage could not have caught it: "restores the previous credential state when the attempt fails" fails at docker run, i.e. before the early credential write, so the restore never ran. Two new cases cancel during the readiness wait — the path a user actually takes — and assert both that a fresh attempt's credentials are cleared and that a failed recreate puts the previous ones back.

No migration

Per the maintainer's call — the feature has not shipped, so there is nothing to migrate from. That is what took the risk out of this item: the danger was never the destination, it was a migration that had to carry live lease fields (operationId, leaseAt) ahead of reconcile(), where a mistake is a silent volume wipe. There is no such migration now.

Also dropped: the registry's nextSuffix counter, which had no production reader (multi-instance is out of scope) and has no home in a per-item store.

Coverage

New quickStartStore.test.ts — record/credential round-trip, credentials preserved when only the record is rewritten, create-on-first-write, the clear path, and the in-lock conditional delete the lease release depends on.


Verification: npm run prettier-fixnpm run lint (clean) → npx jest --no-coverage (204 suites / 3344 tests, all passing) → npm run build (clean).

All three review iterations are now closed — every finding routed through §11 of the review doc is resolved. What remains is the deferred pool (B1 footer experiment, N4, N7, multi-instance, issues #864/#865), none of it blocking and none of it release work.

…xists

A container removed outside VS Code is reported by the service as state Stopped
WITH missing set, not as a state of its own. The Configure step's guard read
only the state, so it classified it as 'stopped' and offered a Start button
that could not do anything.

Three fixes:

- The guard now checks missing before the state, and does not guard a missing
  instance at all: its container is gone, so recreating is exactly what the user
  came for. Extracted to existingInstanceGuard.ts with tests, since this is a
  distinction that is easy to get wrong twice.
- The recreate-vs-fresh radio is hidden whenever the guard blocks setup. Showing
  a choice next to a disabled primary action read as a third, broken control
  (Fluent also dims unselected radio labels, so it looked disabled as well). The
  group now carries an explicit question instead of two bare labels.
- The panel subscribes to instance status instead of reading it once on open, so
  a tree action, another window, or a docker rm in a terminal no longer leaves
  the guard describing an instance that is gone (review N1). The subscription is
  deliberately cheap: no isDockerReady, no refreshLiveState, and it skips events
  that change nothing user-visible.

Also drops em dashes from this feature's user-facing strings.
…he copy

The Configure step showed a notice and a separate radio group, so the same
question was effectively asked twice and the pair read as competing controls.
They are now one MessageBar: the explanation on top, the choice beneath it.
MessageBar renders role=group, which is the right container for a set of
related controls, and this panel already nests interactive content in a
message bar body.

Copy changes:
- No title on the notice. 'The DocumentDB Local container is gone' read as an
  alarm for what is a recoverable, data-preserving situation.
- The question is asked once, by the field label. It stays self-contained when
  no explanatory sentence precedes it.
- Radio labels are parallel and shorter: 'Keep the existing data' /
  'Erase the existing data and start empty'.
- The primary button is fixed at 'Start DocumentDB Local' again. The footer
  note above it still follows the selection, since 'Nothing else on your
  machine is changed' is only true for a genuinely fresh install.
N1 (panel reads instance status once per open) was recorded as 'resolved by
construction' by the M4 explicit-choice decision. It was not: making the choice
explicit removed the inference, but the status snapshot was still fetched on
mount and never refreshed. That staleness is how the P-1 guard bug was
reachable in practice. Both are now fixed and recorded in a new 11.6.

Also notes the process failure: a full audit repeated the document's claim
instead of checking it against the code, the same way the phantom TDD blocker
in 11.5 came from trusting a summary over the tree.
… and tell them to restart VS Code

Closes #855, #856.

#856: the Docker setup link was the same on every platform —
docs.docker.com/engine/install/, which documents a Linux-only Docker Engine
install. Worse, the "Install Docker" button users actually click hardcoded
that URL and ignored the guide the host had already resolved, so the
per-platform mapping could not have taken effect even once it existed. The
CTA now follows the resolved guide, and a missing CLI on Windows or macOS
resolves to the Docker Desktop install page. WSL and the remote targets keep
the Engine guide: the CLI has to exist in the environment the extension runs
in, which is a Linux one whatever the desktop OS is.

#855: after installing Docker Desktop, a VS Code that was already running
keeps the environment it was launched with, so the new PATH is invisible to
it and Docker stays undetected — the setup looks broken even though the
install succeeded. The Windows and macOS guidance now says to restart VS Code
explicitly, and calls out that reloading the window is not enough: the
extension host is respawned from the same main process and inherits the same
stale environment. The existing "Check again" action covers the retry after
the restart.
…ill has to be created

Closes #857.

The post-start footer put "Copy Connection String" next to "Open Connection",
which read as the next required setup step. A bug-bash user followed it and
created the localhost connection by hand, even though Quick Start had already
added the instance to the Connections view.

The success screen now states outright that the connection already exists and
does not need to be created. The copy action leaves the footer — where it
competed with the primary action — and becomes an inline link in Next steps,
introduced as optional and scoped to what it is actually for: reaching the
instance from a client outside VS Code, such as mongosh or the user's own
app. The footer keeps Open Connection with a note that it simply takes you to
the connection that is already there.
…ction is added by hand

Closes #858.

Adding a localhost connection manually never checked whether Quick Start's
managed instance already served that endpoint, so a second entry appeared for
the same local service. The instance is service-owned and in-memory rather
than a stored connection, so the existing duplicate scan — which reads stored
Emulator connections — could not see it at all.

The wizard now compares the new endpoint against the managed instances first.
On a match it explains that Quick Start already added the instance and offers
to open the existing entry, add a separate connection anyway (a different
configuration for the same endpoint is legitimate), or cancel; dismissing the
dialog cancels rather than creating the duplicate it just warned about.

Host comparison is also normalized. `localhost`, `127.0.0.1`, and `::1` are
the same machine, but the stored-connection scan compared host strings
verbatim, so the same service added under two spellings read as two different
connections. Endpoints now normalize to a canonical `host:port` — loopback
forms collapse, a missing port fills in the wire-protocol default, and both
the bracketed and bare IPv6 forms parse correctly. `127.0.0.2` is left
distinct on purpose: it is loopback at the IP layer, but a service bound to
one such address is not reachable on another, so collapsing them would report
duplicates that are not.
Two defects from the `0.10.0-bug-bash-1` bash have no webview surface, so no
service-level test can reach them. `contributions.test.ts` covers them.

It asserts the `commandPalette` gating that #851 was missing, because that defect
lived in package.json data rather than in code, and that the Quick Start entry
point stays reachable. It also asserts the #852 strings appear in
`l10n/bundle.l10n.json`, which is proof they go through `l10n.t()`, since the
extractor sees nothing else. #858 was already covered by `localEndpoint.test.ts`.
The success screen's primary action ran `connectionsView.focus` and nothing
else. When the Connections view is already the active view in the sidebar —
the normal case, since Quick Start is opened FROM it — focusing it changes
nothing on screen, so clicking Open Connection appeared to do nothing at all.
The code comment claimed it revealed the connection; it never did.

It now reveals the managed-instance row, selects it, and expands it, which is
what "open" means for a cluster row in this tree: expanding connects and lists
the databases. The reveal is progressive, because the instance row is a lazily
created child and its parent has to be expanded before the child can be found.
The panel still stays open — only the explicit Close button dismisses it.

The paths and the navigation move into `revealQuickStartInstance`, shared with
the duplicate-endpoint prompt's "Open Existing" (#858), so both routes to the
instance land in the same place. A test asserts the path constants match the
ids `LocalQuickStartItem` actually builds: nothing in the type system ties them
together, and a silent mismatch would turn every "take me to the instance"
action back into a no-op.

The footer note is reworded to promise what now happens rather than "just takes
you there".
…art-bugbash-fixes

This branch predates the code-review work that landed on the base, so the
overlapping areas are reconciled here rather than rebased away.

Resolutions:

* `LocalQuickStart.tsx`, Docker guidance maps. Neither side verbatim. The base
  converted `DOCKER_GUIDANCE` / `DOCKER_GUIDES` from module-scope consts into
  render-time `dockerGuidance()` / `dockerGuides()`, because `WebviewRegistry`
  imports the component statically, so a module body runs before `l10n.config()`
  and a module-scope `l10n.t()` is permanently English (review M1, issue #864).
  This branch added four keys to those same maps for the Windows and macOS
  Docker Desktop guidance. Kept the function form, with the four new keys inside
  it: taking either side verbatim would have shipped the new strings
  untranslatable, or dropped them.

* `LocalQuickStart.tsx`, `handleInstallDocker`. Took this branch's deletion. The
  callback hardcoded `dockerGuides().install.href` and ignored the guide the host
  had already resolved, which is defect #856 itself; the per-platform mapping
  cannot take effect while it exists.

* `QuickStartService.provision()` signature. Took the base's
  `(signal, options, alias = DEFAULT_ALIAS)`; review N5 threaded `alias`
  consistently and this branch predates it. Kept this branch's hoisted message
  const.

* `QuickStartService.resumeReadiness()` busy event. Took this branch's version.
  The base yielded `error: 'in progress'`, and the webview renders
  `event.error ?? event.message`, so that untranslated marker reached the message
  bar verbatim. A real bug the review missed.

* `QuickStartService.provision()` port selection. Took the base wholesale. The
  auto-relocation path this branch corrected the wording of was removed outright
  by the explicit-port model (review L3, "no magic after execute").

* `localQuickStartRouter.test.ts`. Adjacent inserts into the same `describe`;
  kept both.

* `l10n/bundle.l10n.json`. Generated, never hand-merged. Discarded both sides and
  regenerated with `npm run l10n` once the source strings had settled.

* `package.json` auto-merged cleanly but wrongly: both sides added the same seven
  `"when": "never"` `commandPalette` entries at different offsets, so git kept
  all fourteen. Deduplicated, keeping the block that carries the explanatory
  `"//"` key.
… code

Four things the merge could not flag, because nothing conflicted textually.

* `LocalQuickStart.tsx` still read `DOCKER_GUIDES[...]` at the #856 install CTA.
  The maps became render-time functions (review M1), so the reference is now
  `dockerGuides()[...]`. Without this the webview bundle does not compile.

* `revealQuickStartInstance.test.ts` spied on `QuickStartService.refreshLiveState`.
  Review I2-5 moved the tree's call to `refreshLiveStateInBackground()`, so the
  spy no longer intercepted anything and the test ran the real background probe.
  Repointed. The path constants it guards are unchanged and still correct.

* `contributions.test.ts` asserted two strings that the explicit-port model
  deleted: the old "Choose a different port or free it" wording, now "Go back to
  Configure to pick a different port", and `Port {0} was busy, using {1} instead.`,
  which no longer exists at all. The guard against a false claim is kept but
  repointed at the current contract: no string may tell the user the port was
  substituted.

* Added a uniqueness assertion for `commandPalette`. Both branches added the same
  seven gating entries at different offsets, and every presence check still
  passed on the merged duplicate block, so only uniqueness catches that class of
  merge artifact.

`l10n/bundle.l10n.json` is regenerated, not hand-merged.
Three strings this branch adds used an em dash where a colon or a sentence break
reads the same. House style has no em or en dashes in product copy.

The Docker Desktop restart guidance (#855) and the two optional connection-string
lines on the success screen (#857). Wording is otherwise unchanged; the bundle is
regenerated.
@tnaum-ms
Tomasz Naumowicz (tnaum-ms) merged commit 6cd7b83 into release/0.10.0 Aug 6, 2026
@tnaum-ms
Tomasz Naumowicz (tnaum-ms) deleted the feature/local-quickstart branch August 6, 2026 21:38
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

✅ Code Quality Checks

Check Status How to fix
Localization (l10n) ✅ Passed
ESLint ✅ Passed
Prettier formatting ✅ Passed

This comment is updated automatically on each push.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

📦 Build Size Report

Metric Base (release/0.10.0) PR Delta
VSIX (vscode-documentdb-0.10.0-bug-bash-1.vsix) 8.01 MB 8.14 MB ⬆️ +135 KB (+1.7%)
Webview bundle (views.js) 5.88 MB 6.11 MB ⬆️ +231 KB (+3.8%)

Download artifact · updated automatically on each push.

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

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

4 participants