vscode-documentdb

# GitHub Copilot Instructions for vscode-documentdb

VS Code Extension for Azure Cosmos DB and the MongoDB API. TypeScript (strict mode), React webviews, Jest testing.

## Critical Build Commands

| Command                | Purpose                                                      |
| ---------------------- | ------------------------------------------------------------ |
| `npm run build`        | **Build the project** (use this, NOT `npm run compile`)      |
| `npm run lint`         | Check for linting errors                                     |
| `npm run prettier-fix` | Format code                                                  |
| `npm run l10n`         | Update localization files after changing user-facing strings |

> ⚠️ **NEVER use `npm run compile`** - always use `npm run build` to build the project.

## Verification: two cases

There are exactly two cases. Work out which one you are in, then run **only** that list.

For PR-related work, tell the operator which case applies in your handoff. If the PR
is still a draft, explicitly say that the full checks (including `prettier-fix`)
were deferred until it is ready for review. When handing a PR over for review,
report the Case 2 checks that ran and any failures or blockers; never imply the
full suite passed if it did not run.

### Case 1 — still working

Any commit, any push, opening or updating a **draft** PR.

```bash
npm run build                     # catches type errors
npx jest --no-coverage <path>     # only the tests covering what you touched
```

Nothing else. Do **not** run `l10n`, `prettier-fix`, `lint`, or `package` here.

### Case 2 — handing over

When asked to **prepare a PR for review** or **mark it ready for review**, run the
full Case 2 list even if another handoff requirement (such as the AI pre-review)
is missing. Report all blockers; do not mark the PR ready until the checks pass
and the other requirements are satisfied. If there is no PR, stay on Case 1.

```bash
npm run l10n            # only if a vscode.l10n.t() string was added, changed, or removed
npm run prettier-fix
npm run lint
npx jest --no-coverage  # full suite
npm run build
npm run package         # catches bundling and missing-asset failures
```

Also confirm the AI pre-review (CONTRIBUTING.md §6) has run and its review file is
committed under `docs/ai-and-plans/features/<name>/iterations/`.

Update `docs/ai-and-plans/features/<name>/README.md` (and `design.md`) in the same PR
when a decision, constraint, or intended design changed, or when you already know a
current document has become materially misleading. This is not a drift sweep.

> ⚠️ **Do not mark a PR ready for review until every Case 2 command has run and passed.**

### `l10n/bundle.l10n.json` conflicts

Never hand-merge a conflict in `l10n/bundle.l10n.json`. It is generated. Take either
side, or delete the file, then run `npm run l10n` and commit the result. Resolving it
by hand is slower and produces a bundle that does not match the source.

## Git Safety

- **Never use `git add -f`** to force-add files. If `git add` refuses a file, it is likely in `.gitignore` for a reason (e.g., `docs/plan/`, `docs/analysis/`, build outputs). Do NOT override this with `-f`.
- When `git add` warns that a path is ignored, **stop and inform the user** instead of force-adding.
- Files in `docs/plan/` and `docs/analysis/` are **local planning documents** that must not be committed to the repository.

## Feature Knowledge Base

`docs/ai-and-plans/` records how features were designed and **why**. Much of it is AI-written under human supervision; the durable value is the recorded operator decisions and their reasoning.

- `docs/ai-and-plans/README.md` — feature index. Short and maintained. Read it when a task touches a feature.
- `features/<name>/README.md` and the flat files beside it are the **best available account** of a feature's design and intent. They are maintained, but they describe intent, not guaranteed current behavior.
- `features/<name>/iterations/**` is **history**. Read only the specific iteration needed to resolve provenance, rationale, or a regression. Never bulk-load it. Plans and reviews there are evidence of past reasoning, not a description of the product today.
- **On conflict, the code wins for behavior; active docs win for intent.** If they disagree, do not silently pick one — name the doc and the code, and offer to correct the doc.
- Never treat `status: historical` or `status: superseded` as current.

When a significant design choice, new constraint, rejected alternative, or deviation from
the agreed plan arises, ask the operator whether to record it in the feature's
`decisions.md` while the reasoning is fresh. Briefly explain the choice and why its
rationale may matter later. If the operator agrees, append the decision with their
reasoning and any rejected alternatives; do not invent their rationale or wait until
PR handoff to reconstruct it. Minor implementation choices do not need this check.

## Project Structure

| Folder          | Purpose                                    |
| --------------- | ------------------------------------------ |
| `src/`          | Main extension source code                 |
| `src/webviews/` | React web view components                  |
| `src/commands/` | Command handlers (one folder per command)  |
| `src/services/` | Singleton services                         |
| `src/tree/`     | Tree view data providers                   |
| `api/`          | Separate Node.js project for extension API |
| `l10n/`         | Localization files                         |
| `test/`         | Jest tests                                 |

## Branching

- **`main`**: Default branch; all PRs target it. Normal releases are tagged on `main` and published from `main`.
- **`release/<X.Y.Z>`**: Created only when a patch must ship while `main` is not releasable. Branched off a release tag, published from, then deleted. Named for the version it ships (`release/0.9.1`), not the minor line.
- Moving a merged fix onto a `release/*` branch is a **backport** — see [skills/backport/SKILL.md](skills/backport/SKILL.md).

## TypeScript Guidelines

- **Never use `any`** - use `unknown` with type guards
- **Prefer `interface`** for object shapes, `type` for unions
- **Always specify return types** for functions
- **Use `vscode.l10n.t()`** for all user-facing strings

```typescript
// ✅ Good - Interface with explicit types
interface ConnectionConfig {
  readonly host: string;
  readonly port: number;
}

// ✅ Good - Named function with return type
export function createConnection(config: ConnectionConfig): Promise<Connection> {
  // implementation
}

// ✅ Good - Localized user-facing string with safe error handling
try {
  await operation();
} catch (error) {
  const errorMessage = error instanceof Error ? error.message : String(error);
  void vscode.window.showErrorMessage(vscode.l10n.t('Failed to connect: {0}', errorMessage));
}
```

## Null Safety

Use `nonNullProp()`, `nonNullValue()`, `nonNullOrEmptyValue()` from `src/utils/nonNull.ts`:

```typescript
// ✅ Good - Use nonNull helpers for internal validation
const connectionString = nonNullProp(
  selectedItem.cluster,
  'connectionString',
  'selectedItem.cluster.connectionString',
  'ExecuteStep.ts',
);

// ✅ Good - Manual check for user-facing validation with l10n
if (!userInput.connectionString) {
  void vscode.window.showErrorMessage(vscode.l10n.t('Connection string is required'));
  return;
}
```

## Error Handling

When accessing error properties in catch blocks or error handlers, always check if the error is an instance of `Error` before accessing `.message`:

```typescript
// ✅ Good - Type-safe error message extraction
try {
  await someOperation();
} catch (error) {
  const errorMessage = error instanceof Error ? error.message : String(error);
  void vscode.window.showErrorMessage(vscode.l10n.t('Operation failed: {0}', errorMessage));
}

// ✅ Good - In promise catch handlers
void task.start().catch((error) => {
  const errorMessage = error instanceof Error ? error.message : String(error);
  void vscode.window.showErrorMessage(vscode.l10n.t('Failed to start: {0}', errorMessage));
});

// ❌ Bad - Direct access to error.message (eslint error)
catch (error) {
  void vscode.window.showErrorMessage(vscode.l10n.t('Failed: {0}', error.message)); // Unsafe!
}
```

## Command Pattern

Each command gets its own folder under `src/commands/`:

```
src/commands/yourCommand/
├── YourCommandWizardContext.ts   # Wizard state interface
├── PromptXStep.ts                # User input steps
├── ExecuteStep.ts                # Final execution
└── yourCommand.ts                # Main orchestration
```

## Security

- Never log passwords, tokens, or connection strings
- Use VS Code's secure storage for credentials
- Validate all user inputs

## Cluster ID Architecture (Dual ID Pattern)

> ⚠️ **CRITICAL**: Using the wrong ID causes silent bugs that only appear when users move connections between folders.

Cluster models have **two distinct ID properties** with different purposes:

| Property    | Purpose                          | Stable?                   | Use For                             |
| ----------- | -------------------------------- | ------------------------- | ----------------------------------- |
| `treeId`    | VS Code TreeView element path    | ❌ Changes on folder move | `this.id`, child item paths         |
| `clusterId` | Cache key (credentials, clients) | ✅ Always stable          | `CredentialCache`, `ClustersClient` |

### Quick Reference

```typescript
// ✅ Tree element identification
this.id = cluster.treeId;

// ✅ Cache operations - ALWAYS use clusterId
CredentialCache.hasCredentials(cluster.clusterId);
ClustersClient.getClient(cluster.clusterId);

// ❌ WRONG - breaks when connection moves to a folder
CredentialCache.hasCredentials(this.id); // BUG!
```

### Model Types

- **`ConnectionClusterModel`** - Connections View (has `storageId`)
- **`AzureClusterModel`** - Azure/Discovery Views (has `azureResourceId`)
- **`BaseClusterModel`** - Shared interface (use for generic code)

For Discovery View, both `treeId` and `clusterId` are sanitized (all `/` replaced with `_`). The original Azure Resource ID is stored in `AzureClusterModel.azureResourceId` for Azure API calls.

> 💡 **Extensibility**: If adding a non-Azure discovery source (e.g., AWS, GCP), consider creating a new model type (e.g., `AwsClusterModel`) extending `BaseClusterModel` with source-specific metadata.

See `src/tree/models/BaseClusterModel.ts` and `docs/analysis/08-cluster-model-simplification-plan.md` for details.

- [skills/tree-cluster-architecture/SKILL.md](skills/tree-cluster-architecture/SKILL.md) - Required patterns for cluster tree items, dual identity, provider lookup, and regression tests
- [skills/telemetry-instrumentation/SKILL.md](skills/telemetry-instrumentation/SKILL.md) - Telemetry instrumentation patterns
- [skills/error-translation/SKILL.md](skills/error-translation/SKILL.md) - Turning infrastructure failures into actionable messages; providers translate, they never show UI

## Terminology

This is a **DocumentDB** extension that uses the **MongoDB-compatible wire protocol**.

- Use **"DocumentDB"** when referring to the database service itself.
- Use **"MongoDB API"** or **"DocumentDB API"** when referring to the wire protocol, query language, or API compatibility layer.
- **Never use "MongoDB" alone** as a product name in code, comments, docs, or user-facing strings.

| ✅ Do                                                | ❌ Don't                         |
| ---------------------------------------------------- | -------------------------------- |
| `// Query operators supported by the DocumentDB API` | `// MongoDB query operators`     |
| `// BSON types per the MongoDB API spec`             | `// Uses MongoDB's $match stage` |
| `documentdbQuery` (variable name)                    | `mongoQuery`                     |

This applies to: code comments, JSDoc/TSDoc, naming (prefer `documentdb` prefix), user-facing strings, docs, and test descriptions.

## TDD Contract Tests

Test suites prefixed with `TDD:` (e.g., `describe('TDD: Completion Behavior', ...)`) are **behavior contracts** written before the implementation. If a `TDD:` test fails after a code change:

1. **Do NOT automatically fix the test.**
2. **Stop and ask the user** whether the behavior change is intentional.
3. The user decides: update the contract (test) or fix the implementation.

This applies to any test whose name starts with `TDD:`, regardless of folder location.

## Additional Patterns

For detailed patterns, see:

- [instructions/typescript.instructions.md](instructions/typescript.instructions.md) - TypeScript patterns and anti-patterns
- [instructions/wizard.instructions.md](instructions/wizard.instructions.md) - AzureWizard implementation details
- [skills/tree-cluster-architecture/SKILL.md](skills/tree-cluster-architecture/SKILL.md) - Cluster tree items, identity, lookup, and test contracts
- [skills/telemetry-instrumentation/SKILL.md](skills/telemetry-instrumentation/SKILL.md) - Telemetry instrumentation patterns
- [skills/backport/SKILL.md](skills/backport/SKILL.md) - Cherry-picking a merged fix onto a `release/*` branch and opening the backport PR