diff --git a/.github/workflows/prerelease.yml b/.github/workflows/prerelease.yml index 4c97daf7e..616712778 100644 --- a/.github/workflows/prerelease.yml +++ b/.github/workflows/prerelease.yml @@ -888,8 +888,11 @@ jobs: # Create a flat structure to avoid duplicates mkdir -p release-files - # Copy archives (tar.gz and zip files) - only versioned, no latest for prereleases - find dist -name "*.tar.gz" -o -name "*.zip" | while read file; do + # Copy archives (tar.gz and zip files) - only versioned, no latest for prereleases. + # The QA gate runs as a reusable workflow of THIS run, so its artifacts land + # in dist/ too; its Playwright HTML report stores traces as + # playwright-report/data/.zip and must never become a release asset. + find dist -not -path "*/web-ui-sweep-playwright-report/*" \( -name "*.tar.gz" -o -name "*.zip" \) | while read file; do filename=$(basename "$file") cp "$file" "release-files/$filename" done diff --git a/.github/workflows/release-qa-gate.yml b/.github/workflows/release-qa-gate.yml index 953fa4f4f..d654cf757 100644 --- a/.github/workflows/release-qa-gate.yml +++ b/.github/workflows/release-qa-gate.yml @@ -16,9 +16,13 @@ name: Release QA Gate # manually against any ref (workflow_dispatch — a dry run that publishes # nothing and never counts as qualification for a later tag, FR-001a). # -# T2/T3/T4 extension slots (reserved manifest entries, recorded as +# T2 landed as an ADVISORY entry — it runs on every tag build and is reported, +# but cannot block publication (`continue-on-error` + a non-blocking manifest +# entry): +# advisory/web-ui-sweep — T2 Playwright sweep against the candidate +# +# T3/T4 extension slots (reserved manifest entries, recorded as # not-run/"not-implemented-yet" until their stage lands): -# reserved/web-ui-sweep — T2 Playwright sweep against the candidate # reserved/macos-smoke — T3 macOS tray smoke (advisory, FR-019/020) # reserved/surface-consistency — T4 REST/CLI/Web-UI/tray state agreement # See docs/development/release-gate.md. @@ -374,6 +378,76 @@ jobs: retention-days: 1 if-no-files-found: warn + # --------------------------------------------------------------------------- + # T2 (US3, FR-016): Playwright Web UI sweep against the Web UI served by the + # CANDIDATE binary (embedded frontend, not a dev server), with a live stdio + # mcpfixture upstream so the servers/tools screens have real data. + # + # ADVISORY, deliberately: `continue-on-error: true` keeps a red sweep out of + # this reusable workflow's conclusion, so a publisher whose job `needs:` the + # gate still publishes. The sweep is nonetheless REPORTED — the driver runs it + # with --advisory, which records `advisory/web-ui-sweep` as `advisory-fail`, + # and the verdict job lists it under advisory_failures. Promote to blocking by + # flipping that manifest entry to Blocking:true and dropping continue-on-error. + # + # Setup is not reinvented here: scripts/run-web-smoke.sh is the same launcher + # used by hand (docs/development/web-ui-verification.md) — it boots a + # throwaway instance, installs Chromium, and runs e2e/web-ui-sweep. + # --------------------------------------------------------------------------- + web-ui-sweep: + name: Web UI sweep (advisory) + needs: build-candidate + runs-on: ubuntu-latest + timeout-minutes: 20 + continue-on-error: true # advisory: never blocks the tag (FR-019 pattern) + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ inputs.ref || github.sha }} + + - name: Set up Node.js # the sweep installs @playwright/test + Chromium + uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0 + with: + node-version: ${{ env.NODE_VERSION }} + + - name: Download candidate binaries + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: gate-candidate + path: dist-bin + + - name: Stage binaries + run: chmod +x dist-bin/* + + - name: Run Web UI sweep + env: + MCPPROXY_BINARY_PATH: ${{ github.workspace }}/dist-bin/mcpproxy + MCPPROXY_FIXTURE_PATH: ${{ github.workspace }}/dist-bin/mcpfixture + ARTIFACT_DIR: ${{ github.workspace }}/tmp/web-smoke-artifacts + run: | + ./dist-bin/release-gate run-suite \ + --name advisory/web-ui-sweep --advisory --report-dir "${GATE_REPORT_DIR}" \ + -- bash scripts/run-web-smoke.sh + + - name: Upload Playwright report + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: web-ui-sweep-playwright-report + path: tmp/web-smoke-artifacts/ + retention-days: 14 + if-no-files-found: warn + + - name: Upload report fragment + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: gate-fragment-web-ui-sweep + path: ${{ env.GATE_REPORT_DIR }}/ + retention-days: 1 + if-no-files-found: warn + # --------------------------------------------------------------------------- # Merge every fragment against the hardcoded gatereport manifest → one # verdict. Runs even if upstream jobs failed (if: always()) so a missing @@ -383,7 +457,7 @@ jobs: # --------------------------------------------------------------------------- verdict: name: Gate verdict - needs: [build-candidate, suite-api-e2e, suite-race, suite-scan-eval, matrix-invariants] + needs: [build-candidate, suite-api-e2e, suite-race, suite-scan-eval, matrix-invariants, web-ui-sweep] if: always() runs-on: ubuntu-latest timeout-minutes: 10 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 98ce9ca12..45ea95d87 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -1298,8 +1298,14 @@ jobs: # Create a flat structure to avoid duplicates mkdir -p release-files - # Copy archives (tar.gz and zip files) - find dist -name "*.tar.gz" -o -name "*.zip" | while read file; do + # Copy archives (tar.gz and zip files). + # The QA gate runs as a reusable workflow of THIS run, so its artifacts + # land in dist/ too. Its Playwright HTML report stores traces as + # playwright-report/data/.zip (retained on failure — precisely the + # case the advisory sweep tolerates), which would otherwise be published + # as unnamed release assets. Excluded here, not upstream, so the report + # stays downloadable as a CI artifact for post-mortems. + find dist -not -path "*/web-ui-sweep-playwright-report/*" \( -name "*.tar.gz" -o -name "*.zip" \) | while read file; do filename=$(basename "$file") cp "$file" "release-files/$filename" done diff --git a/ROADMAP.md b/ROADMAP.md index 4643c721d..c08ca0a79 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -105,15 +105,17 @@ graph LR release_qa_gate_matrix --> release_qa_gate_consistency classDef done fill:#1f7a1f,stroke:#0d3d0d,color:#ffffff; + classDef in_review fill:#9a6700,stroke:#5c3d00,color:#ffffff; classDef todo fill:#6e7781,stroke:#3d4248,color:#ffffff; class release_qa_gate_matrix done; - class release_qa_gate_playwright,release_qa_gate_macos,release_qa_gate_consistency todo; + class release_qa_gate_playwright in_review; + class release_qa_gate_macos,release_qa_gate_consistency todo; ``` | Task | Status | Refs | | --- | --- | --- | | T1: tag-blocking release-gate workflow: server-type matrix (stdio/http/sse/docker/oauth) + invariants (activity-log/request-id, token+telemetry counters, quarantine flow, reconnect, upgrade-in-place), publish jobs gated on the verdict, scan-eval unconditional on tags | 🟢 Done | #819 | -| T2: wire the Playwright Web UI sweep into the gate (currently manual-trigger only) | ⚪ Todo | — | +| T2: wire the Playwright Web UI sweep into the gate (currently manual-trigger only) | 🟡 In review | — | | T3: macOS app smoke on a macos runner, advisory until 3 consecutive passes (today zero CI automation for the tray app) | ⚪ Todo | — | | T4: surface-state consistency check (tray/Web UI/CLI agree with core on server states) | ⚪ Todo | — | @@ -801,3 +803,4 @@ Legend: `shipped` ≥95% checked · `in-flight` 1–94% · `drafted` 0% · `—` | [097-stored-scripts](./specs/097-stored-scripts/) | `in-flight` | 13/14 (93%) | | [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%) | +| [100-prompt-rugpull-baseline](./specs/100-prompt-rugpull-baseline/) | — | — | diff --git a/cmd/release-gate/main.go b/cmd/release-gate/main.go index 5a7dc0af4..e14b4cbbe 100644 --- a/cmd/release-gate/main.go +++ b/cmd/release-gate/main.go @@ -76,7 +76,7 @@ usage: [--work-dir DIR] [--cell-timeout 300s] release-gate invariants --state-file FILE --report-dir DIR [--prev-binary PATH] [--skip-upgrade] \ [--upgrade-repo owner/repo] [--keep-core] - release-gate run-suite --name suite/api-e2e --report-dir DIR -- CMD [ARGS...] + release-gate run-suite --name suite/api-e2e --report-dir DIR [--advisory] -- CMD [ARGS...] release-gate report --report-dir DIR [--out gate-report.json] [--summary summary.md] `) } @@ -128,6 +128,7 @@ func cmdRunSuite(ctx context.Context, args []string) (bool, error) { fs := flag.NewFlagSet("run-suite", flag.ExitOnError) name := fs.String("name", "", "manifest entry name (e.g. suite/api-e2e)") reportDir := fs.String("report-dir", "", "directory for report fragments (required)") + advisory := fs.Bool("advisory", false, "non-blocking entry: record failures as advisory-fail") if err := fs.Parse(args); err != nil { return false, err } @@ -138,7 +139,7 @@ func cmdRunSuite(ctx context.Context, args []string) (bool, error) { if len(cmdArgs) == 0 { return false, fmt.Errorf("no command given after flags (use: run-suite --name N --report-dir D -- CMD ARGS)") } - return runSuite(ctx, *name, *reportDir, cmdArgs) + return runSuite(ctx, *name, *reportDir, *advisory, cmdArgs) } func mustAbs(p string) string { diff --git a/cmd/release-gate/suite.go b/cmd/release-gate/suite.go index 0685109b2..418b68e3c 100644 --- a/cmd/release-gate/suite.go +++ b/cmd/release-gate/suite.go @@ -14,7 +14,13 @@ import ( // test-api-e2e.sh, go race tests, scan-eval --gate) and records its outcome // as a report fragment. The command's output streams through so CI logs stay // useful. -func runSuite(ctx context.Context, name, reportDir string, cmdArgs []string) (bool, error) { +// +// advisory marks a non-blocking entry (Spec 081 T2 Web UI sweep, T3 macOS +// smoke): its failures are recorded as `advisory-fail` so the merged report +// distinguishes "went red but cannot block the tag" from a real blocking +// failure. The returned ok is false either way — the CI job still goes red +// (under continue-on-error) so a maintainer sees it. +func runSuite(ctx context.Context, name, reportDir string, advisory bool, cmdArgs []string) (bool, error) { frag := &gatereport.Fragment{Name: name, StartedAt: time.Now().UTC()} cmd := exec.CommandContext(ctx, cmdArgs[0], cmdArgs[1:]...) cmd.Stdout = os.Stdout @@ -26,6 +32,9 @@ func runSuite(ctx context.Context, name, reportDir string, cmdArgs []string) (bo frag.DurationMS = frag.FinishedAt.Sub(frag.StartedAt).Milliseconds() if err != nil { frag.Status = gatereport.StatusFail + if advisory { + frag.Status = gatereport.StatusAdvisoryFail + } frag.Reason = fmt.Sprintf("suite command failed: %v", err) frag.Classification = gatereport.ClassificationProduct } else { diff --git a/cmd/release-gate/suite_test.go b/cmd/release-gate/suite_test.go new file mode 100644 index 000000000..8ed13dccd --- /dev/null +++ b/cmd/release-gate/suite_test.go @@ -0,0 +1,70 @@ +package main + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/gatereport" +) + +func readFragment(t *testing.T, dir, name string) gatereport.Fragment { + t.Helper() + data, err := os.ReadFile(filepath.Join(dir, gatereport.FragmentFileName(name))) + if err != nil { + t.Fatalf("read fragment %s: %v", name, err) + } + var frag gatereport.Fragment + if err := json.Unmarshal(data, &frag); err != nil { + t.Fatalf("parse fragment %s: %v", name, err) + } + return frag +} + +// A failing advisory suite must be recorded as advisory-fail (never plain +// fail), so the merged report reads honestly: the check ran, it went red, and +// it did not block the tag (Spec 081 T2/FR-019). +func TestRunSuite_AdvisoryFailure_RecordsAdvisoryFail(t *testing.T) { + dir := t.TempDir() + ok, err := runSuite(context.Background(), gatereport.EntryAdvisoryWebUISweep, dir, true, + []string{"sh", "-c", "exit 3"}) + if err != nil { + t.Fatalf("runSuite returned an error: %v", err) + } + if ok { + t.Error("a failing suite must report ok=false even when advisory (the job goes red under continue-on-error)") + } + frag := readFragment(t, dir, gatereport.EntryAdvisoryWebUISweep) + if frag.Status != gatereport.StatusAdvisoryFail { + t.Errorf("status=%s want %s", frag.Status, gatereport.StatusAdvisoryFail) + } + if frag.Reason == "" { + t.Error("a non-pass fragment must carry a reason (FR-004)") + } +} + +func TestRunSuite_AdvisorySuccess_RecordsPass(t *testing.T) { + dir := t.TempDir() + ok, err := runSuite(context.Background(), gatereport.EntryAdvisoryWebUISweep, dir, true, + []string{"sh", "-c", "exit 0"}) + if err != nil || !ok { + t.Fatalf("runSuite ok=%v err=%v, want true/nil", ok, err) + } + if frag := readFragment(t, dir, gatereport.EntryAdvisoryWebUISweep); frag.Status != gatereport.StatusPass { + t.Errorf("status=%s want %s", frag.Status, gatereport.StatusPass) + } +} + +// Blocking suites keep the original behaviour: a failure is a plain fail. +func TestRunSuite_BlockingFailure_RecordsFail(t *testing.T) { + dir := t.TempDir() + if _, err := runSuite(context.Background(), gatereport.EntrySuiteAPIE2E, dir, false, + []string{"sh", "-c", "exit 1"}); err != nil { + t.Fatalf("runSuite returned an error: %v", err) + } + if frag := readFragment(t, dir, gatereport.EntrySuiteAPIE2E); frag.Status != gatereport.StatusFail { + t.Errorf("status=%s want %s", frag.Status, gatereport.StatusFail) + } +} diff --git a/cmd/release-gate/webui_sweep_audit_test.go b/cmd/release-gate/webui_sweep_audit_test.go new file mode 100644 index 000000000..3844b97f4 --- /dev/null +++ b/cmd/release-gate/webui_sweep_audit_test.go @@ -0,0 +1,146 @@ +package main + +import ( + "path/filepath" + "strings" + "testing" + + "gopkg.in/yaml.v3" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/gatereport" +) + +// gateWorkflowPath is the reusable gate workflow the publishers call. go test +// runs with cmd/release-gate as the working directory. +var gateWorkflowPath = filepath.Join("..", "..", ".github", "workflows", "release-qa-gate.yml") + +const webUISweepJob = "web-ui-sweep" + +// TestWebUISweepJobIsAdvisory is the Spec 081 T2 wiring audit: the Playwright +// Web UI sweep must run inside the gate (so it runs on every tag build, since +// release.yml / prerelease.yml call this workflow) while being incapable of +// blocking the release. +// +// Advisory is enforced structurally: `continue-on-error: true` on the job. A +// failing job inside a reusable workflow makes the whole workflow_call +// conclusion `failure`, which the publishers' `needs:` would treat as a red +// gate — continue-on-error is what keeps the sweep out of that verdict. +func TestWebUISweepJobIsAdvisory(t *testing.T) { + wf := parseWorkflow(t, gateWorkflowPath) + + sweep, ok := wf.Jobs[webUISweepJob] + if !ok { + t.Fatalf("release-qa-gate.yml has no %q job — the Playwright sweep does not run on tag builds", webUISweepJob) + } + if strings.TrimSpace(sweep.ContinueOnError.Value) != "true" { + t.Errorf("job %q must set `continue-on-error: true` (got %q) or a red sweep would block the release", + webUISweepJob, sweep.ContinueOnError.Value) + } + if sweep.disabled() { + t.Errorf("job %q is statically disabled — it would never run on a tag", webUISweepJob) + } + + // The verdict job must wait for the sweep, otherwise the fragment can land + // after the merge and the report would show a missing entry every run. + verdict, ok := wf.Jobs["verdict"] + if !ok { + t.Fatal("release-qa-gate.yml has no verdict job") + } + var needsSweep bool + for _, n := range verdict.Needs { + if n == webUISweepJob { + needsSweep = true + } + } + if !needsSweep { + t.Errorf("verdict job must list %q in needs (got %v) so the sweep fragment is merged", webUISweepJob, verdict.Needs) + } +} + +// TestWebUISweepJobReportsAndUploads pins the two observable outputs of the +// advisory job: the manifest fragment name the merger expects, and the +// Playwright HTML report artifact a maintainer reads after a red sweep. +func TestWebUISweepJobReportsAndUploads(t *testing.T) { + wf := parseWorkflow(t, gateWorkflowPath) + sweep := wf.Jobs[webUISweepJob] + + var runs, uploads string + for _, s := range sweep.Steps { + runs += s.Run + "\n" + if strings.Contains(s.Uses, "upload-artifact") { + uploads += yamlFlatten(&s.With) + "\n" + } + } + + if !strings.Contains(runs, gatereport.EntryAdvisoryWebUISweep) { + t.Errorf("the sweep job must record its outcome under the manifest entry %q; run steps: %s", + gatereport.EntryAdvisoryWebUISweep, runs) + } + if !strings.Contains(runs, "--advisory") { + t.Errorf("the sweep job must pass --advisory to release-gate run-suite so a failure is recorded as advisory-fail; run steps: %s", runs) + } + if !strings.Contains(uploads, "playwright-report") { + t.Errorf("the sweep job must upload the Playwright HTML report artifact; upload steps: %s", uploads) + } +} + +// yamlFlatten renders a `with:` mapping node as a flat string for substring +// assertions (the audit only cares whether a path/name appears at all). +func yamlFlatten(n *yaml.Node) string { + var b strings.Builder + var walk func(node *yaml.Node) + walk = func(node *yaml.Node) { + if node == nil { + return + } + b.WriteString(node.Value) + b.WriteString(" ") + for _, c := range node.Content { + walk(c) + } + } + walk(n) + return b.String() +} + +// sweepArtifactName is the advisory job's Playwright artifact. It lands in the +// SAME workflow run as the publishers (the gate is a reusable workflow the +// publishers call via `uses:`), so anything the publishers glob out of "all +// artifacts" can pick it up. +const sweepArtifactName = "web-ui-sweep-playwright-report" + +// TestPublishersDoNotShipSweepArtifacts guards a sharp edge introduced by +// wiring the sweep into the gate: release.yml / prerelease.yml download EVERY +// artifact of the run and copy every *.tar.gz / *.zip they find into the +// published release assets. A Playwright HTML report contains its traces as +// `playwright-report/data/.zip`, and traces are retained on failure — +// exactly the case the advisory job is designed to tolerate. Without an +// exclusion, a red (or merely flaky-then-green) sweep on a tag attaches +// unnamed trace zips to the public release. +func TestPublishersDoNotShipSweepArtifacts(t *testing.T) { + for _, name := range []string{"release.yml", "prerelease.yml"} { + t.Run(name, func(t *testing.T) { + path := filepath.Join("..", "..", ".github", "workflows", name) + wf := parseWorkflow(t, path) + + var checked int + for jobName, job := range wf.Jobs { + for _, s := range job.Steps { + // The indiscriminate archive collector: `find dist -name "*.zip" ...` + if !strings.Contains(s.Run, `-name "*.zip"`) { + continue + } + checked++ + if !strings.Contains(s.Run, sweepArtifactName) { + t.Errorf("%s job %q collects every *.zip into the release assets without excluding %q; "+ + "a failed advisory sweep would publish Playwright trace zips as release files:\n%s", + name, jobName, sweepArtifactName, s.Run) + } + } + } + if checked == 0 { + t.Fatalf("%s: found no archive-collection step to audit (did the publisher change shape?)", name) + } + }) + } +} diff --git a/cmd/release-gate/workflow_audit_test.go b/cmd/release-gate/workflow_audit_test.go index ed416983c..2fef486bf 100644 --- a/cmd/release-gate/workflow_audit_test.go +++ b/cmd/release-gate/workflow_audit_test.go @@ -66,15 +66,17 @@ type workflow struct { } type job struct { - Needs needsList `yaml:"needs"` - Uses string `yaml:"uses"` - If yaml.Node `yaml:"if"` - Steps []step `yaml:"steps"` + Needs needsList `yaml:"needs"` + Uses string `yaml:"uses"` + If yaml.Node `yaml:"if"` + ContinueOnError yaml.Node `yaml:"continue-on-error"` + Steps []step `yaml:"steps"` } type step struct { - Uses string `yaml:"uses"` - Run string `yaml:"run"` + Uses string `yaml:"uses"` + Run string `yaml:"run"` + With yaml.Node `yaml:"with"` } // needsList decodes `needs:` which may be a scalar (`needs: build`) or a diff --git a/docs/development/release-gate.md b/docs/development/release-gate.md index 31164c266..7773fbcb7 100644 --- a/docs/development/release-gate.md +++ b/docs/development/release-gate.md @@ -33,6 +33,7 @@ fragments against a hardcoded manifest and exits per the verdict. | `suite-race` | `suite/unit-race`, `suite/server-race` | 25 min | `go test -race ./internal/...` + `go test -tags server -race ./internal/serveredition/...`. | | `suite-scan-eval` | `suite/scan-eval` | 10 min | `go run ./cmd/scan-eval --gate --min-recall 0.90 --max-fp 0.05` over the detect corpus — runs on **every** tag regardless of changed paths (FR-015). | | `matrix-invariants` | `matrix/{stdio,http,sse,docker,oauth}`, `invariant/{activity-request-id,counters,quarantine-flow,upgrade-in-place}` | 20 min | Boots the candidate against five local fixture upstreams (connect → list → call → kill/reconnect) and asserts the US2 invariants against the live instance. | +| `web-ui-sweep` | — (advisory `advisory/web-ui-sweep`, T2) | 20 min | Playwright sweep over the Web UI **served by the candidate binary**, with a live stdio fixture upstream. `continue-on-error: true` — see [Web UI sweep](#web-ui-sweep-t2--advisory). | | `verdict` | (merges all) | 10 min | `release-gate report` → `gate-report.json` + `$GITHUB_STEP_SUMMARY`; its exit code **is** the gate verdict. | Every job uploads its fragment with `if: always()`, so a job that dies before @@ -101,17 +102,55 @@ scripts/gate/build-fixture-image.sh ./release-gate report --report-dir ./gate-report ``` -## Extension slots (T2 / T3 / T4) - -The manifest reserves three non-blocking slots, recorded as +## Web UI sweep (T2) — advisory + +The `web-ui-sweep` job runs the Playwright sweep +([`e2e/web-ui-sweep`](https://github.com/smart-mcp-proxy/mcpproxy-go/tree/main/e2e/web-ui-sweep), +see [Web UI verification](web-ui-verification.md)) against the Web UI **served by +the candidate binary** — embedded frontend, never a dev server — with a live +`mcpfixture` stdio upstream so the servers/tools screens have real data. It +covers the servers list, server detail (+ security tab), tools page and its +search, activity log, and settings, and fails on uncaught page exceptions. + +Setup is not duplicated in YAML: the job calls +[`scripts/run-web-smoke.sh`](https://github.com/smart-mcp-proxy/mcpproxy-go/blob/main/scripts/run-web-smoke.sh), +the same launcher used by hand (`./scripts/run-web-smoke.sh --show-report`), +passing `MCPPROXY_BINARY_PATH` / `MCPPROXY_FIXTURE_PATH` from the downloaded +`gate-candidate` artifact. + +It is **advisory**, on purpose while the sweep earns its flake record: + +- the job sets `continue-on-error: true`, so its failure never becomes the + reusable workflow's conclusion — publishers whose `needs:` list the gate still + publish; +- the driver runs with `--advisory`, so a red sweep is recorded as + `advisory-fail` under the non-blocking manifest entry `advisory/web-ui-sweep` + and listed in `advisory_failures` — reported, never silent; +- the Playwright HTML report + server log are uploaded as the + `web-ui-sweep-playwright-report` artifact (14 days) for post-mortem. + +> **Do not rename that artifact casually.** The gate is a *reusable* workflow, so +> its artifacts land in the publisher's own run, and `release.yml` / +> `prerelease.yml` build their release assets by globbing every `*.tar.gz` / +> `*.zip` out of *all* run artifacts. A Playwright report stores its traces as +> `playwright-report/data/.zip` (retained on failure — exactly what an +> advisory sweep tolerates), so both publishers exclude +> `*/web-ui-sweep-playwright-report/*` from that glob by name. Renaming the +> artifact without updating both publishers would publish unnamed trace zips as +> public release assets; +> `TestPublishersDoNotShipSweepArtifacts` guards this. + +**Promotion to blocking** is a two-line change: set `Blocking: true` on the +`advisory/web-ui-sweep` manifest entry and drop `continue-on-error` from the job +(the FR-016 end state). Do it once the sweep has passed on three consecutive +release tags with no flaky or infrastructure failure — the same criterion T3 +uses (FR-021). + +## Extension slots (T3 / T4) + +The manifest reserves two non-blocking slots, recorded as `not-run` / `not-implemented-yet` until their stage lands: -- **`reserved/web-ui-sweep` (T2)** — run the existing Playwright Web UI sweep - (`docs/development/web-ui-verification.md`) against the candidate binary's - **embedded** frontend, with the matrix fixtures as its upstream data - (US3, FR-016/017). Add a `web-ui-sweep` job that downloads the `gate-candidate` - artifact, serves it, runs the sweep, and writes a `reserved/web-ui-sweep` - fragment; then flip that manifest entry to blocking. - **`reserved/macos-smoke` (T3)** — a macOS-runner job that launches the tray against a running core and uses the `mcpproxy-ui-test` accessibility primitives to assert presence, menu items, and state agreement (US4, diff --git a/docs/development/web-ui-verification.md b/docs/development/web-ui-verification.md index 49f06799a..1925e5e9b 100644 --- a/docs/development/web-ui-verification.md +++ b/docs/development/web-ui-verification.md @@ -8,6 +8,20 @@ description: "Playwright sweep and HTML report workflow for verifying changes to When you modify the Web UI (any Vue file under `frontend/src/`), verify it end-to-end with a Playwright sweep that captures screenshots and packages them into a self-contained HTML report. This is the same workflow used to verify Spec 046 v2 — see `specs/046-local-first-onboarding/verification/` for a worked example. +## The standing sweep (one command) + +The core-screen sweep is committed and scripted — run it before you hand-roll anything: + +```bash +./scripts/run-web-smoke.sh --show-report # boots a throwaway instance, runs e2e/web-ui-sweep +``` + +The launcher builds `./mcpproxy` if needed, serves a throwaway instance on `127.0.0.1:18080` under a freshly generated throwaway API key (your own `MCPPROXY_API_KEY` is deliberately ignored — the key ends up in report URLs and traces), installs Chromium from the committed `e2e/web-ui-sweep/package-lock.json` via `npm ci`, and runs [`e2e/web-ui-sweep/web-ui-sweep.spec.ts`](https://github.com/smart-mcp-proxy/mcpproxy-go/blob/main/e2e/web-ui-sweep/web-ui-sweep.spec.ts) — servers list, server detail (+ security tab), tools page and search, activity log, settings — failing on uncaught page exceptions. Pass `MCPPROXY_FIXTURE_PATH=$(go build -o /tmp/mcpfixture ./cmd/mcpfixture && echo /tmp/mcpfixture)` to register a live stdio upstream so the server- and tool-dependent checks run instead of skipping. The HTML report lands in `tmp/web-smoke-artifacts/playwright-report/`. + +The release QA gate runs this exact script on every tag as its **advisory** `web-ui-sweep` job — see [Release Gate](release-gate.md#web-ui-sweep-t2--advisory). Extend the committed sweep when you add a screen worth guarding on releases; use the ad-hoc pattern below for the deeper, spec-specific verification that ships beside a spec. + +## Ad-hoc, spec-specific verification + The pattern, in order: 1. **Stand up a fresh mcpproxy.** Use a throwaway data-dir so persisted state doesn't bleed between runs: diff --git a/e2e/web-ui-sweep/package-lock.json b/e2e/web-ui-sweep/package-lock.json new file mode 100644 index 000000000..c2a1299fe --- /dev/null +++ b/e2e/web-ui-sweep/package-lock.json @@ -0,0 +1,78 @@ +{ + "name": "mcpproxy-web-ui-sweep", + "version": "1.0.0", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "mcpproxy-web-ui-sweep", + "version": "1.0.0", + "devDependencies": { + "@playwright/test": "^1.49.0" + } + }, + "node_modules/@playwright/test": { + "version": "1.62.1", + "resolved": "https://registry.npmjs.org/@playwright/test/-/test-1.62.1.tgz", + "integrity": "sha512-DTcUc8qii+cpHvtOwggMtBRMjKZHXYWdw8syRYu2vtzuq4Wxphqq4NfCs5Zt44L6mA8rfDfj+PHnxFc/FeK6mQ==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "playwright": "1.62.1" + }, + "bin": { + "playwright": "cli.js" + }, + "engines": { + "node": ">=20" + } + }, + "node_modules/fsevents": { + "version": "2.3.2", + "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.2.tgz", + "integrity": "sha512-xiqMQR4xAeHTuB9uWm+fFRcIOgKBMiOBP+eXiyT7jsgVCq1bkVygt00oASowB7EdtpOHaaPgKt812P9ab+DDKA==", + "dev": true, + "hasInstallScript": true, + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": "^8.16.0 || ^10.6.0 || >=11.0.0" + } + }, + "node_modules/playwright": { + "version": "1.62.1", + "resolved": "https://registry.npmjs.org/playwright/-/playwright-1.62.1.tgz", + "integrity": "sha512-0M+L3LAD8/nm554LOla9Ayx0j0tmFZ0FBcoQ7F1VuVHpM/XpiC8RcDzBQB8W5+hA8L22THxELzeF+2WcUzvcLg==", + "dev": true, + "license": "Apache-2.0", + "dependencies": { + "playwright-core": "1.62.1" + }, + "bin": { + "playwright": "cli.js" + }, + "engines": { + "node": ">=20" + }, + "optionalDependencies": { + "fsevents": "2.3.2" + } + }, + "node_modules/playwright-core": { + "version": "1.62.1", + "resolved": "https://registry.npmjs.org/playwright-core/-/playwright-core-1.62.1.tgz", + "integrity": "sha512-wPYSwEBJY9GHraISXqyqtx0na0LpO3XEX7jNDhntbex7tzUS7kLnZsOlFruFJB4Hi/rhDMjXGqHewDZ68nYZVw==", + "dev": true, + "license": "Apache-2.0", + "bin": { + "playwright-core": "cli.js" + }, + "engines": { + "node": ">=20" + } + } + } +} diff --git a/e2e/web-ui-sweep/package.json b/e2e/web-ui-sweep/package.json new file mode 100644 index 000000000..0fe8bee27 --- /dev/null +++ b/e2e/web-ui-sweep/package.json @@ -0,0 +1,13 @@ +{ + "name": "mcpproxy-web-ui-sweep", + "version": "1.0.0", + "private": true, + "description": "Playwright Web UI sweep driven by scripts/run-web-smoke.sh (manual) and the release QA gate (advisory job)", + "scripts": { + "test": "playwright test", + "install-browsers": "playwright install chromium" + }, + "devDependencies": { + "@playwright/test": "^1.49.0" + } +} diff --git a/e2e/web-ui-sweep/playwright.config.ts b/e2e/web-ui-sweep/playwright.config.ts new file mode 100644 index 000000000..b04d19f7d --- /dev/null +++ b/e2e/web-ui-sweep/playwright.config.ts @@ -0,0 +1,39 @@ +import { defineConfig, devices } from '@playwright/test' + +// Web UI sweep configuration (docs/development/web-ui-verification.md). +// +// The sweep drives the Web UI SERVED BY A REAL mcpproxy binary (embedded +// frontend, never a dev server), so everything here is parameterised by the +// environment the launcher (scripts/run-web-smoke.sh) exports: +// MCPPROXY_BASE_URL — the running instance, e.g. http://127.0.0.1:18080 +// MCPPROXY_API_KEY — API key of that instance (Web UI accepts ?apikey=) +// SWEEP_REPORT_DIR — where the self-contained HTML report is written +// PW_CHROMIUM — optional explicit Chromium binary (local macOS runs) +const reportDir = process.env.SWEEP_REPORT_DIR || './playwright-report' + +export default defineConfig({ + testDir: '.', + timeout: 45_000, + expect: { timeout: 15_000 }, + fullyParallel: false, + workers: 1, + forbidOnly: !!process.env.CI, + // One retry: the sweep is advisory in CI, and a single flake should not cost + // a maintainer an investigation. Genuine breakage fails both attempts. + retries: process.env.CI ? 1 : 0, + reporter: [ + ['list'], + ['html', { outputFolder: reportDir, open: 'never' }], + ], + use: { + ...devices['Desktop Chrome'], + headless: true, + viewport: { width: 1440, height: 900 }, + screenshot: 'only-on-failure', + video: 'off', + trace: 'retain-on-failure', + ...(process.env.PW_CHROMIUM + ? { launchOptions: { executablePath: process.env.PW_CHROMIUM } } + : {}), + }, +}) diff --git a/e2e/web-ui-sweep/web-ui-sweep.spec.ts b/e2e/web-ui-sweep/web-ui-sweep.spec.ts new file mode 100644 index 000000000..40ec2db91 --- /dev/null +++ b/e2e/web-ui-sweep/web-ui-sweep.spec.ts @@ -0,0 +1,116 @@ +// Web UI sweep — the core-screen pass a maintainer used to run by hand before +// a release (docs/development/web-ui-verification.md). +// +// It drives the Web UI served by a REAL mcpproxy binary with its embedded +// frontend (never a dev server), against a live stdio fixture upstream, and +// covers: servers list, server detail (+ its security tab), tools page and +// its search, activity log, and settings. Uncaught page exceptions fail the +// sweep too — a screen that renders while throwing is not "working". +// +// Launcher: scripts/run-web-smoke.sh (boots the instance, installs Chromium, +// runs this file). The release QA gate calls that same script from its +// advisory `web-ui-sweep` job. +import { test, expect, Page } from '@playwright/test' + +const BASE = process.env.MCPPROXY_BASE_URL || 'http://127.0.0.1:18080' +const KEY = process.env.MCPPROXY_API_KEY || '' +// Name of the fixture upstream the launcher registered, when it registered +// one. Unset ⇒ the sweep sticks to instance-independent structural checks. +const SERVER = process.env.SWEEP_SERVER_NAME || '' + +function url(route: string): string { + const sep = route.includes('?') ? '&' : '?' + return KEY ? `${BASE}/ui${route}${sep}apikey=${encodeURIComponent(KEY)}` : `${BASE}/ui${route}` +} + +/** Uncaught exceptions on the page fail the check that triggered them. */ +function watchPageErrors(page: Page): string[] { + const errors: string[] = [] + page.on('pageerror', (err) => errors.push(String(err))) + return errors +} + +/** + * Navigate to a Web UI route. `networkidle` never settles here — the UI holds + * an SSE channel open — so wait for DOM content plus the expected anchor. + */ +async function goto(page: Page, route: string, anchor: string) { + await page.goto(url(route)) + await page.waitForLoadState('domcontentloaded') + // A fresh instance may open the onboarding wizard over the page. + const closeWizard = page.locator('[data-test="close-wizard"]') + if (await closeWizard.isVisible().catch(() => false)) { + await closeWizard.click() + } + await page.locator(anchor).first().waitFor({ state: 'visible' }) +} + +test('servers list renders the fleet with KPI counters', async ({ page }) => { + const errors = watchPageErrors(page) + await goto(page, '/servers', '[data-test="kpi-card-total"]') + + await expect(page.locator('[data-test="kpi-card-total"]')).toContainText(/\d/) + if (SERVER) { + await expect(page.locator('[data-test="server-card-title"]', { hasText: SERVER }).first()) + .toBeVisible() + } + expect(errors, `uncaught page errors on /servers: ${errors.join(' | ')}`).toHaveLength(0) +}) + +test('server detail opens and exposes the security tab', async ({ page }) => { + test.skip(!SERVER, 'no fixture upstream registered for this sweep') + const errors = watchPageErrors(page) + await goto(page, `/servers/${encodeURIComponent(SERVER)}`, '[data-test="security-tab"]') + + await page.locator('[data-test="security-tab"]').click() + await expect(page.locator('[data-test="server-status-badge"]').first()).toBeVisible() + expect(errors, `uncaught page errors on /servers/${SERVER}: ${errors.join(' | ')}`).toHaveLength(0) +}) + +test('tools page lists upstream tools and search narrows them', async ({ page }) => { + test.skip(!SERVER, 'no fixture upstream registered for this sweep') + const errors = watchPageErrors(page) + await goto(page, '/tools', '[data-test="tools-page"]') + + const rows = page.locator('[data-test="tool-row"]') + // Tool indexing runs in the background after the upstream connects. + await expect.poll(() => rows.count(), { timeout: 30_000 }).toBeGreaterThan(0) + + await page.locator('[data-test="tools-search"]').fill('echo') + // Assert on the SURVIVORS, not on the row count. "count <= before" passed + // vacuously when the search did nothing at all (the unfiltered set is already + // <= itself, and the first row happened to match), and a strict "<" would race + // the background indexer — `before` can be sampled mid-render. Requiring every + // remaining row to match is race-free and catches a dead search box: the + // fixture's non-matching tool (`ping`) would still be listed. + await expect + .poll( + async () => { + const texts = await rows.allTextContents() + return texts.length > 0 && texts.every((t) => /echo/i.test(t)) + }, + { timeout: 10_000 }, + ) + .toBe(true) + expect(errors, `uncaught page errors on /tools: ${errors.join(' | ')}`).toHaveLength(0) +}) + +test('activity log renders and its filter panel expands', async ({ page }) => { + const errors = watchPageErrors(page) + // The compact header is the always-present anchor; the KPI stat cards only + // render once the filter panel is expanded AND a summary has loaded. + await goto(page, '/activity', '[data-test="activity-filters-toggle"]') + + await page.locator('[data-test="activity-filters-toggle"]').click() + await expect(page.locator('[data-test="activity-filter-panel"]')).toBeVisible() + expect(errors, `uncaught page errors on /activity: ${errors.join(' | ')}`).toHaveLength(0) +}) + +test('settings page renders its tabs and security posture', async ({ page }) => { + const errors = watchPageErrors(page) + await goto(page, '/settings', '[data-test="settings-tabs"]') + + await expect(page.locator('[data-test="settings-posture"]')).toBeVisible() + await expect(page.locator('[data-test="setting-secret-api_key"]')).toBeVisible() + expect(errors, `uncaught page errors on /settings: ${errors.join(' | ')}`).toHaveLength(0) +}) diff --git a/internal/gatereport/gatereport.go b/internal/gatereport/gatereport.go index e2fc0aa2b..47888e941 100644 --- a/internal/gatereport/gatereport.go +++ b/internal/gatereport/gatereport.go @@ -6,9 +6,10 @@ // Fragment file into a shared report directory. The merger compares the // fragments against a HARDCODED manifest of expected entries: a missing // fragment for a blocking entry is a FAIL (no silent skips, FR-004), and -// reserved extension slots (web-ui-sweep, macos-smoke, surface-consistency — -// Spec 081 T2-T4) are recorded as `not-run` with reason -// "not-implemented-yet" until their stages land. +// reserved extension slots (macos-smoke, surface-consistency — Spec 081 +// T3/T4) are recorded as `not-run` with reason "not-implemented-yet" until +// their stages land. The T2 Web UI sweep is a live ADVISORY entry: it runs on +// tag builds and its failures are reported without blocking the release. package gatereport import ( @@ -101,13 +102,18 @@ const ( EntrySuiteServerRace = "suite/server-race" EntrySuiteScanEval = "suite/scan-eval" - EntryReservedWebUISweep = "reserved/web-ui-sweep" + // EntryAdvisoryWebUISweep is the Spec 081 T2 Playwright Web UI sweep. It + // runs on every tag build but is ADVISORY: a red sweep is reported (and + // listed under advisory_failures) without blocking publication. + EntryAdvisoryWebUISweep = "advisory/web-ui-sweep" + EntryReservedMacOSSmoke = "reserved/macos-smoke" EntryReservedSurfaceConsistency = "reserved/surface-consistency" ) // Manifest returns the hardcoded expected-entries manifest: 5 matrix cells + -// 4 invariants + 4 assembled suite jobs (FR-003) + 3 reserved slots. +// 4 invariants + 4 assembled suite jobs (FR-003) + 1 advisory entry (the T2 +// Web UI sweep) + 2 reserved slots (T3/T4). func Manifest() []ManifestEntry { return []ManifestEntry{ {Name: EntryMatrixStdio, Blocking: true}, @@ -126,7 +132,8 @@ func Manifest() []ManifestEntry { {Name: EntrySuiteServerRace, Blocking: true}, {Name: EntrySuiteScanEval, Blocking: true}, - {Name: EntryReservedWebUISweep, Blocking: false, Reserved: true}, + {Name: EntryAdvisoryWebUISweep, Blocking: false}, + {Name: EntryReservedMacOSSmoke, Blocking: false, Reserved: true}, {Name: EntryReservedSurfaceConsistency, Blocking: false, Reserved: true}, } diff --git a/internal/gatereport/gatereport_test.go b/internal/gatereport/gatereport_test.go index c9d41d5e1..32994cd84 100644 --- a/internal/gatereport/gatereport_test.go +++ b/internal/gatereport/gatereport_test.go @@ -36,7 +36,7 @@ func TestMerge_AllBlockingPass_VerdictPass_ReservedNotRun(t *testing.T) { t.Fatalf("expected pass verdict, got %s (failures: %v)", r.Verdict, r.BlockingFailures) } // Reserved slots must be recorded, never silently absent (FR-004). - for _, name := range []string{EntryReservedWebUISweep, EntryReservedMacOSSmoke, EntryReservedSurfaceConsistency} { + for _, name := range []string{EntryReservedMacOSSmoke, EntryReservedSurfaceConsistency} { e := entryByName(t, r, name) if e.Status != StatusNotRun || e.Reason != ReasonNotImplemented { t.Errorf("%s: got status=%s reason=%q, want not-run/%s", name, e.Status, e.Reason, ReasonNotImplemented) @@ -119,6 +119,68 @@ func TestMerge_AdvisoryFailOnNonBlockingReservedSlot_DoesNotBlock(t *testing.T) } } +// Spec 081 T2: the Web UI sweep is wired as an ADVISORY entry — it is a real +// manifest entry (no longer a reserved "not-implemented-yet" slot), it never +// blocks the tag, and every non-green outcome (including a missing fragment, +// i.e. the job died before uploading) is still reported as an advisory failure. +func TestManifest_WebUISweep_IsAdvisoryNotReserved(t *testing.T) { + var found bool + for _, m := range Manifest() { + if m.Name != EntryAdvisoryWebUISweep { + continue + } + found = true + if m.Blocking { + t.Errorf("%s must not block the gate (advisory, T2)", m.Name) + } + if m.Reserved { + t.Errorf("%s must no longer be a reserved slot — the sweep job exists", m.Name) + } + } + if !found { + t.Fatalf("manifest is missing the %s entry", EntryAdvisoryWebUISweep) + } +} + +func TestMerge_WebUISweepFailure_IsAdvisoryOnly(t *testing.T) { + for _, status := range []Status{StatusFail, StatusAdvisoryFail} { + frags := passAllBlocking() + for i := range frags { + if frags[i].Name == EntryAdvisoryWebUISweep { + frags[i].Status = status + frags[i].Reason = "sweep spec failed" + } + } + r := Merge(frags) + if !r.Passed() { + t.Fatalf("web-ui sweep %s must not block the gate: %v", status, r.BlockingFailures) + } + if len(r.AdvisoryFailures) != 1 || !strings.Contains(r.AdvisoryFailures[0], EntryAdvisoryWebUISweep) { + t.Errorf("web-ui sweep %s must be reported as an advisory failure, got %v", status, r.AdvisoryFailures) + } + } +} + +func TestMerge_WebUISweepMissingFragment_IsAdvisoryFailureNotBlocking(t *testing.T) { + var kept []Fragment + for _, f := range passAllBlocking() { + if f.Name != EntryAdvisoryWebUISweep { + kept = append(kept, f) + } + } + r := Merge(kept) + if !r.Passed() { + t.Fatalf("a missing web-ui sweep fragment must not block the gate: %v", r.BlockingFailures) + } + e := entryByName(t, r, EntryAdvisoryWebUISweep) + if e.Status != StatusFail || e.Reason != ReasonMissingFragment { + t.Errorf("got status=%s reason=%q, want fail/%q", e.Status, e.Reason, ReasonMissingFragment) + } + if len(r.AdvisoryFailures) != 1 || !strings.Contains(r.AdvisoryFailures[0], EntryAdvisoryWebUISweep) { + t.Errorf("missing sweep fragment must surface as an advisory failure, got %v", r.AdvisoryFailures) + } +} + func TestMerge_UnexpectedFailingFragment_Blocks(t *testing.T) { frags := append(passAllBlocking(), Fragment{Name: "rogue/check", Status: StatusFail, Reason: "boom"}) r := Merge(frags) diff --git a/roadmap.yaml b/roadmap.yaml index b9b15f01a..d6115edc5 100644 --- a/roadmap.yaml +++ b/roadmap.yaml @@ -449,8 +449,9 @@ epics: depends_on: [] - id: release-qa-gate-playwright title: "T2: wire the Playwright Web UI sweep into the gate (currently manual-trigger only)" - status: todo + status: in_review priority: P2 + note: "Landed ADVISORY, not blocking (FR-016 wants blocking eventually). The sweep itself is now committed (e2e/web-ui-sweep: servers list, server detail + security tab, tools page + search, activity log, settings; uncaught page exceptions fail it) and scripts/run-web-smoke.sh — previously dead, it pointed at a spec file that lives in a gitignored dir — is its single launcher for both `./scripts/run-web-smoke.sh` by hand and the gate's `web-ui-sweep` job. Advisory is enforced twice: continue-on-error on the job (a failing job inside a reusable workflow would otherwise become the workflow_call conclusion the publishers gate on) and a non-blocking manifest entry advisory/web-ui-sweep (renamed from reserved/web-ui-sweep) whose failures land in advisory_failures via the new `release-gate run-suite --advisory` flag. Promotion to blocking = Blocking:true + drop continue-on-error, after 3 consecutive clean tags." depends_on: [release-qa-gate-matrix] - id: release-qa-gate-macos title: "T3: macOS app smoke on a macos runner, advisory until 3 consecutive passes (today zero CI automation for the tray app)" diff --git a/scripts/run-web-smoke.sh b/scripts/run-web-smoke.sh index 82337317d..9fa765973 100755 --- a/scripts/run-web-smoke.sh +++ b/scripts/run-web-smoke.sh @@ -1,4 +1,14 @@ #!/usr/bin/env bash +# +# Boots a throwaway mcpproxy instance and runs the Playwright Web UI sweep +# (e2e/web-ui-sweep) against the Web UI IT serves — embedded frontend, never a +# dev server. See docs/development/web-ui-verification.md. +# +# Same script both ways it is used: +# * by hand: ./scripts/run-web-smoke.sh --show-report +# * by the release gate: .github/workflows/release-qa-gate.yml `web-ui-sweep` +# job (advisory), with MCPPROXY_BINARY_PATH / MCPPROXY_FIXTURE_PATH pointing +# at the candidate binaries it downloaded. set -euo pipefail @@ -10,11 +20,19 @@ usage() { cat </tmp/web-smoke-artifacts) EOF } @@ -45,58 +63,118 @@ required() { fi } -required go required curl required node -required npx +required npm SCRIPT_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) REPO_ROOT=$(cd "$SCRIPT_DIR/.." && pwd) BINARY_PATH="${MCPPROXY_BINARY_PATH:-$REPO_ROOT/mcpproxy}" +FIXTURE_PATH="${MCPPROXY_FIXTURE_PATH:-}" BASE_URL="${MCPPROXY_BASE_URL:-http://127.0.0.1:18080}" -PLAYWRIGHT_WORKDIR="$REPO_ROOT/.playwright-mcp" -RESULTS_DIR="$PLAYWRIGHT_WORKDIR/test-results" +LISTEN="${BASE_URL#http://}" +SWEEP_DIR="$REPO_ROOT/e2e/web-ui-sweep" ARTIFACT_DIR="${ARTIFACT_DIR:-$REPO_ROOT/tmp/web-smoke-artifacts}" +REPORT_DIR="$ARTIFACT_DIR/playwright-report" +# Always a fresh throwaway key, NEVER $MCPPROXY_API_KEY: the sweep puts the key +# in every navigation URL, and Playwright stores those URLs in the HTML report +# and failure traces. Honouring an ambient MCPPROXY_API_KEY would copy a +# developer's real key into tmp/web-smoke-artifacts (and, in CI, an uploaded +# artifact). The instance is throwaway, so its key may as well be too. +# +# Throwaway, but NOT guessable. The key it protects is the instance's full REST +# admin API, which can register a stdio upstream — i.e. run an arbitrary command +# as whoever launched the sweep. 127.0.0.1 is reachable by every local account, +# so a key derived from the clock and the PID (a search space of seconds x pids) +# is brute-forceable for the minute the sweep is up. 24 random bytes are not. +API_KEY="web-sweep-$(node -e 'process.stdout.write(require("crypto").randomBytes(24).toString("hex"))')" +if [[ ${#API_KEY} -lt 32 ]]; then + echo "failed to generate a random API key for the throwaway instance" >&2 + exit 1 +fi +# The sweep's server-dependent checks run only when a fixture upstream exists. +SWEEP_SERVER_NAME="" -mkdir -p "$ARTIFACT_DIR" -mkdir -p "$PLAYWRIGHT_WORKDIR" +mkdir -p "$ARTIFACT_DIR" "$REPORT_DIR" -pushd "$REPO_ROOT" >/dev/null if [[ ! -x "$BINARY_PATH" ]]; then + required go echo "building mcpproxy binary..." - go build -o "$BINARY_PATH" ./cmd/mcpproxy + (cd "$REPO_ROOT" && go build -o "$BINARY_PATH" ./cmd/mcpproxy) fi -popd >/dev/null -TMPDIR=$(mktemp -d) -CONFIG_PATH="$TMPDIR/config.json" -DATA_DIR="$TMPDIR/data" -LOG_PATH="$TMPDIR/mcpproxy.log" +TMPDIR_SWEEP=$(mktemp -d) +CONFIG_PATH="$TMPDIR_SWEEP/config.json" +DATA_DIR="$TMPDIR_SWEEP/data" +LOG_PATH="$TMPDIR_SWEEP/mcpproxy.log" cleanup() { - if [[ -n "${SERVER_PID:-}" ]]; then - if kill -0 "$SERVER_PID" >/dev/null 2>&1; then - kill "$SERVER_PID" >/dev/null 2>&1 || true - wait "$SERVER_PID" >/dev/null 2>&1 || true - fi + if [[ -n "${SERVER_PID:-}" ]] && kill -0 "$SERVER_PID" >/dev/null 2>&1; then + kill "$SERVER_PID" >/dev/null 2>&1 || true + wait "$SERVER_PID" >/dev/null 2>&1 || true + fi + # Preserve the server log before the temp dir goes away. Doing this in the + # trap (not only on the happy path) is what makes an EARLY failure — + # readiness timeout, npm/Chromium install blowing up under `set -e` — + # diagnosable from the uploaded artifact instead of only from the CI console. + if [[ -f "$LOG_PATH" ]]; then + cp "$LOG_PATH" "$ARTIFACT_DIR/server.log" 2>/dev/null || true fi - rm -rf "$TMPDIR" + rm -rf "$TMPDIR_SWEEP" } trap cleanup EXIT +# mcpServers: the stdio fixture when one was supplied, otherwise an empty fleet +# (the sweep then skips its server-dependent checks instead of failing). +SERVERS_JSON="[]" +if [[ -n "$FIXTURE_PATH" ]]; then + if [[ ! -x "$FIXTURE_PATH" ]]; then + echo "MCPPROXY_FIXTURE_PATH is not executable: $FIXTURE_PATH" >&2 + exit 1 + fi + # The config below is a heredoc, not JSON-encoded output: a path containing a + # quote, a backslash or a newline would silently produce a malformed (or + # subtly different) config rather than an error. Reject it loudly instead of + # taking on a jq/python dependency for a path that is normally boring. + case "$FIXTURE_PATH" in + *'"'* | *\\* | *$'\n'*) + echo "MCPPROXY_FIXTURE_PATH contains a quote, backslash or newline and cannot be embedded in the sweep config: $FIXTURE_PATH" >&2 + exit 1 + ;; + esac + SWEEP_SERVER_NAME="sweep-stdio" + SERVERS_JSON=$(cat <"$CONFIG_PATH" { - "listen": "127.0.0.1:18080", + "listen": "${LISTEN}", "data_dir": "${DATA_DIR}", - "api_key": "", + "api_key": "${API_KEY}", "enable_tray": false, + "enable_socket": false, + "enable_web_ui": true, + "check_server_repo": false, "logging": { "level": "info", "enable_file": false, "enable_console": true }, - "mcpServers": [], + "mcpServers": ${SERVERS_JSON}, "top_k": 10, "tools_limit": 20, "tool_response_limit": 20000, @@ -110,17 +188,41 @@ cat <"$CONFIG_PATH" } JSON -"$BINARY_PATH" serve --config "$CONFIG_PATH" --listen 127.0.0.1:18080 >"$LOG_PATH" 2>&1 & +# HEADLESS/DO_NOT_TRACK: a QA sweep must never open a browser for OAuth nor +# emit production telemetry (same rule the gate driver applies). +# MCPPROXY_API_KEY is pinned to the generated throwaway key, NOT inherited: the +# env var outranks the config file ("source": "environment variable"), so a +# developer with their own key exported would otherwise leave the server +# demanding that key while the sweep probes with this one — a 46s readiness +# timeout with nothing but 401s in the log. +HEADLESS=1 DO_NOT_TRACK=1 MCPPROXY_API_KEY="$API_KEY" "$BINARY_PATH" serve \ + --config "$CONFIG_PATH" --listen "$LISTEN" >"$LOG_PATH" 2>&1 & SERVER_PID=$! -echo "mcpproxy started (PID ${SERVER_PID}); waiting for readiness..." +echo "mcpproxy started (PID ${SERVER_PID}); waiting for readiness at ${BASE_URL}..." attempt=0 -until curl -sS -o /dev/null -w '%{http_code}' "$BASE_URL/api/v1/servers" | grep -q '^200$'; do +# --connect-timeout/--max-time are load-bearing, not decoration: a process that +# holds the port but never answers (a hung instance, a half-open socket) makes an +# untimed curl block forever, and the liveness check below would never get a turn +# — the job would sit until its 20-minute timeout instead of failing in seconds. +until curl -s --connect-timeout 2 --max-time 5 -o /dev/null -w '%{http_code}' \ + -H "X-API-Key: ${API_KEY}" "$BASE_URL/api/v1/servers" | grep -q '^200$'; do + # Fail fast if the candidate already exited (port in use → exit code 2, bad + # config → 4, DB locked → 3). Without this the loop would burn the full + # timeout, and — worse — a leftover instance of a PREVIOUS sweep listening on + # the same port could answer 200 and the sweep would silently exercise that + # stale binary instead of the candidate. + if ! kill -0 "$SERVER_PID" >/dev/null 2>&1; then + wait "$SERVER_PID" >/dev/null 2>&1 || true + echo "mcpproxy exited before becoming ready at ${BASE_URL} (is ${LISTEN} already in use?)" >&2 + cat "$LOG_PATH" >&2 + exit 1 + fi sleep 1 attempt=$((attempt + 1)) if [[ $attempt -gt 45 ]]; then - echo "server did not become ready after ${attempt}s" + echo "server did not become ready after ${attempt}s" >&2 cat "$LOG_PATH" >&2 exit 1 fi @@ -128,33 +230,63 @@ done echo "server ready at $BASE_URL" -rm -rf "$RESULTS_DIR" - -export MCPPROXY_BASE_URL="$BASE_URL" -export PLAYWRIGHT_HTML_PATH="$ARTIFACT_DIR/playwright-report" export PLAYWRIGHT_BROWSERS_PATH="${PLAYWRIGHT_BROWSERS_PATH:-$REPO_ROOT/tmp/playwright-browsers}" -export CI=${CI:-1} +mkdir -p "$PLAYWRIGHT_BROWSERS_PATH" -mkdir -p "$PLAYWRIGHT_HTML_PATH" +# `npm ci` against the COMMITTED e2e/web-ui-sweep/package-lock.json, never a +# bare `npm install`: this runs during release qualification, and resolving the +# mutable `^1.49.0` range there would execute whatever Playwright and its +# transitive deps published since the last review. The lockfile pins exact +# versions + integrity hashes; bump it deliberately with `npm install`. +# +# The install is skipped only when what is on disk was installed FROM THE CURRENT +# lockfile. A mere "is playwright present?" check would let a stale tree — e.g. +# one an older revision of this script installed with an unlocked `npm install` — +# keep serving an unreviewed version forever; checking only @playwright/test's +# version would still miss a lockfile that moved a transitive dependency. So the +# hash of the whole lockfile is stamped into node_modules after a successful +# `npm ci`, and any drift re-runs it. This keeps the hand-run fast path (no +# registry round-trip when nothing changed) without weakening the pinning. +# +# The path goes through process.argv, never string interpolation into the JS — +# a repo checked out under a path containing a quote would otherwise produce +# invalid JavaScript and a silently empty result. +lock_hash() { + node -e 'const c=require("crypto"),f=require("fs");process.stdout.write(c.createHash("sha256").update(f.readFileSync(process.argv[1])).digest("hex"))' "$1" 2>/dev/null || true +} -if [[ ! -d "$PLAYWRIGHT_WORKDIR/node_modules/@playwright/test" ]]; then - echo "installing @playwright/test into $PLAYWRIGHT_WORKDIR" - npm install --prefix "$PLAYWRIGHT_WORKDIR" --no-save --package-lock=false @playwright/test +LOCK_STAMP="$SWEEP_DIR/node_modules/.sweep-lock-sha256" +WANT_LOCK_HASH=$(lock_hash "$SWEEP_DIR/package-lock.json") +if [[ -z "$WANT_LOCK_HASH" ]]; then + echo "cannot hash $SWEEP_DIR/package-lock.json — is the lockfile committed?" >&2 + exit 1 fi -export PATH="$PLAYWRIGHT_WORKDIR/node_modules/.bin:$PATH" +HAVE_LOCK_HASH="" +if [[ -f "$LOCK_STAMP" ]]; then + HAVE_LOCK_HASH=$(cat "$LOCK_STAMP") +fi + +if [[ ! -x "$SWEEP_DIR/node_modules/.bin/playwright" || "$HAVE_LOCK_HASH" != "$WANT_LOCK_HASH" ]]; then + echo "installing the locked @playwright/test into $SWEEP_DIR (npm ci)" + # cd rather than `npm ci --prefix`: --prefix has a long history of version- + # dependent behaviour for `ci` specifically, and this runs during release + # qualification — an install that silently resolves against the repo root + # instead of the sweep's lockfile is not a failure mode worth risking. + (cd "$SWEEP_DIR" && npm ci) + # After npm ci, which wipes node_modules (and with it any previous stamp). + printf '%s' "$WANT_LOCK_HASH" >"$LOCK_STAMP" +else + echo "@playwright/test already installed from the current lockfile" +fi -PLAYWRIGHT_BIN="$PLAYWRIGHT_WORKDIR/node_modules/.bin/playwright" +PLAYWRIGHT_BIN="$SWEEP_DIR/node_modules/.bin/playwright" if [[ ! -x "$PLAYWRIGHT_BIN" ]]; then echo "playwright CLI not found after install" >&2 exit 1 fi -mkdir -p "$PLAYWRIGHT_BROWSERS_PATH" - -pushd "$PLAYWRIGHT_WORKDIR" >/dev/null - -echo "installing Playwright browsers (cached under $PLAYWRIGHT_BROWSERS_PATH)" +echo "installing Playwright Chromium (cached under $PLAYWRIGHT_BROWSERS_PATH)" if [[ "$(uname -s)" == "Linux" ]]; then "$PLAYWRIGHT_BIN" install --with-deps chromium else @@ -162,29 +294,29 @@ else fi set +e -"$PLAYWRIGHT_BIN" test web-smoke.spec.ts --project=chromium +( + cd "$SWEEP_DIR" || exit 1 + MCPPROXY_BASE_URL="$BASE_URL" \ + MCPPROXY_API_KEY="$API_KEY" \ + SWEEP_SERVER_NAME="$SWEEP_SERVER_NAME" \ + SWEEP_REPORT_DIR="$REPORT_DIR" \ + "$PLAYWRIGHT_BIN" test web-ui-sweep.spec.ts +) PLAYWRIGHT_STATUS=$? set -e -popd >/dev/null - cp "$LOG_PATH" "$ARTIFACT_DIR/server.log" -if [[ -d "$RESULTS_DIR" ]]; then - mkdir -p "$ARTIFACT_DIR/test-results" - cp -R "$RESULTS_DIR/." "$ARTIFACT_DIR/test-results/" >/dev/null 2>&1 || true -fi - if [[ $PLAYWRIGHT_STATUS -ne 0 ]]; then - echo "web smoke failed; artifacts stored in $ARTIFACT_DIR" >&2 + echo "web UI sweep FAILED; artifacts (HTML report + server log) in $ARTIFACT_DIR" >&2 exit $PLAYWRIGHT_STATUS fi -echo "web smoke passed; artifacts stored in $ARTIFACT_DIR" +echo "web UI sweep passed; artifacts stored in $ARTIFACT_DIR" if [[ $SHOW_REPORT -eq 1 ]]; then echo "launching Playwright HTML report (Ctrl+C to exit)..." - "$PLAYWRIGHT_BIN" show-report "$PLAYWRIGHT_HTML_PATH" + "$PLAYWRIGHT_BIN" show-report "$REPORT_DIR" fi exit 0