diff --git a/AGENTS.md b/AGENTS.md index f64e61a6e8..7484c06a85 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -210,6 +210,23 @@ FNXC:TestFlakeRegister 2026-08-01-07:00: Issue #2862 recorded three suite-only P - Motivating incident for UI affordances: the workflow-row drop-down arrow removal took three tasks (FN-6115 → FN-6118 → FN-6123) because the affordance rendered in two components and mobile kept an empty 36×36 `btn-icon` button shell. - If a regression test only proves the exact reported case, it is incomplete; extend it until the invariant holds across all known surfaces. +### Standing Rule: Tests Assert Behavior, Never Source Text Or Comments + +**A test may never assert that a comment exists.** Not an `FNXC:` block, not a date stamp, not prose lifted from a source or CSS file. Comments are documentation; asserting them tests nothing a user, caller, or operator can observe. + +This is not a style preference — it is a rule this repo's own conventions make self-defeating. The FNXC convention above tells authors to *keep these comments updated as requirements change*. A test pinning one guarantees a future false failure, and the cheapest way to make that failure go away is the worst possible action: **re-adding a comment to shipped source purely to satisfy a test.** + +**Measured 2026-08-23.** `grok-runtime-bootstrap.test.ts` asserted `runTaskMerge`'s body contained the literal `FNXC:GrokCliRouting 2026-07-15-10:17`. FN-9167 legitimately rewrote that function and dropped the block while leaving the behavior intact. The test went red, and the fix applied was to restore the comment in `packages/cli/src/commands/task.ts` — a comment returned to the product not because it documented anything true, but to appease a test. Four more such assertions sat in the dashboard's CSS tests (`NewTaskModal`, `FloatingWindow`, `TaskDetailModal.responsive-and-dependencies`, `CommandCenter.tablet-layout`), each beside a real assertion and each adding nothing. + +What to do instead, in order of preference: +1. **Assert the behavior** the comment describes: the rendered style, the computed value, the observable outcome, the state after the action. +2. **Assert the code construct** if the invariant is genuinely structural — `expect(body).not.toContain("mergePluginRunner")` is a real guard; `expect(body).toContain("FNXC:…")` is not. +3. **Delete the assertion.** If nothing behavioral or structural is behind it, it was never guarding anything. + +**Boundary — this rule does not touch code-construct guards.** Source scans that enforce call-site allowlists and architectural ratchets (`engine-no-blocking-shellout`, `user-configured-command-no-execsync`, `vi-mock-specifiers-resolve`, the durable-write and emit-surface inventories, `legacy-tombstones`, `lazy-loaded-views-docs`) assert code structure, are mandated elsewhere in this document, and stay. The line is: **prose, comments, and date stamps are never a test subject; code constructs may be.** + +Enforced by `scripts/check-no-comment-assertions-in-tests.mjs`, which runs in `pretest`, `pretest:full`, and `test:gate:static`. It flags the unambiguous case (an `FNXC:` stamp inside an assertion matcher); an earlier draft that also matched `/*` produced 24 false positives and zero true ones, because a regex cannot tell comment prose from a path glob. The rest of this rule is enforced by review. + ### Standing Rule: A Behavior Change Owns Every Test That Asserts The Old Behavior **Changing behavior is not done until the tests that encoded the old behavior are updated or deleted — in the same change.** This is not the same rule as "keep the tests green": targeted verification runs the tests for the files you touched, and the stale assertions are almost always in files you did not touch, so a green targeted run is not evidence that no test still encodes the behavior you just changed. diff --git a/package.json b/package.json index 3ecb7cf325..7e9ce0b604 100644 --- a/package.json +++ b/package.json @@ -14,8 +14,8 @@ "type": "module", "packageManager": "pnpm@10.33.0", "scripts": { - "pretest": "node scripts/check-no-nohup.mjs && node scripts/check-no-cwd-relative-dashboard-test-reads.mjs && node scripts/check-no-kill-4040.mjs && node scripts/check-no-getdatabase.mjs && node scripts/check-prerebase-inert.mjs && node scripts/check-capacity-pool-id.mjs && node scripts/check-cli-runtime-routing.mjs && node scripts/check-no-node-only-core-imports-in-dashboard.mjs && node scripts/check-pi-versions-pinned.mjs && node scripts/check-workspace-package-graph.mjs && node scripts/check-no-test-timeout-appeasement.mjs && node scripts/check-changeset-format.mjs && node scripts/check-pre-json-anchor.mjs && node scripts/check-routes-modular.mjs && node scripts/check-runtime-skill-loader-drift.mjs", - "pretest:full": "node scripts/check-no-nohup.mjs && node scripts/check-no-cwd-relative-dashboard-test-reads.mjs && node scripts/check-no-kill-4040.mjs && node scripts/check-no-getdatabase.mjs && node scripts/check-prerebase-inert.mjs && node scripts/check-capacity-pool-id.mjs && node scripts/check-cli-runtime-routing.mjs && node scripts/check-no-node-only-core-imports-in-dashboard.mjs && node scripts/check-pi-versions-pinned.mjs && node scripts/check-workspace-package-graph.mjs && node scripts/check-no-test-timeout-appeasement.mjs && node scripts/check-changeset-format.mjs && node scripts/check-routes-modular.mjs && node scripts/check-runtime-skill-loader-drift.mjs", + "pretest": "node scripts/check-no-nohup.mjs && node scripts/check-no-cwd-relative-dashboard-test-reads.mjs && node scripts/check-no-kill-4040.mjs && node scripts/check-no-getdatabase.mjs && node scripts/check-prerebase-inert.mjs && node scripts/check-capacity-pool-id.mjs && node scripts/check-cli-runtime-routing.mjs && node scripts/check-no-node-only-core-imports-in-dashboard.mjs && node scripts/check-pi-versions-pinned.mjs && node scripts/check-workspace-package-graph.mjs && node scripts/check-no-test-timeout-appeasement.mjs && node scripts/check-no-comment-assertions-in-tests.mjs && node scripts/check-changeset-format.mjs && node scripts/check-pre-json-anchor.mjs && node scripts/check-routes-modular.mjs && node scripts/check-runtime-skill-loader-drift.mjs", + "pretest:full": "node scripts/check-no-nohup.mjs && node scripts/check-no-cwd-relative-dashboard-test-reads.mjs && node scripts/check-no-kill-4040.mjs && node scripts/check-no-getdatabase.mjs && node scripts/check-prerebase-inert.mjs && node scripts/check-capacity-pool-id.mjs && node scripts/check-cli-runtime-routing.mjs && node scripts/check-no-node-only-core-imports-in-dashboard.mjs && node scripts/check-pi-versions-pinned.mjs && node scripts/check-workspace-package-graph.mjs && node scripts/check-no-test-timeout-appeasement.mjs && node scripts/check-no-comment-assertions-in-tests.mjs && node scripts/check-changeset-format.mjs && node scripts/check-routes-modular.mjs && node scripts/check-runtime-skill-loader-drift.mjs", "check:line-count": "node scripts/check-file-line-count.mjs", "check:routes-modular": "node scripts/check-routes-modular.mjs", "check:changesets": "node scripts/check-changeset-format.mjs", @@ -32,7 +32,7 @@ "census:lifecycle-columns": "node scripts/lifecycle-column-census.mjs", "check:quarantine-ledger": "node scripts/check-quarantine-ledger.mjs --strict", "check:mock-completeness": "node scripts/check-mock-completeness.mjs", - "test:gate:static": "node scripts/check-no-nohup.mjs && node scripts/check-no-cwd-relative-dashboard-test-reads.mjs && node scripts/check-no-kill-4040.mjs && node scripts/check-no-getdatabase.mjs && node scripts/check-prerebase-inert.mjs && node scripts/check-capacity-pool-id.mjs && node scripts/check-cli-runtime-routing.mjs && node scripts/check-no-node-only-core-imports-in-dashboard.mjs && node scripts/check-pi-versions-pinned.mjs && node scripts/check-workspace-package-graph.mjs && node scripts/check-no-test-timeout-appeasement.mjs && node scripts/check-changeset-format.mjs && node scripts/check-mock-completeness.mjs && node scripts/check-inert-sync-lane-conversions.mjs && node scripts/check-runtime-skill-loader-drift.mjs", + "test:gate:static": "node scripts/check-no-nohup.mjs && node scripts/check-no-cwd-relative-dashboard-test-reads.mjs && node scripts/check-no-kill-4040.mjs && node scripts/check-no-getdatabase.mjs && node scripts/check-prerebase-inert.mjs && node scripts/check-capacity-pool-id.mjs && node scripts/check-cli-runtime-routing.mjs && node scripts/check-no-node-only-core-imports-in-dashboard.mjs && node scripts/check-pi-versions-pinned.mjs && node scripts/check-workspace-package-graph.mjs && node scripts/check-no-test-timeout-appeasement.mjs && node scripts/check-no-comment-assertions-in-tests.mjs && node scripts/check-changeset-format.mjs && node scripts/check-mock-completeness.mjs && node scripts/check-inert-sync-lane-conversions.mjs && node scripts/check-runtime-skill-loader-drift.mjs", "test:gate": "node scripts/run-static-gate-checks.mjs && sh -c 'pnpm --filter @fusion/engine test:core & engine_pid=$!; pnpm --filter @fusion/core test:pg-gate & pg_pid=$!; pnpm --filter @fusion/core test:unit-gate & unit_pid=$!; status=0; wait $engine_pid || status=1; wait $pg_pid || status=1; wait $unit_pid || status=1; exit $status' && pnpm --filter @runfusion/fusion test:ci-shape", "pg:test:up": "node scripts/pg-test-server.mjs up", "pg:test:status": "node scripts/pg-test-server.mjs status", diff --git a/packages/cli/src/commands/__tests__/grok-runtime-bootstrap.test.ts b/packages/cli/src/commands/__tests__/grok-runtime-bootstrap.test.ts index f28a7fe32b..b747e7c67c 100644 --- a/packages/cli/src/commands/__tests__/grok-runtime-bootstrap.test.ts +++ b/packages/cli/src/commands/__tests__/grok-runtime-bootstrap.test.ts @@ -97,8 +97,6 @@ describe("Grok CLI PluginRunner host wiring", () => { expect(mergeFnIndex).toBeGreaterThanOrEqual(0); const nextExport = source.indexOf("export async function runTaskAttach", mergeFnIndex); const mergeFnBody = source.slice(mergeFnIndex, nextExport > 0 ? nextExport : undefined); - expect(mergeFnBody).toContain("FNXC:GrokCliRouting 2026-07-15-10:17"); - expect(mergeFnBody).toContain("Do not invent a full PluginRunner bootstrap"); expect(mergeFnBody).not.toContain("mergePluginRunner"); }); }); diff --git a/packages/core/src/tasks/prompt-overrides.ts b/packages/core/src/tasks/prompt-overrides.ts index 046c14f51b..7b31da22f2 100644 --- a/packages/core/src/tasks/prompt-overrides.ts +++ b/packages/core/src/tasks/prompt-overrides.ts @@ -178,6 +178,10 @@ If there are merge conflicts: 5. Run \`git add \` for each resolved file 6. Do NOT change anything beyond what's needed to resolve the conflict`, }, + /* + FNXC:PromptOverrides 2026-08-23-23:25: + THIS defaultContent is the prompt agent generation actually ships: `resolvePrompt` returns it whenever no operator override exists, so the identical literal exported as `AGENT_GENERATION_SYSTEM_PROMPT` in the dashboard is only the fallback-of-a-fallback. FN-021 added the xhigh/max thinking levels to that copy alone, which left every real generation run advertising levels that stop at "high". Any edit to one copy must be made to both. + */ "agent-generation-system": { key: "agent-generation-system", name: "Agent Generation System", @@ -202,7 +206,7 @@ You MUST respond with ONLY valid JSON (no markdown, no explanation): "role": "The most appropriate capability: triage | executor | reviewer | merger | scheduler | engineer | custom", "description": "A brief 1-2 sentence description of the agent's purpose and expertise", "systemPrompt": "A detailed markdown system prompt for the agent. This should be comprehensive and include:\\n- Role definition\\n- Core responsibilities\\n- Specific areas of expertise\\n- Behavioral guidelines\\n- Output format expectations\\n- Edge case handling instructions", - "thinkingLevel": "off | minimal | low | medium | high", + "thinkingLevel": "off | minimal | low | medium | high | xhigh | max", "maxTurns": 1000 } @@ -220,6 +224,7 @@ You MUST respond with ONLY valid JSON (no markdown, no explanation): - "low": For moderate complexity tasks - "medium": For complex analysis, code review, architecture decisions - "high": For critical decisions, security analysis, complex debugging +- "xhigh" or "max": For the hardest problems when the selected model advertises that level ## Max Turns Guidelines - 5-10: Simple, focused tasks (quick reviews, status checks) diff --git a/packages/i18n/locales/en/app.json b/packages/i18n/locales/en/app.json index 12aaad294d..1e9534c3b2 100644 --- a/packages/i18n/locales/en/app.json +++ b/packages/i18n/locales/en/app.json @@ -6300,6 +6300,8 @@ "usedAutomaticallyIfThePrimaryDefaultModelHits": "Used automatically if the primary default model hits a retryable provider error like rate limiting or overload. No default — unset.", "useDefault": "Use default", "whenEnabledStartupFetchesTheLatestAvailableModels": " When enabled, startup fetches the latest available models from the OpenRouter API so model pickers always include the newest catalog. Default: enabled. ", + "syncOrcaRouterModelListAtStartup": " Sync OrcaRouter model list at startup ", + "whenEnabledStartupFetchesTheLatestOrcaRouterModels": " When enabled, startup fetches the latest available models from the OrcaRouter API so model pickers include the OrcaRouter catalog. Default: enabled. ", "whenEnabledStartupRefreshesModelsThroughTheLocal": " When enabled, startup refreshes models through the local ", "commaSeparatedValuesSentToOpenRouterModelSyncOutputModalities": "Comma-separated values sent to OpenRouter model sync. No default — unset (unfiltered).", "openRouterRoutingOrderHint": "No default — unset (OpenRouter's own default routing order applies).", diff --git a/scripts/check-no-comment-assertions-in-tests.mjs b/scripts/check-no-comment-assertions-in-tests.mjs new file mode 100644 index 0000000000..ffdac5c7d2 --- /dev/null +++ b/scripts/check-no-comment-assertions-in-tests.mjs @@ -0,0 +1,93 @@ +#!/usr/bin/env node +/* +FNXC:TestHygiene 2026-08-23-23:50: +TESTS ASSERT BEHAVIOR, NEVER COMMENT TEXT. A test that pins an `FNXC:` block, a date stamp, or any +comment prose from a source/CSS file guards documentation instead of behavior — and this repo's own +convention tells authors to keep those comments updated as requirements change, so the two rules +fight and the test loses in the worst possible way. + +Measured 2026-08-23: a commit legitimately rewrote `runTaskMerge` and dropped its FNXC block; +`grok-runtime-bootstrap.test.ts` went red because it asserted that comment existed, and the "fix" +was to RE-ADD THE COMMENT to the product file. A comment returned to the shipped source not because +it documented anything true, but to appease a test. Four more such assertions existed across the +dashboard's CSS tests, each sitting beside a real assertion and adding nothing. + +Scope is deliberately narrow. This bans asserting PROSE. Guards that scan source for CODE CONSTRUCTS +or call-site allowlists — engine-no-blocking-shellout, vi-mock specifier resolution, durable-write +and emit-surface inventories, legacy tombstones — are a different category, are mandated elsewhere +in AGENTS.md, and are untouched here. + +If a pinned comment was standing in for a real invariant, assert the invariant: the rendered style, +the observable outcome, the absent construct. If nothing behavioral is behind it, delete the line. +*/ +import { readFileSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; + +const SCAN_ROOTS = ["packages", "plugins"]; +const TEST_FILE_PATTERN = /\.test\.(?:ts|tsx|mts|cts|mjs|cjs|js|jsx)$/; +/* +An assertion matcher applied to a string carrying an FNXC stamp. + +DELIBERATELY NARROW: an earlier draft also flagged `/*` and `*​/` as comment delimiters and produced +24 false positives and zero true ones - every hit was a path glob or a legitimate pattern +assertion, which a regex cannot tell apart from comment prose. Comment prose is not reliably distinguishable from +data by regex, so this check enforces the one unambiguous, observed case and the standing rule in +AGENTS.md covers the rest for human and agent reviewers. +*/ +const ASSERTION_PATTERN = + /\.(?:toContain|toMatch|toContainEqual|toHaveTextContent)\s*\(\s*[`"'][^`"']*FNXC:/; + +function isTestFile(filePath) { + return TEST_FILE_PATTERN.test(filePath); +} + +function listTrackedTestFiles() { + const result = spawnSync("git", ["ls-files", "--", ...SCAN_ROOTS], { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }); + if (result.status !== 0) throw new Error(result.stderr?.trim() || "git ls-files failed"); + return result.stdout.split("\n").map((line) => line.trim()).filter(isTestFile); +} + +export function scanTrackedFiles(files = listTrackedTestFiles()) { + const matches = []; + for (const filePath of files) { + let source; + try { + source = readFileSync(filePath, "utf8"); + } catch { + continue; + } + if (!source.includes("FNXC:")) continue; + source.split("\n").forEach((line, index) => { + if (ASSERTION_PATTERN.test(line)) { + matches.push({ filePath, lineNumber: index + 1, line }); + } + }); + } + return matches; +} + +export function formatFailureMessage(matches) { + return [ + "[check-no-comment-assertions-in-tests] a test asserts COMMENT TEXT instead of behavior.", + "Comments are documentation: AGENTS.md asks authors to keep FNXC blocks current, so pinning one guarantees a future false failure —", + "and the tempting 'fix' is to re-add a comment to product source purely to satisfy the test (observed 2026-08-23).", + "Assert the behavior instead — the rendered style, the observable outcome, the absent code construct — or delete the assertion if nothing behavioral is behind it.", + "Code-construct and call-site-allowlist guards are a different category and are not affected by this check.", + ...matches.map(({ filePath, lineNumber, line }) => `${filePath}:${lineNumber}: ${line.trim()}`), + ].join("\n"); +} + +export function main() { + const matches = scanTrackedFiles(); + if (matches.length === 0) return 0; + console.error(formatFailureMessage(matches)); + return 1; +} + +if (process.argv[1] && fileURLToPath(import.meta.url) === process.argv[1]) { + process.exitCode = main(); +}