LearnNewsExamplesServices
Frontmatter
id10878
titleUpdate Authorization.spec.mjs env-var fixtures post-#10877 NEO_ rename
stateClosed
labels
bugaitestingmodel-experience
assigneesneo-opus-4-7
createdAtMay 7, 2026, 9:31 AM
updatedAtMay 7, 2026, 7:03 PM
githubUrlhttps://github.com/neomjs/neo/issues/10878
authorneo-opus-4-7
commentsCount0
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
blockedBy[]
blocking[]
closedAtMay 7, 2026, 11:14 AM

Update Authorization.spec.mjs env-var fixtures post-#10877 NEO_ rename

Closedbugaitestingmodel-experience
neo-opus-4-7
neo-opus-4-7 commented on May 7, 2026, 9:31 AM

Context

PR #10877 (Resolves #10862) institutionalized the canonical NEO_ prefix on 13 substrate env vars including TRANSPORT → NEO_TRANSPORT, AUTO_SYNC → NEO_AUTO_SYNC, AUTO_SUMMARIZE → NEO_AUTO_SUMMARIZE. The PR's AC4 grep verified zero residual bare-name occurrences in ai/, src/, buildScripts/ — but did NOT scope the audit to test/. As a result, one Playwright spec that spawns the MCP server with the now-renamed env vars was missed and ships its fixtures with the old (no-longer-read) names.

Surfaced post-merge by @tobiu while reviewing what the test suite was doing under the rename.

The Problem

test/playwright/unit/ai/mcp/Authorization.spec.mjs spawns node ai/mcp/server/.../mcp-server.mjs with an env block that uses pre-#10877 names:

mcpServerProcess = spawn('node', [SERVER_PATH, '--debug'], {
    env: {
        ...process.env,
        TRANSPORT      : 'sse',           // server reads NEO_TRANSPORT, falls back to default 'stdio'
        SSE_PORT       : MCP_SSE_PORT.toString(),  // legacy alias still works per #10808 deprecation window
        AUTO_SYNC      : 'false',         // server reads NEO_AUTO_SYNC, fixture value ignored
        AUTO_SUMMARIZE : 'false',         // server reads NEO_AUTO_SUMMARIZE, fixture value ignored
        NEO_AUTH_HOST      : 'localhost',     // third-party canonical (OIDC), no rename — OK
        NEO_AUTH_PORT      : MOCK_OIDC_PORT.toString(),  // OK
        NEO_AUTH_REALM     : 'test',          // OK
        NEO_OAUTH_CLIENT_ID: TEST_CLIENT_ID,  // OK
        HOST           : 'localhost'      // OK (host config, not renamed)
    }
});

Effect: the server boots with default transport: 'stdio' instead of fixture-injected 'sse', and default autoSync: false / autoSummarize: false (which happens to match what the fixture also passes — so the AUTO_* mismatches don't cause behavioral divergence here, BUT the TRANSPORT mismatch DOES — server doesn't open SSE, test setup fails or stalls).

The fixture has 6 affected occurrences across 2 spawn blocks (lines 76-79 and 188-191).

The Architectural Reality

  • test/playwright/unit/ai/mcp/Authorization.spec.mjs:73-86 — first spawn block (test.beforeAll), uses fixture env injection.
  • test/playwright/unit/ai/mcp/Authorization.spec.mjs:185-198 — second spawn block (similar shape).
  • Server reads from process.env.NEO_TRANSPORT / process.env.NEO_AUTO_SYNC / process.env.NEO_AUTO_SUMMARIZE post-#10877 — the env-binding tables in ai/mcp/server/{memory-core,knowledge-base}/config.template.mjs were updated, but no test file was touched in #10877's diff.
  • Empirical: grep -rln "TRANSPORT\|AUTO_SUMMARIZE\|AUTO_SYNC" test/ returns ONLY this single file (other test files don't spawn server processes with env injection).
  • SSE_PORT (line 77) is the legacy alias retained during the #10808 deprecation window — still resolves correctly to MCP_HTTP_PORT for now. Modernizing to MCP_HTTP_PORT is a separate concern under #10808's deprecation cleanup.

The Fix

Single-file rename in test/playwright/unit/ai/mcp/Authorization.spec.mjs:

Line(s) Before After
76, 188 TRANSPORT : 'sse', NEO_TRANSPORT : 'sse',
78, 190 AUTO_SYNC : 'false', NEO_AUTO_SYNC : 'false',
79, 191 AUTO_SUMMARIZE : 'false', NEO_AUTO_SUMMARIZE: 'false',

Block-formatting alignment may need adjustment after the rename (longer property names shift the colon column). Apply repo coding-guideline §8.32 column-alignment after the substitution.

SSE_PORT left as-is (legacy alias under #10808 deprecation window, separate concern). Other env vars in the block (NEO_AUTH_*, ONEO_AUTH_*, HOST) are third-party canonical or host-config and are correctly NOT prefixed per #10862's out-of-scope policy.

Acceptance Criteria

  • All 6 occurrences in test/playwright/unit/ai/mcp/Authorization.spec.mjs use NEO_ prefix where applicable (TRANSPORT, AUTO_SYNC, AUTO_SUMMARIZE).
  • npm run test-unit -- test/playwright/unit/ai/mcp/Authorization.spec.mjs passes (or its pre-existing pass/fail state is preserved post-rename — if the spec was already broken for unrelated reasons, this PR doesn't fix it).
  • Block-formatting alignment per coding-guideline §8.32 maintained after rename.
  • Repo-wide grep grep -rE "(?<![A-Z_])(AUTO_SUMMARIZE|AUTO_DREAM|AUTO_GOLDEN_PATH|REAL_TIME_MEMORY_PARSING|AUTO_INGEST_FS|AUTO_SYNC|MEMORY_COLLECTION_NAME|SESSION_COLLECTION_NAME|GRAPH_COLLECTION_NAME|GRAPH_DECAY_FACTOR)\\b" . returns zero results post-PR (full repo audit, not just ai/src/buildScripts from #10862's AC4).

Out of Scope

  • SSE_PORT modernization to MCP_HTTP_PORT — that's #10808's deprecation-cleanup lane.
  • TRANSPORT value semantics — fixture passes 'sse'; server post-#10877 reads NEO_TRANSPORT='sse'. No behavior change beyond the env-var-name surface.
  • Adding test-execution to npm run test-unit -- test/playwright/unit/ai/mcp/ discovery patterns — already covered.
  • Repo-wide test audit for OTHER env-var-using fixtures — confirmed empirically only this one file affected via grep -rln.

Avoided Traps

  • Bundling SSE_PORT modernization — that's a separate substrate concern under #10808's deprecation chain. Keep this fix scoped tight.
  • Re-deriving the env-binding table at test-fixture level — the test should pass env vars by their canonical names, not re-implement the binding logic.
  • Implicit-pass via default-value coincidence — even though AUTO_SUMMARIZE='false' and the default is also false, the rename should still happen so a future test changing the value to 'true' actually takes effect.

Related

  • #10862 — parent NEO_ prefix institutionalization
  • PR #10877 — the merged rename PR (this is its follow-up)
  • #10808MCP_HTTP_PORT / SSE_PORT deprecation window (separate concern)
  • #10822 — config substrate cleanup epic (parent of #10862)

Lessons Learned (Reviewer-Side)

This issue surfaced because my Cycle 1 / Cycle 2 reviews of #10877 cited "substrate-contention concern in this active worktree" as rationale for skipping the test-execution step. Per pr-review-guide.md §2.2 Empirical Checkout Mandate: "A static diff read is insufficient to score [EXECUTION_QUALITY] for code changes. You MUST use the active harness tool (e.g., checkout_pull_request) to load the branch locally and execute RELATED tests." Renaming env-var consumers in production code IS structural code change; tests that spawn server processes ARE related tests. Skipping was wrong-shape regardless of worktree contention. Memory anchor feedback_run_related_tests_in_pr_review.md filed alongside this ticket.

Origin Session ID: 78a3272e-847b-4799-ad6c-ce334464844c

Retrieval Hint: query_raw_memories(query="Authorization.spec.mjs NEO_ env-var fixture rename followup")

tobiu referenced in commit 95f451d - "test(mcp/Authorization): align fixture env-vars with #10877 NEO_ rename (#10878) (#10879) on May 7, 2026, 11:14 AM
tobiu closed this issue on May 7, 2026, 11:14 AM