|
| 1 | +# Plan: AX Constitution + uninstall-mapper + http.test.ts Rewrite |
| 2 | + |
| 3 | +**Branch:** main | **Date:** 2026-04-09 | **Target version:** v1.6.0 |
| 4 | + |
| 5 | +--- |
| 6 | + |
| 7 | +## Execution Model: 3 Parallel Sub-Agents |
| 8 | + |
| 9 | +These three deliverables are fully independent — no shared files, no ordering constraints. After plan approval, three Engineer agents execute concurrently in isolated worktrees, each owning one domain. Results are merged. |
| 10 | + |
| 11 | +| Workstream | Agent | Domain | Files touched | |
| 12 | +|-----------|-------|--------|---------------| |
| 13 | +| **W1 — AX Constitution** | Engineer (docs) | Documentation | `docs/AX.md`, `CLAUDE.md`, `Plans/federated-coalescing-lampson.md` | |
| 14 | +| **W2 — uninstall-mapper** | Engineer (CLI) | vslsp.ts CLI | `vslsp.ts` only | |
| 15 | +| **W3 — http.test.ts** | Engineer (test) | Test behavior | `tests/http.test.ts` only | |
| 16 | + |
| 17 | +Each agent has its own sub-plan below. No cross-agent dependencies. |
| 18 | + |
| 19 | +--- |
| 20 | + |
| 21 | +## Context |
| 22 | + |
| 23 | +Three deliverables, all guided by the same principle: **agent experience (AX) is paramount**. |
| 24 | + |
| 25 | +1. **AX Constitution** — The existing AX philosophy is scattered across `Plans/federated-coalescing-lampson.md`, code comments, and tests. There is no canonical document an agent or developer can read to understand _why_ vslsp makes the design decisions it does, what the inviolable contracts are, and how to stay compliant when extending the tool. This is the most leveraged deliverable: it shapes every future change. |
| 26 | + |
| 27 | +2. **`vslsp uninstall-mapper <lang>`** — Install has no removal path. Once a mapper is installed, the only way to remove it is manual filesystem surgery. This is a broken DX story and a gap in the CLI surface. |
| 28 | + |
| 29 | +3. **`http.test.ts` rewrite** — The current tests check source-code strings, not actual HTTP behavior. They provide false confidence: a completely broken HTTP server would pass them. Real behavior tests protect the daemon API that agents depend on. |
| 30 | + |
| 31 | +--- |
| 32 | + |
| 33 | +## ISC (Ideal State Criteria) |
| 34 | + |
| 35 | +- [ ] `docs/AX.md` exists, covers: goal, budget thresholds, principles, tool schema guidance, error message standards, CI enforcement, extension checklist |
| 36 | +- [ ] `vslsp uninstall-mapper <lang>` removes the correct directory, exits 0 if not installed, exits 1 on unknown language |
| 37 | +- [ ] `vslsp uninstall-mapper` appears in `vslsp --help` output |
| 38 | +- [ ] `tests/http.test.ts` makes real HTTP requests, asserts status codes and JSON response bodies |
| 39 | +- [ ] All 8+ HTTP routes have at least one success test and one failure/edge-case test |
| 40 | +- [ ] `bun run tsc --noEmit` clean after all changes |
| 41 | +- [ ] `bun test --timeout 60000` green (70+ pass, 0 fail) |
| 42 | + |
| 43 | +--- |
| 44 | + |
| 45 | +## Deliverable 1: AX Constitution (`docs/AX.md`) |
| 46 | + |
| 47 | +### File to create |
| 48 | +`docs/AX.md` — new file, canonical reference |
| 49 | + |
| 50 | +### Structure |
| 51 | + |
| 52 | +``` |
| 53 | +# vslsp AX Constitution |
| 54 | +
|
| 55 | +## The AX Guarantee |
| 56 | +(One paragraph: core contract — tool calls must never pollute agent context window) |
| 57 | +
|
| 58 | +## Budget Thresholds |
| 59 | +(Table: same thresholds from federated-coalescing-lampson.md, with reasoning) |
| 60 | +
|
| 61 | +## Design Principles |
| 62 | +1. Filter by default, not by exception |
| 63 | +2. Warn with agent-actionable messages (not just what — what to do next) |
| 64 | +3. Tool schemas describe the filtering surface, not the data surface |
| 65 | +4. Never return 0 files silently — warn when filters produce empty results |
| 66 | +5. Ratchet tests lock every contract in CI |
| 67 | +
|
| 68 | +## Tool Schema Standards |
| 69 | +- `description` field must tell agents WHEN to use this tool and HOW to scope it |
| 70 | +- Parameter descriptions must include the AX implication (e.g. "without this filter, response may exceed context window budget") |
| 71 | +- Error messages must include the corrective action, not just the error |
| 72 | +
|
| 73 | +## Error Message Standard |
| 74 | +(Template: "[What went wrong]. [What the agent should do next]. Example: ...") |
| 75 | +
|
| 76 | +## CI Enforcement |
| 77 | +(How AX contract tests work — dual bound: lower + upper) |
| 78 | +
|
| 79 | +## Extension Checklist |
| 80 | +(For any new tool or language: what AX obligations must be met) |
| 81 | +``` |
| 82 | + |
| 83 | +### Cross-references |
| 84 | +- Update `Plans/federated-coalescing-lampson.md` status line to reference `docs/AX.md` as the canonical doc |
| 85 | +- Update `CLAUDE.md` Agent Quick-Start to mention AX.md |
| 86 | + |
| 87 | +--- |
| 88 | + |
| 89 | +## Deliverable 2: `vslsp uninstall-mapper <lang>` |
| 90 | + |
| 91 | +### Files to modify |
| 92 | +- `vslsp.ts` — add command type, arg parsing, function, help text |
| 93 | + |
| 94 | +### Implementation |
| 95 | + |
| 96 | +**Step 1 — Command type** (`vslsp.ts:81`) |
| 97 | +```typescript |
| 98 | +type Command = "serve" | "query" | "status" | "notify" | "map" | "install-mapper" | "uninstall-mapper" | "oneshot"; |
| 99 | +``` |
| 100 | + |
| 101 | +**Step 2 — Arg parsing** (in `parseArgs()`, after the install-mapper block, around line 130) |
| 102 | +```typescript |
| 103 | +} else if (firstArg === "uninstall-mapper") { |
| 104 | + result.command = "uninstall-mapper"; |
| 105 | + args.shift(); |
| 106 | + result.installLang = args[0] && !args[0].startsWith("-") ? args.shift()! : ""; |
| 107 | +} |
| 108 | +``` |
| 109 | +Note: `installLang` is already in the `CLIArgs` interface (shared with install-mapper). |
| 110 | + |
| 111 | +**Step 3 — Function** (add after `installMapper()`, around line 246) |
| 112 | +```typescript |
| 113 | +async function uninstallMapper(language: string): Promise<void> { |
| 114 | + const m = getMapper(language); |
| 115 | + if (!m) { |
| 116 | + console.error(`Unknown language: ${language}\nSupported: csharp, rust, typescript`); |
| 117 | + process.exit(1); |
| 118 | + } |
| 119 | + |
| 120 | + const installDir = m.installDir; // absolute path from registry |
| 121 | + const legacyDir = language === "csharp" |
| 122 | + ? join(homedir(), ".local", "share", "vslsp", "code-mapper") |
| 123 | + : null; |
| 124 | + |
| 125 | + let removed = false; |
| 126 | + if (existsSync(installDir)) { |
| 127 | + rmSync(installDir, { recursive: true, force: true }); |
| 128 | + console.log(`Removed: ${installDir}`); |
| 129 | + removed = true; |
| 130 | + } |
| 131 | + if (legacyDir && existsSync(legacyDir)) { |
| 132 | + rmSync(legacyDir, { recursive: true, force: true }); |
| 133 | + console.log(`Removed legacy: ${legacyDir}`); |
| 134 | + removed = true; |
| 135 | + } |
| 136 | + if (!removed) { |
| 137 | + console.log(`${language} mapper is not installed.`); |
| 138 | + } |
| 139 | +} |
| 140 | +``` |
| 141 | + |
| 142 | +**Imports to add** (`vslsp.ts` top): |
| 143 | +- Add `rmSync` to existing fs import (already has `existsSync`, `mkdirSync`) |
| 144 | +- Add `homedir` from `os` if not already imported (check: `installDir` in registry may already be absolute) |
| 145 | + |
| 146 | +> **Check first**: Read `src/code-mapping/registry.ts` to confirm whether `installDir` is already an absolute path (via `homedir()` or `DEFAULT_VSLSP`). If it is, no extra import needed. |
| 147 | +
|
| 148 | +**Step 4 — Command dispatch** (in the main switch, around line 355) |
| 149 | +```typescript |
| 150 | +case "uninstall-mapper": { |
| 151 | + if (!args.installLang) { |
| 152 | + error("Usage: vslsp uninstall-mapper <lang> (csharp | rust | typescript)"); |
| 153 | + } |
| 154 | + await uninstallMapper(args.installLang); |
| 155 | + break; |
| 156 | +} |
| 157 | +``` |
| 158 | + |
| 159 | +**Step 5 — Help text** (in the HELP string, around line 15) |
| 160 | +Add to COMMANDS section: |
| 161 | +``` |
| 162 | + uninstall-mapper <lang> Remove an installed mapper binary (csharp | rust | typescript) |
| 163 | +``` |
| 164 | +Add to EXAMPLES section: |
| 165 | +``` |
| 166 | + vslsp uninstall-mapper rust |
| 167 | +``` |
| 168 | + |
| 169 | +--- |
| 170 | + |
| 171 | +## Deliverable 3: `http.test.ts` Rewrite |
| 172 | + |
| 173 | +### File to replace |
| 174 | +`tests/http.test.ts` — full rewrite, same file location |
| 175 | + |
| 176 | +### Approach: real HTTP server with mocked dependencies |
| 177 | + |
| 178 | +The `createHttpServer()` function in `src/diagnostics/http.ts` accepts `{ port, client, store, solutionPath }`. For tests: |
| 179 | +- Use a free port (e.g. 7860 or find-free-port) |
| 180 | +- Mock `client` with a minimal object implementing only the methods http.ts calls (`didSave`, `didChange`) |
| 181 | +- Mock `store` with a minimal object implementing `getAll()`, `getByFile()`, `getSummary()` |
| 182 | +- Start server in `beforeAll`, stop in `afterAll` |
| 183 | + |
| 184 | +### Test structure |
| 185 | + |
| 186 | +``` |
| 187 | +describe("HTTP server — real behavior", () => { |
| 188 | + // beforeAll: start server on a free port with mocks |
| 189 | + // afterAll: call /stop or server.stop() |
| 190 | +
|
| 191 | + describe("GET /health", () => { ... }) |
| 192 | + describe("GET /status", () => { ... }) |
| 193 | + describe("GET /diagnostics", () => { ... }) |
| 194 | + describe("GET /diagnostics/summary", () => { ... }) |
| 195 | + describe("POST /file-changed", () => { ... }) |
| 196 | + describe("POST /file-content", () => { ... }) |
| 197 | + describe("POST /stop", () => { ... }) // last — shuts server down |
| 198 | + describe("unknown routes", () => { ... }) |
| 199 | +}) |
| 200 | +``` |
| 201 | + |
| 202 | +### Key test cases (each route: ≥1 success + ≥1 error/edge case) |
| 203 | + |
| 204 | +| Route | Success case | Error/edge case | |
| 205 | +|-------|-------------|-----------------| |
| 206 | +| GET /health | 200, `{status:"ok", pid:N}` | — | |
| 207 | +| GET /status | 200, has `solution`, `ready`, `updateCount` | — | |
| 208 | +| GET /diagnostics | 200, has `files[]` | `?file=` filters correctly | |
| 209 | +| GET /diagnostics/summary | 200, has `errors`, `warnings`, `info`, `hints` | — | |
| 210 | +| POST /file-changed | 200, `{ok:true, action:"didSave", path}` | missing path/uri → 400; nonexistent file → 404; `file://` URI normalized | |
| 211 | +| POST /file-content | 200, `{ok:true, action:"didChange", path}` | missing content → 400; non-string content → 400 | |
| 212 | +| POST /stop | 200, `{ok:true, message:"Daemon stopping"}` | — | |
| 213 | +| Unknown | 404, `{error:"Not found"}` | — | |
| 214 | +| Error resilience | After bad JSON body → 500, server still serves /health | — | |
| 215 | + |
| 216 | +### AX-aligned test: localhost only |
| 217 | +Instead of source-text matching, assert the server `hostname` config: |
| 218 | +```typescript |
| 219 | +test("server binds to 127.0.0.1 only", () => { |
| 220 | + expect(serverConfig.hostname).toBe("127.0.0.1"); |
| 221 | +}); |
| 222 | +``` |
| 223 | + |
| 224 | +### Security tests to keep (rewritten as behavior, not string match) |
| 225 | +- Localhost binding: verify real `fetch()` to `http://0.0.0.0:PORT/health` fails or is refused |
| 226 | +- No CORS headers: check real response headers don't include `Access-Control-Allow-Origin` |
| 227 | + |
| 228 | +--- |
| 229 | + |
| 230 | +## Verification |
| 231 | + |
| 232 | +```bash |
| 233 | +# TypeScript clean |
| 234 | +bun run tsc --noEmit |
| 235 | + |
| 236 | +# All tests pass |
| 237 | +bun test --timeout 60000 |
| 238 | + |
| 239 | +# CLI smoke test |
| 240 | +vslsp --help | grep uninstall-mapper |
| 241 | +vslsp uninstall-mapper rust # "not installed" or removes binary |
| 242 | +vslsp uninstall-mapper unknown # exits 1 with supported langs message |
| 243 | + |
| 244 | +# AX doc exists and is readable |
| 245 | +cat docs/AX.md |
| 246 | +``` |
| 247 | + |
| 248 | +--- |
| 249 | + |
| 250 | +## Merge Strategy |
| 251 | + |
| 252 | +After all three agents complete: |
| 253 | +1. Review diffs from each worktree |
| 254 | +2. Merge W1 (docs) first — no conflicts possible |
| 255 | +3. Merge W2 (CLI) — single file, no conflicts expected |
| 256 | +4. Merge W3 (tests) — single file, no conflicts expected |
| 257 | +5. Run `bun run tsc --noEmit` and `bun test --timeout 60000` on merged result |
| 258 | +6. Tag v1.6.0 |
| 259 | + |
| 260 | +--- |
| 261 | + |
| 262 | +## Open Questions (resolve before executing) |
| 263 | + |
| 264 | +1. Does `registry.ts` expose absolute paths for `installDir`? If so, `homedir()` import is not needed in `uninstallMapper`. **Read `src/code-mapping/registry.ts` at execution time to confirm.** |
| 265 | +2. Does `createHttpServer()` return the server instance (needed for `afterAll`)? **Read `src/diagnostics/http.ts:15-25` to confirm.** If not, the stop route (`POST /stop`) serves as teardown — test it last. |
| 266 | +3. Port for HTTP tests: use a fixed test-only port (e.g. `7860`) or find a free port dynamically? Prefer fixed to avoid flakiness, but check it doesn't conflict with daemon default (`7850`). |
0 commit comments