From 9517f8d98d793b6a8eb03d4f2c08f16d56adbc0a Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 17 Aug 2026 05:41:07 +0000 Subject: [PATCH 1/2] chore(specs): gardener checkbox sync --- specs/098-tools-preflight/tasks.md | 52 +++++++++++++++--------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/specs/098-tools-preflight/tasks.md b/specs/098-tools-preflight/tasks.md index 372d0e46..46720e25 100644 --- a/specs/098-tools-preflight/tasks.md +++ b/specs/098-tools-preflight/tasks.md @@ -7,43 +7,43 @@ ## Phase 1: Setup - [ ] T001 Create `internal/preflight` package skeleton (`reasons.go`, `evaluator.go`, doc comment stating zero-I/O invariant and the error-not-fabricated-reason rule) per plan.md structure -- [ ] T002 [P] Capture tools/list golden snapshots for all three routing modes **from merge-base (origin/main)** into `internal/server/testdata/toolslist_goldens/` + snapshot test `internal/server/toolslist_snapshot_test.go` comparing the feature branch byte-for-byte (FR-015; goldens must predate any dispatch refactor) +- [x] T002 [P] Capture tools/list golden snapshots for all three routing modes **from merge-base (origin/main)** into `internal/server/testdata/toolslist_goldens/` + snapshot test `internal/server/toolslist_snapshot_test.go` comparing the feature branch byte-for-byte (FR-015; goldens must predate any dispatch refactor) ## Phase 2: Foundational (blocking all stories) -- [ ] T003 Define the 15-code reason enum, classes, retryable defaults, action mapping (action omitted = ""), precedence chain, set-verdict + exit-code mapping in `internal/preflight/reasons.go` as single source of truth (data-model tables), with table-driven test `internal/preflight/reasons_test.go` asserting the full FR-003 table -- [ ] T004 Define narrow read interfaces + `EvalContext`/`Result` types in `internal/preflight/evaluator.go` (`IndexReader`, `ApprovalReader`, `StateReader`, `ConfigPolicy`, tier, profile scope, filters, pins); `Evaluate` returns `([]Result, error)` — infra read errors are errors, never reason codes -- [ ] T005 Extract the spec 094 annotation classifier (`excludeReason` logic) from `internal/server/mcp_annotations.go` into a shared lower-level package (`internal/toolannotations`) with `internal/server` delegating; unit tests moved/extended (unexported-symbol cycle fix, review finding 14) -- [ ] T006 Implement `ClassifyTool` shared classification in `internal/preflight` (quarantine-enabled/skip flags honored; `changed` distinct from `pending`; `auto_approve_tool_changes` ⇒ ready) with unit tests covering the documented divergences (research D2) -- [ ] T007 Implement `Evaluate` walking the FR-004 precedence chain, incl. PendingAuth→`oauth_required` explicit map, server-level `server_initializing`, pin check (`sha256/v{N}:{hex}`, schema-version aware), annotation slot via `internal/toolannotations`; table-driven tests: every enum cell + co-occurrence pairs per adjacent precedence pair in `internal/preflight/evaluator_test.go` -- [ ] T008 Implement scope/tier disclosure: agent-token tier out-of-scope ⇒ byte-indistinguishable `not_found` (serialized-bytes comparison test); operator tier ⇒ `server_not_in_scope` with profile-session detail; profile semantics = shared index existence + profile scope filter, **no `ForProfile` calls** (FR-010/FR-013, review findings 9/10/13) -- [ ] T009 [P] Implement `did_you_mean` helper (prefix + Levenshtein ≤2, ≤3 suggestions, caller-visible names only, quarantined-server names excluded) in `internal/preflight/suggest.go` + tests -- [ ] T010 Propagate agent-token `ProfilePin` through REST auth into the evaluation context (`internal/auth/agent_token.go`, `internal/httpapi/server.go`); evaluation scope = token scope ∩ token pin ∩ requested profile; tests for each intersection case (review finding 11) -- [ ] T011 Mirror DTOs + reason constants into `internal/contracts/types.go` per data-model.md; add the new types to the `cmd/generate-types` **generator source** and regen `frontend/src/types/contracts.ts`; anti-drift unit test (preflight enum ≡ contracts constants) -- [ ] T012 Glue: `internal/server/preflight_glue.go` — build EvalContext from index manager, storage, stateview snapshot, config, resolved profile scope; expose `RunPreflight` via ServerController (`internal/httpapi/server.go`); explicitly no `serverToolNames` (live-ListTools fallback); test with instrumented transport asserting zero upstream calls **plus before/after snapshots of runtime/index/config/approval state proving no mutation** (FR-006, review finding 17) -- [ ] T013 Dispatch consolidation onto shared primitives across all four paths: `handleCallToolVariant` (internal/server/mcp.go), direct-mode callability (`internal/server/mcp_direct_callability.go`), code_execution dispatch, stored-script dispatch; refactor `classifyServerToolStatus` + `describeGateReason` to delegate; FR-002 contract tests: two-way equivalence for shared policy gates, one-way (refusal ⇒ non-ready) for fail-open existence gates, covering `auto_approve_tool_changes` and quarantine-skip cases (review findings 1/2/3) -- [ ] T014 Activity seam: add activity type `preflight` to `internal/storage/activity_models.go` allowlist with Metadata payload `{verdict, reasons{code:count}, per_tool[{id,status,reason?}]}` (RequestID first-class, existing status vocabulary); add synchronous durable `RecordPreflight` to `internal/runtime/activity_service.go` (bypasses the bounded async channel); storage + service unit tests incl. write-failure propagation (FR-014, review findings 4/5/15) +- [x] T003 Define the 15-code reason enum, classes, retryable defaults, action mapping (action omitted = ""), precedence chain, set-verdict + exit-code mapping in `internal/preflight/reasons.go` as single source of truth (data-model tables), with table-driven test `internal/preflight/reasons_test.go` asserting the full FR-003 table +- [x] T004 Define narrow read interfaces + `EvalContext`/`Result` types in `internal/preflight/evaluator.go` (`IndexReader`, `ApprovalReader`, `StateReader`, `ConfigPolicy`, tier, profile scope, filters, pins); `Evaluate` returns `([]Result, error)` — infra read errors are errors, never reason codes +- [x] T005 Extract the spec 094 annotation classifier (`excludeReason` logic) from `internal/server/mcp_annotations.go` into a shared lower-level package (`internal/toolannotations`) with `internal/server` delegating; unit tests moved/extended (unexported-symbol cycle fix, review finding 14) +- [x] T006 Implement `ClassifyTool` shared classification in `internal/preflight` (quarantine-enabled/skip flags honored; `changed` distinct from `pending`; `auto_approve_tool_changes` ⇒ ready) with unit tests covering the documented divergences (research D2) +- [x] T007 Implement `Evaluate` walking the FR-004 precedence chain, incl. PendingAuth→`oauth_required` explicit map, server-level `server_initializing`, pin check (`sha256/v{N}:{hex}`, schema-version aware), annotation slot via `internal/toolannotations`; table-driven tests: every enum cell + co-occurrence pairs per adjacent precedence pair in `internal/preflight/evaluator_test.go` +- [x] T008 Implement scope/tier disclosure: agent-token tier out-of-scope ⇒ byte-indistinguishable `not_found` (serialized-bytes comparison test); operator tier ⇒ `server_not_in_scope` with profile-session detail; profile semantics = shared index existence + profile scope filter, **no `ForProfile` calls** (FR-010/FR-013, review findings 9/10/13) +- [x] T009 [P] Implement `did_you_mean` helper (prefix + Levenshtein ≤2, ≤3 suggestions, caller-visible names only, quarantined-server names excluded) in `internal/preflight/suggest.go` + tests +- [x] T010 Propagate agent-token `ProfilePin` through REST auth into the evaluation context (`internal/auth/agent_token.go`, `internal/httpapi/server.go`); evaluation scope = token scope ∩ token pin ∩ requested profile; tests for each intersection case (review finding 11) +- [x] T011 Mirror DTOs + reason constants into `internal/contracts/types.go` per data-model.md; add the new types to the `cmd/generate-types` **generator source** and regen `frontend/src/types/contracts.ts`; anti-drift unit test (preflight enum ≡ contracts constants) +- [x] T012 Glue: `internal/server/preflight_glue.go` — build EvalContext from index manager, storage, stateview snapshot, config, resolved profile scope; expose `RunPreflight` via ServerController (`internal/httpapi/server.go`); explicitly no `serverToolNames` (live-ListTools fallback); test with instrumented transport asserting zero upstream calls **plus before/after snapshots of runtime/index/config/approval state proving no mutation** (FR-006, review finding 17) +- [x] T013 Dispatch consolidation onto shared primitives across all four paths: `handleCallToolVariant` (internal/server/mcp.go), direct-mode callability (`internal/server/mcp_direct_callability.go`), code_execution dispatch, stored-script dispatch; refactor `classifyServerToolStatus` + `describeGateReason` to delegate; FR-002 contract tests: two-way equivalence for shared policy gates, one-way (refusal ⇒ non-ready) for fail-open existence gates, covering `auto_approve_tool_changes` and quarantine-skip cases (review findings 1/2/3) +- [x] T014 Activity seam: add activity type `preflight` to `internal/storage/activity_models.go` allowlist with Metadata payload `{verdict, reasons{code:count}, per_tool[{id,status,reason?}]}` (RequestID first-class, existing status vocabulary); add synchronous durable `RecordPreflight` to `internal/runtime/activity_service.go` (bypasses the bounded async channel); storage + service unit tests incl. write-failure propagation (FR-014, review findings 4/5/15) ## Phase 3: User Story 1 — CLI preflight for cron/CI (P1) 🎯 MVP **Goal**: `mcpproxy tools preflight` exits 0/10/11/12 with per-tool verdicts; endpoint + activity land together. **Independent test**: quickstart §2–§3 CLI cells against isolated instance. -- [ ] T015 [US1] REST handler `internal/httpapi/preflight.go` with swag annotations (standard `APIResponse{data}` envelope, existing security schemes): validation (empty list 400, raw >100 entries 400, conflicting duplicate pins 400, unknown profile 400, wait_ms range 400, runtime-unavailable + evaluator infra error 503), dedup preserving first-occurrence order, tier detection (API key/socket/pipe vs agent token), **synchronous `RecordPreflight` before every 200**; route registration; handler tests for every 400/503 rule and the no-record-on-reject rule (FR-008/FR-014; review findings 6/17/18/20) -- [ ] T016 [US1] wait_ms poll loop in handler (floor 250 ms, cap 10 s, early-terminate on non-retryable, always resolves at deadline) bounded by a dedicated preflight-wait semaphore (exhausted ⇒ immediate resolve, `waited_ms: 0`); tests incl. deadline, early-termination, semaphore-exhausted degrade (FR-012, review finding 8) -- [ ] T017 [US1] Typed preflight exit-code error + central classification in `cmd/mcpproxy/exit_codes.go` + `main.go` error mapping; `cliclient` Preflight method; tests (review finding 21) -- [ ] T018 [US1] CLI `tools preflight` subcommand in `cmd/mcpproxy/tools_cmd.go`: args = tool IDs, flags `--profile`, `--pin id=hash` (repeatable), `--read-only-only`, `--exclude-destructive`, `--exclude-open-world`, `--wait`, `-o json|yaml|table` + `MCPPROXY_OUTPUT`; exit codes 0/10/11/12 worst-class-wins, transport errors exit 1; `--help-json` metadata; unit tests for exit-code precedence (12>11>10), env output, formats (FR-009) -- [ ] T019 [US1] Benchmarks: evaluator micro-benchmark in `internal/preflight/bench_test.go` + normative handler-level benchmark (incl. response encoding + activity-record build) in `internal/httpapi/preflight_bench_test.go`; SC-002 asserted as committed benchmark with generous CI threshold, not a brittle wall-clock gate (review findings 16/25) +- [x] T015 [US1] REST handler `internal/httpapi/preflight.go` with swag annotations (standard `APIResponse{data}` envelope, existing security schemes): validation (empty list 400, raw >100 entries 400, conflicting duplicate pins 400, unknown profile 400, wait_ms range 400, runtime-unavailable + evaluator infra error 503), dedup preserving first-occurrence order, tier detection (API key/socket/pipe vs agent token), **synchronous `RecordPreflight` before every 200**; route registration; handler tests for every 400/503 rule and the no-record-on-reject rule (FR-008/FR-014; review findings 6/17/18/20) +- [x] T016 [US1] wait_ms poll loop in handler (floor 250 ms, cap 10 s, early-terminate on non-retryable, always resolves at deadline) bounded by a dedicated preflight-wait semaphore (exhausted ⇒ immediate resolve, `waited_ms: 0`); tests incl. deadline, early-termination, semaphore-exhausted degrade (FR-012, review finding 8) +- [x] T017 [US1] Typed preflight exit-code error + central classification in `cmd/mcpproxy/exit_codes.go` + `main.go` error mapping; `cliclient` Preflight method; tests (review finding 21) +- [x] T018 [US1] CLI `tools preflight` subcommand in `cmd/mcpproxy/tools_cmd.go`: args = tool IDs, flags `--profile`, `--pin id=hash` (repeatable), `--read-only-only`, `--exclude-destructive`, `--exclude-open-world`, `--wait`, `-o json|yaml|table` + `MCPPROXY_OUTPUT`; exit codes 0/10/11/12 worst-class-wins, transport errors exit 1; `--help-json` metadata; unit tests for exit-code precedence (12>11>10), env output, formats (FR-009) +- [x] T019 [US1] Benchmarks: evaluator micro-benchmark in `internal/preflight/bench_test.go` + normative handler-level benchmark (incl. response encoding + activity-record build) in `internal/httpapi/preflight_bench_test.go`; SC-002 asserted as committed benchmark with generous CI threshold, not a brittle wall-clock gate (review findings 16/25) ## Phase 4: User Story 2 — REST harness extras (P1) -- [ ] T020 [US2] Hash-pin authoring surface: expose approval `CurrentHash` + `HashSchemaVersion` as `sha256/v{N}:{hex}` on the operator-tier per-tool REST payload and `tools list -o json` (explicit contract change; generator + swagger + disclosure tests — never exposed to agent-token tier) (FR-011, review finding 22) -- [ ] T021 [US2] OAS: `make swagger` regen from annotations, `scripts/verify-oas.sh` + `make swagger-verify` clean; server-edition gates: `go build -tags server ./cmd/mcpproxy`, `go test -tags server ./internal/serveredition/... -race`, `golangci-lint --build-tags server` (review findings 6/14-oas) +- [x] T020 [US2] Hash-pin authoring surface: expose approval `CurrentHash` + `HashSchemaVersion` as `sha256/v{N}:{hex}` on the operator-tier per-tool REST payload and `tools list -o json` (explicit contract change; generator + swagger + disclosure tests — never exposed to agent-token tier) (FR-011, review finding 22) +- [x] T021 [US2] OAS: `make swagger` regen from annotations, `scripts/verify-oas.sh` + `make swagger-verify` clean; server-edition gates: `go build -tags server ./cmd/mcpproxy`, `go test -tags server ./internal/serveredition/... -race`, `golangci-lint --build-tags server` (review findings 6/14-oas) ## Phase 5: User Story 3 — Activity browsability (P2) -- [ ] T022 [US3] CLI `activity list` renders type `preflight` (allowlist + verdict summary from Metadata) in `cmd/mcpproxy/activity_cmd.go`; tests (review finding 16) -- [ ] T023 [P] [US3] Frontend: extend activity type union/filter menu in `frontend/src/types/api.ts` + activity view rendering of preflight verdict; Playwright web-ui verification per docs/development/web-ui-verification.md (review finding 16) +- [x] T022 [US3] CLI `activity list` renders type `preflight` (allowlist + verdict summary from Metadata) in `cmd/mcpproxy/activity_cmd.go`; tests (review finding 16) +- [x] T023 [P] [US3] Frontend: extend activity type union/filter menu in `frontend/src/types/api.ts` + activity view rendering of preflight verdict; Playwright web-ui verification per docs/development/web-ui-verification.md (review finding 16) ## Phase 6: User Story 4 — Non-regression (P3) @@ -52,14 +52,14 @@ ## Phase 7: Sabotage E2E matrix (acceptance gate) -- [ ] T026 Committed scenario-keyed matrix (`internal/server/testdata/preflight_sabotage_matrix.json`: scenario → expected {reason, retryable, action}) + E2E `internal/server/preflight_e2e_test.go` driving ctl-server fixtures (DESC_FILE rug-pull): quarantine flip, tool-definition drift, tool block, config denial, server disable, SIGSTOP/kill, mid-indexing, missing/explicit annotation per each of the three filters, unknown ID, unknown server, hash mismatch + schema-version-bump variant, PendingAuth, profile out-of-scope at both tiers; independent assertions per row PLUS reflection check that every enum code appears in ≥1 row; after each cell, `activity list --request-id` lookup asserts the preflight record (FR-016, SC-005; review findings 23/22-activity) +- [x] T026 Committed scenario-keyed matrix (`internal/server/testdata/preflight_sabotage_matrix.json`: scenario → expected {reason, retryable, action}) + E2E `internal/server/preflight_e2e_test.go` driving ctl-server fixtures (DESC_FILE rug-pull): quarantine flip, tool-definition drift, tool block, config denial, server disable, SIGSTOP/kill, mid-indexing, missing/explicit annotation per each of the three filters, unknown ID, unknown server, hash mismatch + schema-version-bump variant, PendingAuth, profile out-of-scope at both tiers; independent assertions per row PLUS reflection check that every enum code appears in ≥1 row; after each cell, `activity list --request-id` lookup asserts the preflight record (FR-016, SC-005; review findings 23/22-activity) - [ ] T027 Scripted incident-diagnosis scenario (SC-006): tool quarantined between runs → preflight names `tool_changed`/`server_quarantined` in ≤1 step; committed as an E2E assertion ## Phase 8: Docs & Polish -- [ ] T028 [P] `docs/api/rest-api.md`: endpoint reference (envelope, tiers, 400/503 rules, wait semantics) (FR-017) -- [ ] T029 [P] `docs/cli-management-commands.md`: `tools preflight` reference with exit-code table and cron/CI recipe (FR-017) -- [ ] T030 [P] NEW `docs/features/tools-preflight.md`: concept, taxonomy + precedence tables, disclosure tiers, transparency/activity story, cron + GitHub Actions + n8n recipes, composition with code_execution/stored scripts (REST-from-harness pattern), Phase-2+ roadmap (FR-017) +- [x] T028 [P] `docs/api/rest-api.md`: endpoint reference (envelope, tiers, 400/503 rules, wait semantics) (FR-017) +- [x] T029 [P] `docs/cli-management-commands.md`: `tools preflight` reference with exit-code table and cron/CI recipe (FR-017) +- [x] T030 [P] NEW `docs/features/tools-preflight.md`: concept, taxonomy + precedence tables, disclosure tiers, transparency/activity story, cron + GitHub Actions + n8n recipes, composition with code_execution/stored scripts (REST-from-harness pattern), Phase-2+ roadmap (FR-017) - [ ] T031 [P] Usage-examples expansion: README/docs agent-workflow examples — token-saving discovery flow, typical agent actions through mcpproxy, preflight-gated automation example (FR-017) - [ ] T032 Full gates: `go test -race ./...` (incl. cmd/ CLI tests), server-edition build+test+lint (T021 set), `./scripts/test-api-e2e.sh`, golangci-lint v2 `.github/.golangci.yml`, swagger + generate-types diff-clean - [ ] T033 Cross-model review of the full diff (opencode gpt-5.6-sol), fix→re-review ≤5 rounds; then quickstart walkthrough end-to-end on the isolated instance From e996b81ab9471e6060777431e71a9ce5e26d84c0 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 17 Aug 2026 05:41:12 +0000 Subject: [PATCH 2/2] chore(roadmap): regenerate ROADMAP.md after gardener checkbox sync --- ROADMAP.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ROADMAP.md b/ROADMAP.md index 3784b02e..4643c721 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -799,5 +799,5 @@ Legend: `shipped` ≥95% checked · `in-flight` 1–94% · `drafted` 0% · `—` | [095-update-failure-ux](./specs/095-update-failure-ux/) | `shipped` | 28/28 (100%) | | [096-batched-call-tools](./specs/096-batched-call-tools/) | `in-flight` | 15/16 (94%) | | [097-stored-scripts](./specs/097-stored-scripts/) | `in-flight` | 13/14 (93%) | -| [098-tools-preflight](./specs/098-tools-preflight/) | `drafted` | 0/33 (0%) | +| [098-tools-preflight](./specs/098-tools-preflight/) | `in-flight` | 26/33 (79%) | | [099-describe-check-mode](./specs/099-describe-check-mode/) | `in-flight` | 9/10 (90%) |