diff --git a/.github/workflows/fork-only-patches.yml b/.github/workflows/fork-only-patches.yml new file mode 100644 index 0000000000..cc7e274d4c --- /dev/null +++ b/.github/workflows/fork-only-patches.yml @@ -0,0 +1,74 @@ +# Fork-only patch guard. +# +# Every other workflow in this repo triggers on `main`. But fork-only patches +# are lost on `develop_base` - that is the branch rebuilt onto upstream main, +# and the branch every fork PR targets. Without this workflow the guard would +# only ever run when a human remembered to run `pnpm check`, which is the same +# failure mode it was built to remove. +# +# Deliberately NOT gated by paths-ignore: a rebuild can drop a patch regardless +# of which paths its own diff appears to touch. +# +# This file is itself a registered fork-only patch (see +# scripts/fork-only-patches.json), so a rebuild that drops the guard's CI +# trigger is caught by the local `pnpm check` run. +name: Fork-only patches + +on: + push: + branches: [develop_base] + pull_request: + branches: [develop_base] + workflow_dispatch: + +concurrency: + group: fork-only-patches-${{ github.ref }} + cancel-in-progress: true + +jobs: + guard: + name: Fork-only patch guard + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - uses: actions/setup-node@v4 + with: + node-version: 24 + + # The baseline must be the IMMUTABLE PRE-push commit. Fetching + # `origin/develop_base` here would name the tree we are validating - a + # witness that is the accused - and the old code did exactly that. + # GitHub supplies the previous tip out of band, which the pushed tree + # cannot forge: github.event.before for a push (survives force-push), + # pull_request.base.sha for a PR. + - id: baseline + run: | + case "${{ github.event_name }}" in + push) sha='${{ github.event.before }}' ;; + pull_request) sha='${{ github.event.pull_request.base.sha }}' ;; + *) sha='' ;; + esac + # All-zeros means the branch did not exist before: no prior claim. + if [ -z "$sha" ] || [ "$sha" = "0000000000000000000000000000000000000000" ]; then + echo "sha=" >> "$GITHUB_OUTPUT" + else + # A force-pushed rebuild leaves the old tip unreachable from any ref. + git fetch --no-tags origin "$sha" 2>/dev/null || true + echo "sha=$sha" >> "$GITHUB_OUTPUT" + fi + + # No dependency install: the guard uses node builtins only, so it stays + # runnable even when the tree is mid-rebuild and deps are inconsistent. + - run: node --test scripts/check-fork-only-patches.test.mjs + - name: Fork-only patch guard + run: | + # No `||` fallback on purpose: if the baseline was supposed to exist + # and cannot be resolved, the guard must go red, not quietly retry + # with a weaker check. + if [ -n "${{ steps.baseline.outputs.sha }}" ]; then + node scripts/check-fork-only-patches.mjs --baseline-ref "${{ steps.baseline.outputs.sha }}" + else + node scripts/check-fork-only-patches.mjs --no-baseline + fi diff --git a/docs/fork-only-patches.md b/docs/fork-only-patches.md new file mode 100644 index 0000000000..3491c033d2 --- /dev/null +++ b/docs/fork-only-patches.md @@ -0,0 +1,196 @@ +# Fork-only patches + +Patches this fork carries that **upstream `zts212653/clowder-ai` main does not**. + +Every `develop_base` rebuild onto upstream main starts from a tree without them. +If nobody re-applies them, they vanish silently. + +## Truth source + +`scripts/fork-only-patches.json` — machine-readable registry. +Enforced by `scripts/check-fork-only-patches.mjs`, wired into `pnpm check`. + +This document explains the *why*; the registry is the *what*. Do not duplicate +the file/anchor lists here — they would drift. + +## Why a guard instead of a checklist + +The F192 evidence-prerequisite gate was lost **three times**: + +| When | How | Noticed after | +|---|---|---| +| 2026-08-11 | develop_base rebuild | ~2 days (PR #91 recovery) | +| 2026-08-28 | develop_base rebuild | ~1 day (PR #137 recovery) | +| 2026-09-01 | commit `70d79a2f2` — a commit whose own title was *"recover fork-only development layer lost in develop_base rebuild"* | **21 days** | + +Each loss let the scheduler burn one full LLM session per daily eval fire, +re-concluding a gap it could not act on. + +The third loss is the decisive one: a rebuild that was *explicitly trying* to +recover fork-only code still missed this patch. Human/agent recall is not a +mechanism. A guard is. + +## Upstream boundary + +Pushing these patches upstream is **not** the answer, and was already decided: + +> Closing — this is fork-specific harness logic that doesn't belong in upstream. +> The rebuild-loss issue should be solved by improving our fork's develop_base +> rebuild SOP (rebase fork-only patches), not by pushing internal code into the +> community repo. +> +> — `zts212653/clowder-ai#1352`, closed 2026-08-25 + +Upstream review of that PR also surfaced two real defects, both fixed in the +fork version rather than carried forward blindly: + +1. The probe read the **scheduler process's** telemetry handle while evidence is + fetched from the **adapter target** (`EVAL_BASE_URL`). These coincide in this + fork's single-process deployment, but only accidentally. The probe now + asserts co-location via `evidenceTargetIsLocal` and fails closed when it + cannot prove it. +2. The skip notice always advised configuring `TELEMETRY_HMAC_SALT`, even when + the detected cause was an intentional `OTEL_SDK_DISABLED=true`. The next-step + text is now derived from the detected cause. + +## Why a tree-only check is not enough + +A develop_base rebuild is, quoting commit `70d79a2f2`'s own body: + +> The develop_base rebuild (b739c279c) reset to origin/main + fork/optimizations +> but dropped all develop_base-only merges - 33 first-parent commits [...] +> 443 files modified by BOTH main and fork: **NOT restored** + +Two things follow. + +First, the shared-file edits (the `requiredAnchors` below) fall exactly in that +"modified by BOTH / not restored" bucket. The file survives, the fork's edit +inside it does not - so an existence check reads clean while the patch is gone. + +Second, and worse: a check that reads its registry **from the tree** cannot +report on a rebuild that replaces the tree. The reset takes the registry and +this script along with the patches, and a guard that is not there does not go +red. That is how three losses stayed silent. + +`fork/optimizations` does not solve this either. It is an orphan snapshot - 7 +commits, no merge-base with upstream main, a July full-tree copy - so +"is the patch in the carrier" is not a survival predicate, and asking for +current code to be committed onto a stale unrelated history is incoherent. + +## What actually survives: an immutable pre-reset commit + +`git reset --hard upstream/main` replaces the working tree, so the baseline has +to come from outside it. Two sources qualify, and one that looks like it does +**not**: + +| Source | Immutable? | Notes | +|---|---|---| +| `github.event.before` (push) | yes | previous branch tip, supplied by the platform; survives force-push | +| `github.event.pull_request.base.sha` | yes | the PR base | +| local `origin/develop_base` **before** pushing | yes, briefly | still names the prior tip during a manual pre-push run | +| `origin/develop_base` fetched **after** the push | **NO** | it is the tree being validated | + +That last row was a real bug in this workflow's first version: CI fetched +`origin/develop_base` after the push, so the baseline *was* the accused. The +checker now refuses to compare a commit with itself **when the baseline was +named explicitly**, instead of reporting a cheerful green. + +### Explicit vs default baseline + +Who names the baseline decides what "baseline == HEAD" means: + +| Caller | baseline == HEAD means | Result | +|---|---|---| +| explicit `--baseline-ref` (CI) | the caller mis-wired the witness | **fail closed** | +| registry default (manual run) | there is no older tree in this checkout | HEAD witnesses the **working tree** | + +The default case is the normal state of a clean `develop_base` checkout. Failing +closed there made the first step of canonical `pnpm check` red on the branch the +guard protects, so nothing after it ran. Instead, the committed HEAD is compared +against the working tree: an uncommitted registry or patch reversion is still +caught. What the default path **cannot** see is a loss that was already +committed - the output says so explicitly - and that is exactly why CI passes +the immutable pre-reset commit, and why the registry anchors that wiring in the +workflow: deleting `--baseline-ref` from CI turns the tree check red rather +than quietly falling back to the default. + +### The registry is a shared file too + +A rebuild can **revert** `scripts/fork-only-patches.json` rather than delete it. +Both trees then look internally consistent, and a guard that only checks the +patches its *current* registry lists will confirm its own amnesia as healthy. +So the survival check compares the **claim sets**: every patch id the baseline +registered must still be registered. A claim that quietly disappeared from the +registry is a loss, not a smaller honest claim. + +### Running it + +```bash +# Rebuild pipeline: capture the tip BEFORE the destructive reset. +PRE=$(git rev-parse origin/develop_base) +git fetch upstream && git reset --hard upstream/main # ... rebuild ... +node scripts/check-fork-only-patches.mjs --baseline-ref "$PRE" +``` + +`--no-baseline` skips the survival half and says so in the output. It exists for +the one honest case with no prior tree (branch creation) and must never be used +as a fallback when a baseline lookup failed - a guard that quietly downgrades +itself is the disease, not the cure. + +## What this in-tree workflow does NOT close + +`.github/workflows/fork-only-patches.yml` cannot survive its own deletion. If a +rebuild drops workflow + guard + registry together, GitHub runs the workflow +**from the pushed commit**, where it no longer exists, so nothing runs and +nothing goes red. + +So, honestly scoped: + +- **Closed by the workflow**: any rebuild that preserves it - the baseline is the + immutable pre-push SHA, so dropped files, reverted anchors and dropped registry + claims are all named before the result is deployed. +- **NOT closed by the workflow**: a rebuild that deletes the workflow itself. + +The authoritative enforcement for that case lives **outside the repository**, in +branch protection, and is not configured by this PR: + +> Settings -> Branches -> branch protection rule for `develop_base` +> -> **Require status checks to pass before merging** +> -> required check: **`Fork-only patch guard`** + +A required check that never reports **blocks** rather than passes, which is +exactly the fail-closed behaviour the in-tree file cannot provide for its own +absence. This requires repository-admin rights; until it is configured, treat +the workflow as covering the common case only, and do not claim the full-wipe +case is mechanically prevented. + +## Rebuild SOP + +Run the guard as the **last step of a rebuild, before pushing**: + +```bash +git fetch origin develop_base # baseline must be present +pnpm check:fork-only-patches +``` + +Red output names each dropped file and anchor. Re-apply them, then re-run. Do +not push or deploy a rebuild while this check is red. + +CI runs the same guard on every push and PR to `develop_base` +(`.github/workflows/fork-only-patches.yml`) - every other workflow in this repo +triggers on `main` only, which is why losses on develop_base went unseen. + +## Adding a patch to the registry + +Add an entry to `scripts/fork-only-patches.json` with: + +- `id`, `owner`, `why` — what breaks in production if it disappears +- `upstreamStatus` — why it is not upstream (so nobody re-litigates it) +- `requiredFiles` — files that must exist +- `requiredAnchors` — files that must still *contain* given symbols, for patches + that modify shared files rather than adding new ones (these are the ones a + rebuild silently reverts, since the file still exists) +- `order` - call-site strings that must appear in sequence, for patches whose + meaning is positional. Bare substrings are satisfied by an import, an + interface field or a comment; anchor on the call site instead, so a gate + moved after the step it must precede is caught. diff --git a/package.json b/package.json index 644a4e2334..80ee0fb769 100644 --- a/package.json +++ b/package.json @@ -60,7 +60,7 @@ "build": "pnpm -r run build", "build:app-assist": "node desktop/scripts/build-app-assist.cjs", "lint": "pnpm -r run lint", - "check:sources": "pnpm check:brand-guard && pnpm check:no-provider-canary-residue && pnpm check:paw-feel-disposition-boundaries && pnpm check:f290-product-copy && pnpm check:f290-workbench-boundary && pnpm check:dir-size && pnpm check:daemon-lifecycle && pnpm check:capability-tips && pnpm check:co-creation-docs-lane && pnpm check:review-completion-routing && pnpm check:skills:references && pnpm check:skills:surfaces && pnpm check:env-registry && pnpm check:env-example && pnpm check:worktree-reaper && pnpm check:node-runtime-guard && pnpm check:prompt-time-apis && pnpm check:followup-tails && pnpm check:scripts-ascii-only && pnpm check:f212-reconciliation-eval && pnpm check:site-css", + "check:sources": "pnpm check:fork-only-patches && pnpm check:brand-guard && pnpm check:no-provider-canary-residue && pnpm check:paw-feel-disposition-boundaries && pnpm check:f290-product-copy && pnpm check:f290-workbench-boundary && pnpm check:dir-size && pnpm check:daemon-lifecycle && pnpm check:capability-tips && pnpm check:co-creation-docs-lane && pnpm check:review-completion-routing && pnpm check:skills:references && pnpm check:skills:surfaces && pnpm check:env-registry && pnpm check:env-example && pnpm check:worktree-reaper && pnpm check:node-runtime-guard && pnpm check:prompt-time-apis && pnpm check:followup-tails && pnpm check:scripts-ascii-only && pnpm check:f212-reconciliation-eval && pnpm check:site-css", "check:installed": "pnpm check:sidebar-projection-boundary && pnpm biome check . --diagnostic-level=error && pnpm check:biome-review-worktrees && pnpm check:sop-definitions && pnpm check:design-gate-real-interaction && pnpm check:design-md && pnpm check:skills:manifest && pnpm check:env-ports && pnpm check:start-profile-isolation && pnpm check:guides && pnpm check:no-codex-provider-exit-suppression && pnpm check:whisper-dispatch && pnpm check:tts-runtime && pnpm check:site-integrity", "check:artifacts": "pnpm check:paw-feel-disposition-capacity && pnpm check:mcp-surface-governance && pnpm check:features && pnpm check:eval-metric-glossary:prepared && pnpm check:capability-tips-enablement && pnpm check:pre-merge-gate", "check": "pnpm check:sources && pnpm check:installed && pnpm check:artifacts", @@ -72,6 +72,7 @@ "check:mcp-surface-governance": "pnpm --filter @cat-cafe/mcp-server governance:check", "check:no-codex-provider-exit-suppression": "node --test --test-concurrency=1 scripts/check-no-codex-provider-exit-suppression-literal.test.mjs scripts/check-no-codex-provider-exit-suppression-ast.test.mjs scripts/check-no-codex-provider-exit-suppression-isolation.test.mjs scripts/check-no-codex-provider-exit-suppression-structural.test.mjs scripts/check-no-provider-canary-residue.test.mjs && node scripts/check-no-codex-provider-exit-suppression.mjs && node scripts/check-no-provider-canary-residue.mjs", "check:no-provider-canary-residue": "node scripts/check-no-provider-canary-residue.mjs", + "check:fork-only-patches": "node --test scripts/check-fork-only-patches.test.mjs && node scripts/check-fork-only-patches.mjs", "check:paw-feel-disposition-boundaries": "node --test scripts/check-paw-feel-disposition-boundaries.test.mjs && node scripts/check-paw-feel-disposition-boundaries.mjs", "check:paw-feel-disposition-capacity": "pnpm --filter @cat-cafe/shared build && pnpm --filter @cat-cafe/api build && node --test packages/api/test/harness-eval/paw-feel-disposition-capacity.test.js", "check:f212-reconciliation-eval": "node --test scripts/eval-f212-reconciliation/reconciliation-baseline.test.mjs scripts/eval-f212-reconciliation/reconciliation-window.test.mjs scripts/eval-f212-reconciliation/reconciliation-matcher.test.mjs scripts/eval-f212-reconciliation/reconciliation-sequence.test.mjs scripts/eval-f212-reconciliation/reconciliation-malformed.test.mjs scripts/eval-f212-reconciliation/reconciliation-compression.test.mjs", diff --git a/packages/api/src/index.ts b/packages/api/src/index.ts index b00723a4a7..91d57b6b58 100644 --- a/packages/api/src/index.ts +++ b/packages/api/src/index.ts @@ -7415,6 +7415,10 @@ async function main(): Promise { ); // N-day factory is in its own module (split from eval-domain-daily for file-size limit) const { createEvalDomainNDaySpec } = await import('./infrastructure/harness-eval/domain/eval-domain-nday.js'); + // F192 evidence-source prerequisite gate (fork-only; see docs/fork-only-patches.md). + const { createTelemetryEvidencePrereqProbe } = await import( + './infrastructure/harness-eval/domain/eval-domain-evidence-gate.js' + ); const { getOwnerUserId } = await import('./config/cat-config-loader.js'); // cloud R6 P2 (PR-2) + memory wire-up: mirror the same wired set the // eval-hub.ts route computes (Object.keys(verdictGenerators)). Bootstrap-time @@ -7482,6 +7486,17 @@ async function main(): Promise { publishPrereqCache.set(domainId, ok); return ok; }; + // F192 evidence-source prerequisite gate. OTel init state is fixed for the + // process lifetime (salt is read at boot), so a boolean thunk is a complete input. + // No co-location check: f167-runtime-eval evidence is a snapshot+attribution pair + // on disk under harnessFeedbackRoot (below), written by THIS process's OTel + // pipeline. The producer is this process by construction, so the local handle is + // authoritative. See eval-domain-evidence-gate.ts for why the earlier + // EVAL_BASE_URL-derived assertion was removed rather than repaired. + const evidencePrereqProbe = createTelemetryEvidencePrereqProbe({ + otelEnabled: () => telemetryHandle.getMetricsText !== null, + }); + const evalScheduleOpts = { harnessFeedbackRoot: resolve(repoRoot, 'docs', 'harness-feedback'), threadStore, @@ -7490,6 +7505,7 @@ async function main(): Promise { redis: redisClient ?? undefined, wiredPublishDomains, publishPrereqProbe, + evidencePrereqProbe, triggerStore: redisClient ? new ( await import('./infrastructure/harness-eval/domain/eval-domain-trigger-store.js') diff --git a/packages/api/src/infrastructure/harness-eval/domain/eval-domain-daily.ts b/packages/api/src/infrastructure/harness-eval/domain/eval-domain-daily.ts index 0f4e8bd505..fd25825626 100644 --- a/packages/api/src/infrastructure/harness-eval/domain/eval-domain-daily.ts +++ b/packages/api/src/infrastructure/harness-eval/domain/eval-domain-daily.ts @@ -18,6 +18,11 @@ import type { TaskSpec_P1 } from '../../scheduler/types.js'; import { buildEvalCatInvocation } from '../eval-cat-invocation.js'; import { ensureEvalDomainThreads } from '../hub/eval-hub-thread-ensure.js'; import { inventoryLegacyTasks, type LegacyScheduledTaskLike } from '../legacy-task-cleanup.js'; +import { + buildEvidencePrereqSkippedMessage, + type EvidencePrereqProbe, + evaluateEvidencePrereq, +} from './eval-domain-evidence-gate.js'; import { getEvalCatOverride } from './eval-domain-override.js'; import { type EvalDomainRegistryEntry, @@ -67,6 +72,20 @@ export interface EvalDomainScheduleOpts { * checks for a known-post-fix symbol (e.g. `isA2aSourceRefs` for `eval:a2a`). */ publishPrereqProbe?: (domainId: EvalDomainRegistryEntry['domainId']) => boolean | Promise; + /** + * F192 evidence-source prerequisite gate. + * + * Upstream of `publishPrereqProbe` — answers "can the domain's evidence source + * PRODUCE evidence at all?" before the eval cat is invoked. When OTel is + * disabled, `f167-runtime-eval`-backed domains cannot generate fresh + * snapshots, so invoking the eval cat burns an LLM session to re-conclude a + * gap that is already visible. + * + * Runs PER DOMAIN, PER CRON FIRE, BEFORE publishPrereqProbe. Fail-closed: + * probe throws -> treated as "evidence unavailable" -> skip notice posted. + * Omit/undefined -> backward-compat (no skip). + */ + evidencePrereqProbe?: EvidencePrereqProbe; } /** @deprecated Use EvalDomainScheduleOpts — kept for backward compat. */ @@ -207,6 +226,23 @@ function createEvalDomainSpec(config: EvalDomainSpecConfig): TaskSpec_P1; + +export type EvidencePrereqResult = { ok: true } | { ok: false; reason: string }; + +export type EvidencePrereqProbe = (domain: EvidenceGateDomain) => EvidencePrereqResult | Promise; + +/** + * Source adapters whose evidence pipeline hard-requires live OTel telemetry. + * Registry `sourceAdapter` is a free slug (see eval-domain-registry.ts), so + * the adapter → prerequisite mapping lives here, next to the probe. + */ +const TELEMETRY_BACKED_ADAPTERS: ReadonlySet = new Set(['f167-runtime-eval']); + +/** + * Why there is no "is the evidence source co-located?" check here. + * + * Upstream review of zts212653/clowder-ai#1352 raised a real-sounding concern: + * `otelEnabled` observes the telemetry handle of the process running the CRON, + * so if evidence came from a DIFFERENT runtime the local handle would prove + * nothing about it, in both directions. + * + * A first fix attempt asserted co-location by parsing `EVAL_BASE_URL`. Review of + * THIS fork's PR (#185) showed that gate was wired to unrelated configuration: + * `EVAL_BASE_URL` is read only by the manual CLI `scripts/run-f168-external-case-eval.mjs` + * and is never consulted on the scheduled path. It also mis-compared ports and + * could never match `::1` (Node's URL keeps the brackets: `"[::1]"`), so it fell + * closed on exactly the local topology it was meant to admit. + * + * The scheduled path has no remote evidence target to check. `f167-runtime-eval` + * evidence is a snapshot+attribution PAIR ON DISK under the harness-feedback root + * (see publish-verdict/a2a-generator-adapter.ts), written by this process's own + * OTel pipeline. Source resolution is filesystem-relative, not HTTP. The local + * handle is authoritative because the producer IS this process - structurally, + * not accidentally. + * + * If a telemetry-backed adapter ever fetches evidence over the network, the + * decision belongs at THIS boundary: add it to the set above only together with + * a probe that can observe that source. Do not re-derive co-location from env. + */ + +export function isTelemetryBackedAdapter(sourceAdapter: string): boolean { + return TELEMETRY_BACKED_ADAPTERS.has(sourceAdapter); +} + +/** + * Probe factory. Bootstrap wires `otelEnabled: () => !!telemetryHandle.getMetricsText` + * — the same init-state signal `GET /api/telemetry/health` reports as + * `otelEnabled` (routes/telemetry.ts Phase K note: actual init state, not an + * env-var proxy). Non-telemetry-backed adapters always pass through. + */ +export function createTelemetryEvidencePrereqProbe(opts: { + otelEnabled: () => boolean; + /** Override the reason text; defaults to the health route's disabledReason derivation. */ + disabledReason?: () => string; +}): EvidencePrereqProbe { + return (domain) => { + if (!isTelemetryBackedAdapter(domain.sourceAdapter)) return { ok: true }; + if (opts.otelEnabled()) return { ok: true }; + const reason = + opts.disabledReason?.() ?? + (process.env.OTEL_SDK_DISABLED === 'true' + ? 'OTel disabled by OTEL_SDK_DISABLED=true' + : 'OTel disabled at boot: HMAC salt validation failed (TELEMETRY_HMAC_SALT not configured)'); + return { ok: false, reason }; + }; +} + +/** Fail-closed evaluation: a probe that throws is treated as "evidence unavailable". */ +export async function evaluateEvidencePrereq( + probe: EvidencePrereqProbe, + domain: EvidenceGateDomain, +): Promise { + try { + return await Promise.resolve(probe(domain)); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + return { ok: false, reason: `evidence prereq probe threw: ${message}` }; + } +} + +/** + * Derive the actionable next step from the DETECTED cause. + * + * Upstream review finding (zts212653/clowder-ai#1352, comment by 砚砚): the + * notice previously recommended configuring `TELEMETRY_HMAC_SALT` even when + * the detected cause was an intentional `OTEL_SDK_DISABLED=true`. Telling an + * operator to set a salt they already set — for a toggle they deliberately + * flipped — sends them down the wrong path. Match the advice to the cause. + */ +function buildNextActionLines(reason: string): string[] { + if (reason.includes('OTEL_SDK_DISABLED')) { + return [ + 'Next action: OTel is intentionally disabled via `OTEL_SDK_DISABLED=true`.', + 'Unset it (or set it to `false`) and restart the API runtime to resume', + "evidence collection, or set `enabled: false` in this domain's registry", + 'YAML to pause the schedule intentionally.', + ]; + } + if (reason.includes('TELEMETRY_HMAC_SALT') || /HMAC salt/i.test(reason)) { + return [ + 'Next action: configure a non-empty `TELEMETRY_HMAC_SALT` for the API', + 'runtime and restart it (OTel initializes at boot), or set `enabled: false`', + "in this domain's registry YAML to pause the schedule intentionally.", + ]; + } + return [ + 'Next action: resolve the blocker quoted above on the API runtime and', + "restart it, or set `enabled: false` in this domain's registry YAML to", + 'pause the schedule intentionally.', + ]; +} + +/** + * Stable-header skip notice posted to the domain's OWN system thread when the + * cron fails closed. Header format mirrors `buildPublishPrereqSkippedMessage` + * so eval-domain readers / log scrubbers can grep both skip classes uniformly. + */ +export function buildEvidencePrereqSkippedMessage(domain: EvidenceGateDomain, reason: string): string { + return [ + `## Eval Domain: ${domain.domainId} — SKIPPED (evidence source unavailable)`, + '', + "The scheduled eval was skipped because this domain's evidence source", + `(\`${domain.sourceAdapter}\`) cannot produce evidence on this runtime:`, + '', + `> ${reason}`, + '', + 'Why this matters: invoking the eval cat without a live evidence source', + 'burns a full LLM session to re-conclude the same gap on every fire. The', + 'fail-closed skip keeps the gap visible in this thread at zero LLM cost.', + '', + ...buildNextActionLines(reason), + ].join('\n'); +} diff --git a/packages/api/src/infrastructure/harness-eval/domain/eval-domain-nday.ts b/packages/api/src/infrastructure/harness-eval/domain/eval-domain-nday.ts index 6e9f2355dd..5acc37382b 100644 --- a/packages/api/src/infrastructure/harness-eval/domain/eval-domain-nday.ts +++ b/packages/api/src/infrastructure/harness-eval/domain/eval-domain-nday.ts @@ -20,6 +20,7 @@ import { type EvalDomainScheduleOpts, evaluatePublishPrereq, } from './eval-domain-daily.js'; +import { buildEvidencePrereqSkippedMessage, evaluateEvidencePrereq } from './eval-domain-evidence-gate.js'; import { getEvalCatOverride } from './eval-domain-override.js'; import { type EvalDomainRegistryEntry, @@ -157,6 +158,23 @@ export function createEvalDomainNDaySpec(opts: EvalDomainScheduleOpts): TaskSpec } // Direction B publish-prereq gate (same as daily/weekly spec) + // F192 evidence-source prereq gate (parity with daily/weekly). + // Critically: returns BEFORE the Redis last-dispatch write, so a skip does + // NOT consume the N-day window — the domain retries on the next daily probe. + if (opts.evidencePrereqProbe) { + const evidenceResult = await evaluateEvidencePrereq(opts.evidencePrereqProbe, domain); + if (!evidenceResult.ok) { + if (ctx.deliver) { + await ctx.deliver({ + threadId: domain.systemThreadId, + content: buildEvidencePrereqSkippedMessage(domain, evidenceResult.reason), + userId: 'scheduler', + }); + } + return; + } + } + if (opts.publishPrereqProbe) { const prereqOk = await evaluatePublishPrereq(opts.publishPrereqProbe, domain.domainId); if (!prereqOk) { diff --git a/packages/api/test/harness-eval/eval-domain-evidence-gate.test.js b/packages/api/test/harness-eval/eval-domain-evidence-gate.test.js new file mode 100644 index 0000000000..0adb9d4a0a --- /dev/null +++ b/packages/api/test/harness-eval/eval-domain-evidence-gate.test.js @@ -0,0 +1,353 @@ +import assert from 'node:assert/strict'; +import { readFileSync } from 'node:fs'; +import { describe, it, mock } from 'node:test'; +import { fileURLToPath } from 'node:url'; +import { createEvalDomainDailySpec } from '../../dist/infrastructure/harness-eval/domain/eval-domain-daily.js'; +import { + buildEvidencePrereqSkippedMessage, + createTelemetryEvidencePrereqProbe, + evaluateEvidencePrereq, +} from '../../dist/infrastructure/harness-eval/domain/eval-domain-evidence-gate.js'; +import { createEvalDomainNDaySpec } from '../../dist/infrastructure/harness-eval/domain/eval-domain-nday.js'; +import { FIXTURE_FRICTION_3D_YAML, makeTempRoot } from './eval-domain-nday-fixtures.js'; + +const repoHarnessFeedbackRoot = fileURLToPath(new URL('../../../../docs/harness-feedback', import.meta.url)); + +/** + * Evidence-source prereq gate (eval:a2a build verdict + * `2026-07-07-eval-a2a-reeval-telemetry-still-disabled-build`, PR #19). + * + * Bug class: the scheduled eval fires on a runtime whose OTel telemetry is + * disabled (TELEMETRY_HMAC_SALT unset → initTelemetry() returned null handles). + * The `f167-runtime-eval` source cannot produce fresh snapshots, yet the eval + * cat is invoked anyway and burns a full LLM session to re-conclude "telemetry + * still disabled" — every day (2026-06-30 → 2026-07-07 verdict series). The + * gate fails closed BEFORE invocation and posts a zero-LLM-cost skip notice + * to the domain's own system thread instead. + */ +describe('eval-domain evidence-source prereq gate (fork-only; mindfn PR #91/#137)', () => { + /** + * RFC 5.4: the eval cat is woken through the DELIVER seam - the envelope carries + * `targetCatId` + `privateContent`. The old positional `invokeTrigger.trigger()` + * call is dead, so asserting `triggerMock.callCount() === 0` proves NOTHING about + * whether the cat was invoked (it is trivially 0 on every path). + * + * `catWakes()` counts real wakes on the live seam, so "cat never invoked" is a + * claim this suite can actually falsify. + */ + const mkCtx = () => { + const deliverMock = mock.fn(async () => 'msg_evidence'); + const triggerMock = mock.fn(); + const catWakes = () => + deliverMock.mock.calls + .map((c) => c.arguments[0]) + .filter((env) => env?.targetCatId != null || env?.privateContent != null); + return { + deliverMock, + triggerMock, + catWakes, + ctx: { assignedCatId: null, deliver: deliverMock, invokeTrigger: { trigger: triggerMock } }, + }; + }; + + describe('createTelemetryEvidencePrereqProbe', () => { + it('telemetry-backed adapter + OTel disabled → not ok, reason points at salt', () => { + const probe = createTelemetryEvidencePrereqProbe({ otelEnabled: () => false }); + const result = probe({ domainId: 'eval:a2a', sourceAdapter: 'f167-runtime-eval' }); + assert.equal(result.ok, false); + assert.ok( + result.reason.includes('TELEMETRY_HMAC_SALT'), + `default reason must name the missing salt env var (got: ${result.reason})`, + ); + }); + + it('telemetry-backed adapter + OTel enabled → ok', () => { + const probe = createTelemetryEvidencePrereqProbe({ otelEnabled: () => true }); + const result = probe({ domainId: 'eval:a2a', sourceAdapter: 'f167-runtime-eval' }); + assert.equal(result.ok, true); + }); + + it('non-telemetry adapter passes through even when OTel is disabled', () => { + const probe = createTelemetryEvidencePrereqProbe({ otelEnabled: () => false }); + const result = probe({ domainId: 'eval:sop', sourceAdapter: 'sop-trace-eval' }); + assert.equal(result.ok, true, 'gate must only constrain telemetry-backed source adapters'); + }); + + it('OTEL_SDK_DISABLED=true → reason names the env toggle, not the salt', () => { + const prev = process.env.OTEL_SDK_DISABLED; + process.env.OTEL_SDK_DISABLED = 'true'; + try { + const probe = createTelemetryEvidencePrereqProbe({ otelEnabled: () => false }); + const result = probe({ domainId: 'eval:a2a', sourceAdapter: 'f167-runtime-eval' }); + assert.equal(result.ok, false); + assert.ok(result.reason.includes('OTEL_SDK_DISABLED'), `got: ${result.reason}`); + } finally { + if (prev === undefined) delete process.env.OTEL_SDK_DISABLED; + else process.env.OTEL_SDK_DISABLED = prev; + } + }); + }); + + /** + * PR #185 review (砚砚): the previous "co-location" assertion was wired to + * `EVAL_BASE_URL`, which the scheduled path never reads (only the manual CLI + * scripts/run-f168-external-case-eval.mjs does). It also compared the URL port + * against `API_SERVER_PORT` and tested `hostname === '::1'`, which Node reports + * as `'[::1]'` - so it fell CLOSED on the very local topology it meant to admit. + * + * It was removed, not repaired: f167-runtime-eval evidence is a snapshot pair on + * disk under the harness-feedback root, produced by this process. These tests pin + * that decision so the env-derived check is not reintroduced. + */ + describe('evidence-source topology contract', () => { + const A2A = { domainId: 'eval:a2a', sourceAdapter: 'f167-runtime-eval' }; + + function withEnv(key, value, fn) { + const prev = process.env[key]; + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + try { + return fn(); + } finally { + if (prev === undefined) delete process.env[key]; + else process.env[key] = prev; + } + } + + it('a remote-looking EVAL_BASE_URL does not change the verdict', () => { + // Regression: the gate must not re-derive evidence topology from env. The + // scheduled evidence path is filesystem-local regardless of this variable. + const probe = createTelemetryEvidencePrereqProbe({ otelEnabled: () => true }); + withEnv('EVAL_BASE_URL', 'http://evidence.example.com:9999', () => { + assert.equal(probe(A2A).ok, true, 'env must not gate the filesystem-local evidence path'); + }); + }); + + it('local topology is admitted on the documented default port with no env overrides', () => { + // The removed check failed exactly here: "3004" === String(undefined ?? '') + // is false, so a healthy co-located runtime was skipped. + const probe = createTelemetryEvidencePrereqProbe({ otelEnabled: () => true }); + withEnv('EVAL_BASE_URL', 'http://localhost:3004', () => + withEnv('API_SERVER_PORT', undefined, () => { + assert.equal(probe(A2A).ok, true, 'default local target must not fail closed'); + }), + ); + }); + + it('IPv6 loopback is admitted (Node reports hostname as "[::1]", not "::1")', () => { + const probe = createTelemetryEvidencePrereqProbe({ otelEnabled: () => true }); + withEnv('EVAL_BASE_URL', 'http://[::1]:3002', () => { + assert.equal(probe(A2A).ok, true); + }); + }); + + it('OTel state remains the only gate input for a telemetry-backed adapter', () => { + const disabled = createTelemetryEvidencePrereqProbe({ otelEnabled: () => false }); + withEnv('EVAL_BASE_URL', 'http://localhost:3002', () => { + assert.equal(disabled(A2A).ok, false, 'disabled OTel must still fail closed'); + }); + }); + }); + + /** + * Upstream review finding (zts212653/clowder-ai#1352): the notice recommended + * configuring TELEMETRY_HMAC_SALT even when the cause was an intentional + * OTEL_SDK_DISABLED=true - sending the operator down the wrong path. + */ + describe('skip notice next-action matches the detected cause', () => { + const domain = { domainId: 'eval:a2a', sourceAdapter: 'f167-runtime-eval' }; + + it('OTEL_SDK_DISABLED cause -> advises the toggle, NOT the salt', () => { + const msg = buildEvidencePrereqSkippedMessage(domain, 'OTel disabled by OTEL_SDK_DISABLED=true'); + assert.ok(msg.includes('OTEL_SDK_DISABLED'), 'must name the actual toggle'); + assert.ok( + !msg.includes('configure a non-empty `TELEMETRY_HMAC_SALT`'), + 'must NOT tell the operator to set a salt when the cause is a deliberate toggle', + ); + }); + + it('HMAC salt cause -> advises configuring the salt', () => { + const msg = buildEvidencePrereqSkippedMessage( + domain, + 'OTel disabled at boot: HMAC salt validation failed (TELEMETRY_HMAC_SALT not configured)', + ); + assert.ok(msg.includes('configure a non-empty `TELEMETRY_HMAC_SALT`')); + }); + + it('unrecognized cause -> generic advice, no misleading specifics', () => { + const msg = buildEvidencePrereqSkippedMessage(domain, 'evidence prereq probe threw: boom'); + assert.ok(msg.includes('resolve the blocker quoted above')); + assert.ok(!msg.includes('TELEMETRY_HMAC_SALT'), 'must not invent a salt diagnosis'); + assert.ok(!msg.includes('OTEL_SDK_DISABLED'), 'must not invent a toggle diagnosis'); + }); + + it('every cause keeps the stable grep header and quotes the reason', () => { + for (const reason of ['OTel disabled by OTEL_SDK_DISABLED=true', 'HMAC salt validation failed', 'boom']) { + const msg = buildEvidencePrereqSkippedMessage(domain, reason); + assert.ok(msg.includes('SKIPPED (evidence source unavailable)'), 'stable header for grep/dedup'); + assert.ok(msg.includes(`> ${reason}`), 'reason must be quoted verbatim'); + } + }); + }); + + it('bootstrap wires the telemetry init state into every scheduled eval spec', () => { + const indexSource = readFileSync(new URL('../../src/index.ts', import.meta.url), 'utf8'); + + assert.match(indexSource, /createTelemetryEvidencePrereqProbe/); + assert.match( + indexSource, + /otelEnabled:\s*\(\)\s*=>\s*telemetryHandle\.getMetricsText\s*!==\s*null/, + 'the probe must observe the actual boot-time telemetry handle, not an env proxy', + ); + assert.match( + indexSource, + /const evalScheduleOpts = \{[\s\S]*?evidencePrereqProbe,[\s\S]*?\};/, + 'shared daily, weekly, and N-day schedule options must include the evidence probe', + ); + }); + + describe('evaluateEvidencePrereq', () => { + it('probe throw → fail-closed not-ok with reason', async () => { + const result = await evaluateEvidencePrereq( + () => { + throw new Error('synthetic evidence probe failure'); + }, + { domainId: 'eval:a2a', sourceAdapter: 'f167-runtime-eval' }, + ); + assert.equal(result.ok, false); + assert.ok(result.reason.includes('synthetic evidence probe failure')); + }); + }); + + describe('daily spec integration', () => { + async function getA2aItem(spec) { + const gateResult = await spec.admission.gate(); + const item = gateResult.workItems.find((w) => w.subjectKey === 'eval:a2a'); + assert.ok(item, 'eval:a2a must be a registered daily domain'); + return item; + } + + it('probe not-ok → SKIPPED notice in domain thread, cat never invoked', async () => { + const spec = createEvalDomainDailySpec({ + harnessFeedbackRoot: repoHarnessFeedbackRoot, + defaultUserId: 'default-user', + evidencePrereqProbe: () => ({ ok: false, reason: 'OTel disabled at boot: HMAC salt validation failed' }), + }); + const item = await getA2aItem(spec); + const { deliverMock, catWakes, ctx } = mkCtx(); + + await spec.run.execute(item.signal, item.subjectKey, ctx); + + assert.equal(catWakes().length, 0, 'eval cat must NOT be woken when evidence source is down'); + assert.equal(deliverMock.mock.callCount(), 1); + const call = deliverMock.mock.calls[0].arguments[0]; + assert.equal(call.threadId, 'thread_eval_a2a', 'skip notice must stay in the domain system thread'); + assert.equal(call.userId, 'scheduler'); + assert.ok(call.content.includes('SKIPPED (evidence source unavailable)'), 'stable header for grep/dedup'); + assert.ok(call.content.includes('HMAC salt validation failed'), 'notice must carry the probe reason'); + assert.ok(call.content.includes('TELEMETRY_HMAC_SALT'), 'notice must state the actionable next step'); + }); + + it('probe ok → normal invocation proceeds', async () => { + const spec = createEvalDomainDailySpec({ + harnessFeedbackRoot: repoHarnessFeedbackRoot, + defaultUserId: 'default-user', + evidencePrereqProbe: () => ({ ok: true }), + }); + const item = await getA2aItem(spec); + const { deliverMock, catWakes, ctx } = mkCtx(); + + await spec.run.execute(item.signal, item.subjectKey, ctx); + + assert.equal(catWakes().length, 1, 'eval cat is woken when evidence source is healthy'); + assert.equal(deliverMock.mock.callCount(), 1); + assert.ok(!deliverMock.mock.calls[0].arguments[0].content.includes('SKIPPED')); + }); + + it('probe throws → fail-closed skip, no crash, no LLM call', async () => { + const spec = createEvalDomainDailySpec({ + harnessFeedbackRoot: repoHarnessFeedbackRoot, + defaultUserId: 'default-user', + evidencePrereqProbe: () => { + throw new Error('synthetic gate crash'); + }, + }); + const item = await getA2aItem(spec); + const { deliverMock, catWakes, ctx } = mkCtx(); + + await spec.run.execute(item.signal, item.subjectKey, ctx); + + assert.equal(catWakes().length, 0, 'a throwing probe must fail closed without waking the cat'); + assert.equal(deliverMock.mock.callCount(), 1); + assert.ok(deliverMock.mock.calls[0].arguments[0].content.includes('SKIPPED (evidence source unavailable)')); + }); + + it('both gates failing → evidence-source message wins (upstream-first ordering)', async () => { + const spec = createEvalDomainDailySpec({ + harnessFeedbackRoot: repoHarnessFeedbackRoot, + defaultUserId: 'default-user', + evidencePrereqProbe: () => ({ ok: false, reason: 'OTel disabled at boot' }), + publishPrereqProbe: () => false, + }); + const item = await getA2aItem(spec); + const { deliverMock, catWakes, ctx } = mkCtx(); + + await spec.run.execute(item.signal, item.subjectKey, ctx); + + assert.equal(catWakes().length, 0); + assert.equal(deliverMock.mock.callCount(), 1, 'exactly one skip notice, not two'); + const content = deliverMock.mock.calls[0].arguments[0].content; + assert.ok(content.includes('evidence source unavailable'), 'evidence gate runs before publish gate'); + assert.ok(!content.includes('publish prereq missing')); + }); + + it('evidence probe ok + publish gate still enforced → publish skip preserved', async () => { + const spec = createEvalDomainDailySpec({ + harnessFeedbackRoot: repoHarnessFeedbackRoot, + defaultUserId: 'default-user', + evidencePrereqProbe: () => ({ ok: true }), + publishPrereqProbe: () => false, + }); + const item = await getA2aItem(spec); + const { deliverMock, catWakes, ctx } = mkCtx(); + + await spec.run.execute(item.signal, item.subjectKey, ctx); + + assert.equal(catWakes().length, 0); + assert.equal(deliverMock.mock.callCount(), 1); + assert.ok(deliverMock.mock.calls[0].arguments[0].content.includes('publish prereq missing')); + }); + }); + + describe('nday spec integration', () => { + it('probe not-ok → skip notice, no trigger, no Redis last-dispatch write', async () => { + const redisSet = mock.fn(async () => 'OK'); + const redis = { get: mock.fn(async () => null), set: redisSet }; + const harnessFeedbackRoot = makeTempRoot(FIXTURE_FRICTION_3D_YAML); + const spec = createEvalDomainNDaySpec({ + harnessFeedbackRoot, + defaultUserId: 'default-user', + redis, + evidencePrereqProbe: () => ({ ok: false, reason: 'OTel disabled at boot' }), + }); + + const gateResult = await spec.admission.gate(); + assert.equal(gateResult.run, true, 'registry must contain at least one N-day domain'); + const item = gateResult.workItems[0]; + const { deliverMock, catWakes, ctx } = mkCtx(); + + await spec.run.execute(item.signal, item.subjectKey, ctx); + + assert.equal(catWakes().length, 0, 'eval cat must NOT be woken when evidence source is down'); + assert.equal(deliverMock.mock.callCount(), 1); + const call = deliverMock.mock.calls[0].arguments[0]; + assert.equal(call.threadId, item.signal.systemThreadId); + assert.ok(call.content.includes('SKIPPED (evidence source unavailable)')); + assert.equal( + redisSet.mock.callCount(), + 0, + 'skip must NOT consume the N-day window — domain retries on next daily probe', + ); + }); + }); +}); diff --git a/scripts/check-fork-only-patches.mjs b/scripts/check-fork-only-patches.mjs new file mode 100755 index 0000000000..8d9ada8abf --- /dev/null +++ b/scripts/check-fork-only-patches.mjs @@ -0,0 +1,408 @@ +#!/usr/bin/env node +/** + * Fork-only patch guard. + * + * Problem this solves: this fork carries patches that upstream main does NOT + * have. Every `develop_base` rebuild onto upstream main silently drops them. + * The F192 evidence-prerequisite gate was lost three times this way (PR #91, + * PR #137, commit 70d79a2f2) and each loss went unnoticed for days-to-weeks + * while the scheduler burned a full LLM session per daily eval fire. + * + * Maintainer direction (zts212653/clowder-ai#1352, closed 2026-08-25): + * "this is fork-specific harness logic that doesn't belong in upstream. The + * rebuild-loss issue should be solved by improving our fork's develop_base + * rebuild SOP (rebase fork-only patches), not by pushing internal code into + * the community repo." + * + * "Remembering to re-apply" is not a mechanism. This guard is: it asserts each + * registered patch is still present, so a rebuild that drops one turns + * `pnpm check` RED immediately instead of failing silently in production. + * + * Enforcement points (both required, neither sufficient alone): + * - `pnpm check` locally, wired first so it fails fast. + * - .github/workflows/fork-only-patches.yml on every push/PR to develop_base. + * The repo's other workflows trigger on `main` only, so without this the + * guard would never run automatically on the exact branch that loses the + * patches. That workflow is itself a registered fork-only patch. + * + * Fail-CLOSED: a missing/unreadable/malformed registry is a violation, not a + * pass. A guard whose own input vanished must not read as clean. + * + * Truth source: scripts/fork-only-patches.json (see docs/fork-only-patches.md). + */ + +import { execFileSync } from 'node:child_process'; +import { readFileSync, realpathSync } from 'node:fs'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const REGISTRY_REL = 'scripts/fork-only-patches.json'; + +function readRegistry(repoRoot, violations) { + let raw; + try { + raw = readFileSync(join(repoRoot, REGISTRY_REL), 'utf8'); + } catch (err) { + violations.push(`registry unreadable at ${REGISTRY_REL}: ${err.message}`); + return null; + } + let parsed; + try { + parsed = JSON.parse(raw); + } catch (err) { + violations.push('registry is not valid JSON: ' + err.message); + return null; + } + if (!parsed || !Array.isArray(parsed.patches)) { + violations.push('registry is missing a top-level "patches" array'); + return null; + } + if (parsed.patches.length === 0) { + violations.push( + 'registry declares zero patches - an empty registry cannot distinguish "nothing to protect" from "the registry was reverted"', + ); + return null; + } + return parsed; +} + +function checkPatch(repoRoot, patch, violations) { + const id = patch.id ?? ''; + + // Fail closed on a hollow entry. A patch that asserts nothing passes + // vacuously, which is indistinguishable from a rebuild having emptied it - + // the same silent-green failure this guard exists to remove. + if (!patch || typeof patch !== 'object' || typeof patch.id !== 'string' || patch.id.length === 0) { + violations.push('registry contains a patch entry without a string "id"'); + return; + } + const fileCount = (patch.requiredFiles ?? []).length; + const anchorCount = (patch.requiredAnchors ?? []).length; + if (fileCount === 0 && anchorCount === 0) { + violations.push(`[${id}] asserts nothing: needs requiredFiles and/or requiredAnchors`); + return; + } + + for (const rel of patch.requiredFiles ?? []) { + try { + readFileSync(join(repoRoot, rel), 'utf8'); + } catch { + violations.push(`[${id}] required file missing: ${rel}`); + } + } + + for (const anchor of patch.requiredAnchors ?? []) { + let content; + try { + content = readFileSync(join(repoRoot, anchor.file), 'utf8'); + } catch { + violations.push(`[${id}] anchor file missing: ${anchor.file}`); + continue; + } + if ((anchor.mustContain ?? []).length === 0 && (anchor.order ?? []).length === 0) { + violations.push(`[${id}] anchor for ${anchor.file} asserts nothing`); + continue; + } + for (const needle of anchor.mustContain ?? []) { + if (!content.includes(needle)) { + const note = anchor.note ? ` (${anchor.note})` : ''; + violations.push(`[${id}] ${anchor.file} lost anchor ${JSON.stringify(needle)}${note}`); + } + } + // Ordering anchors: substring presence cannot tell an executed gate from a + // stray import or a comment. Where sequence carries the meaning, pin it. + const order = anchor.order ?? []; + for (let i = 0; i + 1 < order.length; i++) { + const first = content.indexOf(order[i]); + const second = content.indexOf(order[i + 1]); + if (first === -1 || second === -1) { + violations.push( + `[${id}] ${anchor.file} is missing an ordered anchor: ${JSON.stringify(order[i])} -> ${JSON.stringify(order[i + 1])}`, + ); + } else if (first > second) { + violations.push( + `[${id}] ${anchor.file} has ${JSON.stringify(order[i])} AFTER ${JSON.stringify(order[i + 1])}; the gate must run first or it cannot prevent the invocation`, + ); + } + } + } +} + +/** + * @param {string} repoRoot absolute path to the repository root + * @returns {{ violations: string[], patchIds: string[] }} + */ +export function checkForkOnlyPatches(repoRoot) { + const violations = []; + const registry = readRegistry(repoRoot, violations); + if (registry) { + for (const patch of registry.patches) checkPatch(repoRoot, patch, violations); + } + return { violations, patchIds: (registry?.patches ?? []).map((p) => p.id) }; +} + +/** + * Did the rebuild that just happened drop anything? + * + * The tree check above answers "is the patch here right now" - but it reads the + * registry FROM THE TREE. A rebuild that resets the whole tree removes the + * registry and this very script along with the patches, so the guard cannot + * report on its own erasure. That is the gap PR #185 review (砚砚 P1) named: + * the registry is only useful if it survives independently. + * + * What survives `git reset --hard upstream/main` is the REMOTE-TRACKING REF. + * `origin/develop_base` still points at the pre-rebuild tip, so the previous + * tree's registry is readable straight out of git even after the working tree + * has been replaced: + * + * git show origin/develop_base:scripts/fork-only-patches.json + * + * This check reads what the PREVIOUS tree claimed to protect and verifies it + * against the CURRENT one. Run it as the last step of a rebuild, before the + * result is pushed or deployed. It stays runnable even when the rebuild deleted + * the guard, because the guard can be read from the same ref: + * + * git show origin/develop_base:scripts/check-fork-only-patches.mjs > /tmp/guard.mjs + * + * Why not assert membership of a "carrier" branch instead: `fork/optimizations` + * is an orphan snapshot (7 commits, no merge-base with upstream/main, a July + * full-tree copy). Membership of it is not a survival predicate, and demanding + * that current code be committed onto a stale unrelated history would be + * incoherent. Comparing against the previous tip is the claim that actually holds. + */ +function gitOut(repoRoot, args) { + return execFileSync('git', args, { + cwd: repoRoot, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + }); +} + +function resolveRef(repoRoot, ref) { + try { + gitOut(repoRoot, ['rev-parse', '--verify', '--quiet', `${ref}^{commit}`]); + return true; + } catch { + return false; + } +} + +/** + * @param {string} repoRoot + * @param {{ baselineRef?: string }} [options] + * @returns {{ violations: string[], baselineRef: string | null, comparedPatches: number }} + */ +export function checkRebuildSurvival(repoRoot, options = {}) { + const violations = []; + // Read the tree registry into a throwaway sink: its problems are the tree + // check's to report. Here it is only the source of the baseline ref, plus the + // signal that the guard's own input was erased. + const treeRegistry = readRegistry(repoRoot, []); + // Who named the baseline matters. An EXPLICIT baseline is a caller asserting + // "this is the pre-reset tree" - CI passes github.event.before. The registry + // DEFAULT is only a best guess for a manual run. + const explicit = options.baselineRef !== undefined; + const baselineRef = options.baselineRef ?? treeRegistry?.baselineRef ?? 'origin/develop_base'; + + if (!resolveRef(repoRoot, baselineRef)) { + violations.push( + `baseline ref "${baselineRef}" is not present in this checkout, so rebuild loss cannot be detected. ` + + `Fetch it first: git fetch origin develop_base`, + ); + return { violations, baselineRef: null, comparedPatches: 0 }; + } + + // PR #185 review (砚砚 P1): CI fetched `origin/develop_base` AFTER the push, so + // the "baseline" was the very tree being validated. A witness that is the + // accused cannot testify - and it fails silently, which is the whole disease. + // The baseline must be an immutable PRE-reset commit, so refuse to compare a + // commit with itself. + // + // PR #185 review round 4 (砚砚 P1): failing closed on the DEFAULT resolving to + // HEAD made canonical `pnpm check` red on every clean develop_base checkout - + // the guard's first step blocked every check after it, on the branch it + // exists to protect. So the rule is scoped to the caller that can actually + // mis-wire it: + // - explicit baseline == HEAD -> fail closed (witness is the accused) + // - default baseline == HEAD -> no older tree exists here, so HEAD itself + // witnesses the WORKING TREE below. An uncommitted registry or patch + // reversion is still caught; only a committed rebuild loss is out of + // reach, and that is what CI's explicit pre-reset baseline is for. + let defaultBaselineIsHead = false; + try { + const head = gitOut(repoRoot, ['rev-parse', 'HEAD^{commit}']).trim(); + const base = gitOut(repoRoot, ['rev-parse', `${baselineRef}^{commit}`]).trim(); + if (head === base) { + if (explicit) { + violations.push( + `baseline "${baselineRef}" resolves to HEAD (${head.slice(0, 9)}), the same tree being checked, ` + + `so it cannot witness a loss. Pass the pre-reset commit explicitly: --baseline-ref `, + ); + return { violations, baselineRef, comparedPatches: 0, defaultBaselineIsHead }; + } + defaultBaselineIsHead = true; + } + } catch { + violations.push(`cannot resolve HEAD or "${baselineRef}" to a commit; rebuild loss cannot be detected`); + return { violations, baselineRef, comparedPatches: 0, defaultBaselineIsHead }; + } + + let baselineRaw; + try { + baselineRaw = gitOut(repoRoot, ['show', `${baselineRef}:${REGISTRY_REL}`]); + } catch { + // The previous tree made no claim, so there is nothing it could have lost. + // This is a real absence of a prior claim, not an unverifiable one - the + // fail-closed rule applies to the latter. + return { violations, baselineRef, comparedPatches: 0, defaultBaselineIsHead }; + } + + let baseline; + try { + baseline = JSON.parse(baselineRaw); + } catch (err) { + violations.push(`baseline registry at ${baselineRef} is not valid JSON: ${err.message}`); + return { violations, baselineRef, comparedPatches: 0, defaultBaselineIsHead }; + } + if (!baseline || !Array.isArray(baseline.patches)) { + violations.push(`baseline registry at ${baselineRef} has no "patches" array`); + return { violations, baselineRef, comparedPatches: 0, defaultBaselineIsHead }; + } + + // The registry is itself a shared file, so a rebuild can REVERT it instead of + // deleting it. Both trees then look internally consistent and the guard + // confirms its own amnesia as healthy. Compare the claim SETS, not just files. + const treeIds = new Set((treeRegistry?.patches ?? []).map((p) => p?.id).filter(Boolean)); + for (const patch of baseline.patches) { + const id = patch?.id; + if (typeof id === 'string' && id.length > 0 && treeRegistry && !treeIds.has(id)) { + violations.push( + `REBUILD LOSS vs ${baselineRef} - patch "${id}" was registered in the baseline but is no longer in ` + + `${REGISTRY_REL}; the registry was reverted, not just the patch`, + ); + } + } + + const dropped = []; + for (const patch of baseline.patches) { + const id = patch?.id ?? ''; + const before = violations.length; + checkPatch(repoRoot, patch, violations); + // Re-label: these are losses relative to the previous tree, which is a + // sharper statement than "missing" - it names a regression with a culprit. + for (let i = before; i < violations.length; i++) { + violations[i] = `REBUILD LOSS vs ${baselineRef} - ${violations[i]}`; + } + if (violations.length > before) dropped.push(id); + } + + // The guard's own erasure is the case that hid the first three losses. + if (!treeRegistry) { + violations.push( + `REBUILD LOSS vs ${baselineRef} - the fork-only patch registry itself is gone from this tree; ` + + `restore ${REGISTRY_REL} and scripts/check-fork-only-patches.mjs`, + ); + } + + return { violations, baselineRef, comparedPatches: baseline.patches.length, defaultBaselineIsHead }; +} + +/** + * Where is the repo? + * + * Not "next to this script". The documented recovery for a rebuild that deleted + * the guard is to read it back out of the surviving ref and run it: + * + * git show origin/develop_base:scripts/check-fork-only-patches.mjs > /tmp/guard.mjs + * node /tmp/guard.mjs + * + * A script-relative root resolves to `/` there and checks nothing. Ask git from + * the working directory first, so the guard works from wherever it was rescued to. + */ +function resolveRepoRoot(argv) { + const flag = argv.indexOf('--repo-root'); + if (flag !== -1 && argv[flag + 1]) return resolve(argv[flag + 1]); + try { + return execFileSync('git', ['rev-parse', '--show-toplevel'], { + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + }).trim(); + } catch { + return join(dirname(fileURLToPath(import.meta.url)), '..'); + } +} + +function readFlag(argv, name) { + const at = argv.indexOf(name); + return at !== -1 && argv[at + 1] ? argv[at + 1] : undefined; +} + +function main() { + const argv = process.argv.slice(2); + const repoRoot = resolveRepoRoot(argv); + const tree = checkForkOnlyPatches(repoRoot); + // --baseline-ref names the immutable PRE-reset commit. CI must pass it + // (github.event.before / pull_request.base.sha); the registry default is only + // good enough for a manual pre-push run, where the local ref still names the + // prior tip. + // --no-baseline is for the one honest case with no prior tree at all (branch + // creation). It is explicit on purpose: skipping the survival half must be a + // stated intent, never a silent fallback when a baseline lookup failed. + const skipBaseline = argv.includes('--no-baseline'); + const survival = skipBaseline + ? { violations: [], baselineRef: null, comparedPatches: 0 } + : checkRebuildSurvival(repoRoot, { baselineRef: readFlag(argv, '--baseline-ref') }); + const seen = new Set(); + const violations = [...tree.violations, ...survival.violations].filter((v) => { + if (seen.has(v)) return false; + seen.add(v); + return true; + }); + const patchIds = tree.patchIds; + + if (violations.length > 0) { + console.error('[check-fork-only-patches] FAIL - fork-only patch(es) missing from this tree.\n'); + for (const v of violations) console.error(` - ${v}`); + console.error('\nThis almost always means a develop_base rebuild onto upstream main dropped them.'); + console.error('Re-apply the listed patches, then re-run this check.'); + console.error(`Registered fork-only patches: ${patchIds.join(', ') || '(registry unavailable)'}`); + console.error('Context: docs/fork-only-patches.md'); + process.exit(1); + } + + const baseline = skipBaseline + ? '; rebuild-survival SKIPPED (--no-baseline: no prior tree)' + : survival.defaultBaselineIsHead + ? `; default baseline ${survival.baselineRef} is HEAD - no older baseline to compare, so uncommitted ` + + `changes were checked against the committed HEAD only (a committed rebuild loss needs an explicit ` + + `pre-reset --baseline-ref, which CI passes)` + : survival.baselineRef + ? `; ${survival.comparedPatches} claimed by ${survival.baselineRef} still present` + : ''; + console.log(`[check-fork-only-patches] OK - ${patchIds.length} fork-only patch(es) present${baseline}`); +} + +/** + * Symlink-safe entrypoint detection. + * + * `fileURLToPath(import.meta.url)` is the REAL path; `process.argv[1]` is the + * path as typed. On macOS `/tmp` is a symlink to `/private/tmp`, so comparing + * them with resolve() alone returns false and main() never runs - the guard + * exits 0 having checked nothing. A guard that can silently no-op is the exact + * failure mode this file exists to remove, so compare real paths. + */ +function isDirectInvocation() { + if (!process.argv[1]) return false; + const real = (p) => { + try { + return realpathSync(p); + } catch { + return resolve(p); + } + }; + return real(process.argv[1]) === real(fileURLToPath(import.meta.url)); +} + +if (isDirectInvocation()) { + main(); +} diff --git a/scripts/check-fork-only-patches.test.mjs b/scripts/check-fork-only-patches.test.mjs new file mode 100644 index 0000000000..0964d49ad6 --- /dev/null +++ b/scripts/check-fork-only-patches.test.mjs @@ -0,0 +1,585 @@ +/** + * Tests for the fork-only patch guard. + * + * The guard's whole claim is that it turns a silent rebuild loss into a red + * check. An untested guard cannot support that claim - it would be the same + * "trust me, I verified it once" the guard exists to replace. These tests pin + * the two detection modes that matter and the fail-closed behaviour: + * + * - required file deleted (patch added a new file, rebuild dropped it) + * - anchor lost from a shared file (file survives the rebuild, patch reverted) + * - registry missing/malformed/empty (the guard's own input vanished) + * + * The last block asserts against the REAL repo: the guard's automatic + * enforcement point (its develop_base workflow) must stay registered, or the + * guard silently degrades to "someone remembers to run pnpm check". + */ + +import assert from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; +import { + copyFileSync, + mkdirSync as mkdirp, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + symlinkSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; +import test from 'node:test'; +import { fileURLToPath } from 'node:url'; + +import { checkForkOnlyPatches, checkRebuildSurvival } from './check-fork-only-patches.mjs'; + +const REPO_ROOT = join(dirname(fileURLToPath(import.meta.url)), '..'); + +const REGISTRY = { + patches: [ + { + id: 'demo-patch', + requiredFiles: ['src/added-by-fork.ts'], + requiredAnchors: [ + { + file: 'src/shared.ts', + mustContain: ['forkOnlySymbol'], + note: 'wiring a rebuild reverts silently', + }, + ], + }, + ], +}; + +function fixture(t, { registry = REGISTRY, files = {} } = {}) { + const root = mkdtempSync(join(tmpdir(), 'fork-only-patches-')); + t.after(() => rmSync(root, { recursive: true, force: true })); + + mkdirSync(join(root, 'scripts'), { recursive: true }); + mkdirSync(join(root, 'src'), { recursive: true }); + + if (registry !== null) { + writeFileSync( + join(root, 'scripts/fork-only-patches.json'), + typeof registry === 'string' ? registry : JSON.stringify(registry, null, 2), + ); + } + + const tree = { + 'src/added-by-fork.ts': 'export const addedByFork = true;\n', + 'src/shared.ts': 'export const forkOnlySymbol = 1;\n', + ...files, + }; + for (const [rel, content] of Object.entries(tree)) { + if (content === null) continue; + writeFileSync(join(root, rel), content); + } + return root; +} + +test('intact tree reports no violations', (t) => { + const { violations, patchIds } = checkForkOnlyPatches(fixture(t)); + assert.deepEqual(violations, []); + assert.deepEqual(patchIds, ['demo-patch']); +}); + +test('detects a required file dropped by a rebuild', (t) => { + const root = fixture(t, { files: { 'src/added-by-fork.ts': null } }); + const { violations } = checkForkOnlyPatches(root); + assert.equal(violations.length, 1); + assert.match(violations[0], /required file missing: src\/added-by-fork\.ts/); +}); + +test('detects a lost anchor when the shared file itself survives', (t) => { + // The silent case: existence checks read clean because the file is still + // there; only the fork's edit inside it was reverted. + const root = fixture(t, { + files: { 'src/shared.ts': 'export const somethingElse = 1;\n' }, + }); + const { violations } = checkForkOnlyPatches(root); + assert.equal(violations.length, 1); + assert.match(violations[0], /lost anchor "forkOnlySymbol"/); + assert.match(violations[0], /wiring a rebuild reverts silently/); +}); + +test('fails closed when the registry is missing', (t) => { + const { violations } = checkForkOnlyPatches(fixture(t, { registry: null })); + assert.equal(violations.length, 1); + assert.match(violations[0], /registry unreadable/); +}); + +test('fails closed when the registry is malformed', (t) => { + const { violations } = checkForkOnlyPatches(fixture(t, { registry: '{ not json' })); + assert.equal(violations.length, 1); + assert.match(violations[0], /not valid JSON/); +}); + +test('fails closed when the registry declares zero patches', (t) => { + // An emptied registry must not read as "nothing to protect" - that is + // indistinguishable from the registry itself being reverted. + const { violations } = checkForkOnlyPatches(fixture(t, { registry: { patches: [] } })); + assert.equal(violations.length, 1); + assert.match(violations[0], /zero patches/); +}); + +test('the guard is green against the real repo tree', () => { + const { violations } = checkForkOnlyPatches(REPO_ROOT); + assert.deepEqual(violations, [], `real tree has fork-only patch violations:\n${violations.join('\n')}`); +}); + +test('the guard keeps its own automatic enforcement point on develop_base', () => { + // Regression guard for the gap this test file shipped with: every other + // workflow triggers on `main` only, so a guard without this workflow never + // runs automatically on the branch that actually loses the patches. + const workflowRel = '.github/workflows/fork-only-patches.yml'; + const workflow = readFileSync(join(REPO_ROOT, workflowRel), 'utf8'); + // Strip comments: the file explains *why* paths-ignore is absent, and that + // prose must not satisfy - or trip - a check about the directive itself. + const directives = workflow + .split('\n') + .filter((line) => !line.trim().startsWith('#')) + .join('\n'); + + assert.match(directives, /push:\s*\n\s*branches: \[develop_base\]/); + assert.match(directives, /pull_request:\s*\n\s*branches: \[develop_base\]/); + assert.doesNotMatch( + directives, + /paths-ignore/, + 'a rebuild can drop a patch regardless of which paths its diff touches', + ); + + const registry = JSON.parse(readFileSync(join(REPO_ROOT, 'scripts/fork-only-patches.json'), 'utf8')); + const registeredFiles = registry.patches.flatMap((p) => p.requiredFiles ?? []); + assert.ok( + registeredFiles.includes(workflowRel), + `${workflowRel} must be a registered fork-only patch, or a rebuild drops the guard's CI trigger unnoticed`, + ); +}); + +// --------------------------------------------------------------------------- +// Rebuild fixture (PR #185 review, 砚砚 P1): a tree check reads the registry +// FROM the tree, so a rebuild that wipes the tree erases the guard together +// with the patches and nothing reports. These tests reproduce that exact +// rebuild and prove the loss is detected from the surviving remote-tracking ref. +// --------------------------------------------------------------------------- + +const BASELINE = 'refs/remotes/origin/develop_base'; + +function git(root, ...args) { + execFileSync('git', args, { cwd: root, stdio: ['ignore', 'pipe', 'pipe'] }); +} + +function gitCapture(root, ...args) { + return execFileSync('git', args, { cwd: root, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); +} + +const BASELINE_REGISTRY = { + baselineRef: BASELINE, + patches: [ + { + id: 'demo-patch', + requiredFiles: ['src/added-by-fork.ts'], + requiredAnchors: [{ file: 'src/shared.ts', mustContain: ['forkOnlySymbol'] }], + }, + ], +}; + +/** Builds a repo whose pre-rebuild tip is recorded as origin/develop_base. */ +function rebuiltRepo(t, { rebuild }) { + const root = mkdtempSync(join(tmpdir(), 'fork-only-rebuild-')); + t.after(() => rmSync(root, { recursive: true, force: true })); + mkdirSync(join(root, 'scripts'), { recursive: true }); + mkdirSync(join(root, 'src'), { recursive: true }); + + git(root, 'init', '--initial-branch=develop_base'); + git(root, 'config', 'user.email', 'guard@test.local'); + git(root, 'config', 'user.name', 'guard'); + + // Pre-rebuild develop_base: patch present, registry present, guard green. + writeFileSync(join(root, 'scripts/fork-only-patches.json'), JSON.stringify(BASELINE_REGISTRY, null, 2)); + writeFileSync(join(root, 'src/added-by-fork.ts'), 'export const addedByFork = true;\n'); + writeFileSync(join(root, 'src/shared.ts'), 'export const forkOnlySymbol = 1;\n'); + git(root, 'add', '-A'); + git(root, 'commit', '-m', 'pre-rebuild develop_base'); + + // The ref that survives `git reset --hard upstream/main`. + git(root, 'update-ref', BASELINE, gitCapture(root, 'rev-parse', 'HEAD').trim()); + + rebuild(root); + // A real rebuild produces a commit, so HEAD must advance past the baseline. + // Leaving it uncommitted would make the baseline and the tree under test the + // same commit, which the guard now (correctly) refuses to compare. + git(root, 'add', '-A'); + git(root, 'commit', '--allow-empty', '-m', 'rebuild onto upstream main'); + return root; +} + +test('a rebuild that erases the registry AND the patch is still detected', (t) => { + const root = rebuiltRepo(t, { + rebuild: (r) => { + // Upstream main has none of it - registry, guard and patch all gone. + rmSync(join(r, 'scripts/fork-only-patches.json')); + rmSync(join(r, 'src/added-by-fork.ts')); + writeFileSync(join(r, 'src/shared.ts'), 'export const upstreamOnly = 1;\n'); + }, + }); + + // The tree check alone can only say "my own input vanished". + const tree = checkForkOnlyPatches(root); + assert.ok(tree.violations.every((v) => v.includes('registry unreadable'))); + + const { violations } = checkRebuildSurvival(root, { baselineRef: BASELINE }); + assert.ok( + violations.some((v) => v.includes('REBUILD LOSS') && v.includes('src/added-by-fork.ts')), + `expected the dropped file to be named, got: ${violations.join(' | ')}`, + ); + assert.ok( + violations.some((v) => v.includes('src/shared.ts')), + 'a reverted anchor in a surviving shared file must be reported too', + ); + assert.ok( + violations.some((v) => v.includes('registry itself is gone')), + 'the guard must report its own erasure, which is what hid the first three losses', + ); +}); + +test('a rebuild that preserved everything reports no loss', (t) => { + const root = rebuiltRepo(t, { rebuild: () => {} }); + const { violations, comparedPatches } = checkRebuildSurvival(root, { baselineRef: BASELINE }); + assert.deepEqual(violations, []); + assert.equal(comparedPatches, 1); +}); + +test('an unreachable baseline fails closed with an actionable recovery', (t) => { + const root = rebuiltRepo(t, { rebuild: () => {} }); + const { violations } = checkRebuildSurvival(root, { baselineRef: 'refs/remotes/origin/nope' }); + assert.equal(violations.length, 1); + assert.match(violations[0], /cannot be detected/); + assert.match(violations[0], /git fetch origin develop_base/); +}); + +test('a baseline that never carried a registry makes no claim to lose', (t) => { + // First adoption: the prior tip predates the guard. Absence of a prior claim + // is not an unverifiable claim, so it must not be dressed up as a violation. + const root = rebuiltRepo(t, { rebuild: () => {} }); + const registry = readFileSync(join(root, 'scripts/fork-only-patches.json'), 'utf8'); + + rmSync(join(root, 'scripts/fork-only-patches.json')); + git(root, 'add', '-A'); + git(root, 'commit', '-m', 'pre-adoption tip: no registry'); + git(root, 'update-ref', 'refs/remotes/origin/empty', gitCapture(root, 'rev-parse', 'HEAD').trim()); + + // HEAD then advances past that baseline and adopts the registry. + writeFileSync(join(root, 'scripts/fork-only-patches.json'), registry); + git(root, 'add', '-A'); + git(root, 'commit', '-m', 'adopt the guard'); + + const { violations, comparedPatches } = checkRebuildSurvival(root, { + baselineRef: 'refs/remotes/origin/empty', + }); + assert.deepEqual(violations, []); + assert.equal(comparedPatches, 0); +}); + +// --- hollow entries (PR #185 review, 砚砚 P2) ------------------------------- + +test('a patch entry that asserts nothing fails closed', (t) => { + const root = fixture(t, { registry: { baselineRef: BASELINE, patches: [{ id: 'hollow' }] } }); + const { violations } = checkForkOnlyPatches(root); + assert.equal(violations.length, 1); + assert.match(violations[0], /asserts nothing/); +}); + +test('a patch entry without an id fails closed', (t) => { + const root = fixture(t, { + registry: { baselineRef: BASELINE, patches: [{ requiredFiles: ['src/added-by-fork.ts'] }] }, + }); + const { violations } = checkForkOnlyPatches(root); + assert.equal(violations.length, 1); + assert.match(violations[0], /without a string "id"/); +}); + +// --- ordering anchors (PR #185 review, 砚砚 P2) ----------------------------- + +test('ordering anchor catches a gate moved after the step it must precede', (t) => { + const root = fixture(t, { + registry: { + baselineRef: BASELINE, + patches: [ + { + id: 'ordered', + requiredAnchors: [{ file: 'src/shared.ts', order: ['gateRuns()', 'thenPublishes()'] }], + }, + ], + }, + files: { 'src/shared.ts': 'thenPublishes();\ngateRuns();\n' }, + }); + const { violations } = checkForkOnlyPatches(root); + assert.equal(violations.length, 1); + assert.match(violations[0], /AFTER/); + assert.match(violations[0], /cannot prevent the invocation/); +}); + +test('ordering anchor passes when the gate runs first', (t) => { + const root = fixture(t, { + registry: { + baselineRef: BASELINE, + patches: [ + { + id: 'ordered', + requiredAnchors: [{ file: 'src/shared.ts', order: ['gateRuns()', 'thenPublishes()'] }], + }, + ], + }, + files: { 'src/shared.ts': 'gateRuns();\nthenPublishes();\n' }, + }); + assert.deepEqual(checkForkOnlyPatches(root).violations, []); +}); + +// --------------------------------------------------------------------------- +// The guard must never silently no-op. +// +// Found while testing the documented recovery from docs/fork-only-patches.md: +// `node /tmp/guard.mjs` printed nothing and exited 0. `import.meta.url` gives +// the REAL path (/private/tmp/...) while `process.argv[1]` gives the path as +// typed (/tmp/...), so the entrypoint comparison was false and main() never +// ran. A guard that exits 0 having checked nothing is the precise failure this +// file exists to remove, so it is pinned here. +// --------------------------------------------------------------------------- + +const GUARD = join(REPO_ROOT, 'scripts/check-fork-only-patches.mjs'); + +function runGuard(args, cwd) { + try { + const stdout = execFileSync('node', args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); + return { code: 0, out: stdout }; + } catch (err) { + return { code: err.status ?? 1, out: `${err.stdout ?? ''}${err.stderr ?? ''}` }; + } +} + +test('runs when invoked through a symlinked path instead of exiting 0 in silence', (t) => { + const box = mkdtempSync(join(tmpdir(), 'guard-symlink-')); + t.after(() => rmSync(box, { recursive: true, force: true })); + mkdirp(join(box, 'real')); + copyFileSync(GUARD, join(box, 'real/guard.mjs')); + symlinkSync(join(box, 'real'), join(box, 'link')); + + const { out } = runGuard([join(box, 'link/guard.mjs')], REPO_ROOT); + assert.match(out, /\[check-fork-only-patches\]/, 'guard produced no verdict at all'); +}); + +test('rescued out of the repo, it still checks the repo it is run from', (t) => { + // The documented rebuild recovery: the tree lost the guard, so it is read + // back out of the surviving ref into /tmp and run from the repo. + const box = mkdtempSync(join(tmpdir(), 'guard-rescue-')); + t.after(() => rmSync(box, { recursive: true, force: true })); + const rescued = join(box, 'guard.mjs'); + copyFileSync(GUARD, rescued); + + const { code, out } = runGuard([rescued], REPO_ROOT); + assert.equal(code, 0, out); + assert.match(out, /fork-only patch\(es\) present/); + // A script-relative root would have resolved to "/" and found no registry. + assert.doesNotMatch(out, /registry unreadable/); +}); + +test('--repo-root targets an explicit tree', (t) => { + const root = rebuiltRepo(t, { rebuild: () => {} }); + const { code, out } = runGuard([GUARD, '--repo-root', root], tmpdir()); + assert.equal(code, 0, out); + assert.match(out, /1 fork-only patch\(es\) present/); +}); + +test('an explicit tree with no resolvable baseline still fails closed', (t) => { + // --repo-root must not become a way to opt out of the survival half. + const root = fixture(t, { registry: { baselineRef: BASELINE, patches: REGISTRY.patches } }); + const { code, out } = runGuard([GUARD, '--repo-root', root], tmpdir()); + assert.equal(code, 1); + assert.match(out, /rebuild loss cannot be detected/); +}); + +// --------------------------------------------------------------------------- +// Post-push lifecycle (PR #185 review round 2, 砚砚 P1). +// +// CI fetched `origin/develop_base` AFTER the push, so the "baseline" was the +// very tree being validated. Worse, the registry is itself a shared file: a +// rebuild can REVERT it rather than delete it, and then both trees look +// internally consistent and the guard confirms its own amnesia as healthy. +// --------------------------------------------------------------------------- + +function pushedRepo(t, { baselinePatches, afterPatches, files }) { + const root = mkdtempSync(join(tmpdir(), 'guard-postpush-')); + t.after(() => rmSync(root, { recursive: true, force: true })); + mkdirp(join(root, 'scripts')); + mkdirp(join(root, 'src')); + git(root, 'init', '--initial-branch=develop_base'); + git(root, 'config', 'user.email', 'guard@test.local'); + git(root, 'config', 'user.name', 'guard'); + + const writeRegistry = (patches) => + writeFileSync( + join(root, 'scripts/fork-only-patches.json'), + JSON.stringify({ baselineRef: BASELINE, patches }, null, 2), + ); + + writeRegistry(baselinePatches); + writeFileSync(join(root, 'src/kept.ts'), 'export const kept = 1;\n'); + writeFileSync(join(root, 'src/dropped.ts'), 'export const dropped = 1;\n'); + git(root, 'add', '-A'); + git(root, 'commit', '-m', 'pre-rebuild'); + const preResetSha = gitCapture(root, 'rev-parse', 'HEAD').trim(); + git(root, 'update-ref', BASELINE, preResetSha); + + writeRegistry(afterPatches); + for (const [rel, content] of Object.entries(files ?? {})) { + if (content === null) rmSync(join(root, rel)); + else writeFileSync(join(root, rel), content); + } + git(root, 'add', '-A'); + git(root, 'commit', '--allow-empty', '-m', 'rebuild onto upstream main'); + // The push: the remote-tracking ref advances to the tree under validation. + git(root, 'update-ref', BASELINE, gitCapture(root, 'rev-parse', 'HEAD').trim()); + return { root, preResetSha }; +} + +const KEPT = { id: 'kept-patch', requiredFiles: ['src/kept.ts'] }; +const DROPPED = { id: 'f192-gate', requiredFiles: ['src/dropped.ts'] }; + +test('a baseline that advanced to the validated tree fails closed, never green', (t) => { + const { root } = pushedRepo(t, { + baselinePatches: [KEPT, DROPPED], + afterPatches: [KEPT], + files: { 'src/dropped.ts': null }, + }); + // This is exactly what CI did: fetch origin/develop_base after the push. + const { violations } = checkRebuildSurvival(root, { baselineRef: BASELINE }); + assert.ok(violations.length > 0, 'a witness that is the accused must not return green'); + assert.match(violations[0], /resolves to HEAD/); + assert.match(violations[0], /--baseline-ref/); +}); + +test('a reverted registry is a loss, not a smaller honest claim', (t) => { + const { root, preResetSha } = pushedRepo(t, { + baselinePatches: [KEPT, DROPPED], + afterPatches: [KEPT], + files: { 'src/dropped.ts': null }, + }); + const { violations } = checkRebuildSurvival(root, { baselineRef: preResetSha }); + assert.ok( + violations.some((v) => v.includes('f192-gate') && v.includes('registry was reverted')), + `the dropped CLAIM must be named, not just the file: ${violations.join(' | ')}`, + ); + assert.ok( + violations.some((v) => v.includes('src/dropped.ts')), + 'the dropped file must still be named too', + ); +}); + +test('the CLI honours --baseline-ref instead of the registry default', (t) => { + const { root, preResetSha } = pushedRepo(t, { + baselinePatches: [KEPT, DROPPED], + afterPatches: [KEPT], + files: { 'src/dropped.ts': null }, + }); + copyFileSync(GUARD, join(root, 'scripts/check-fork-only-patches.mjs')); + + const withBaseline = runGuard( + [join(root, 'scripts/check-fork-only-patches.mjs'), '--baseline-ref', preResetSha], + root, + ); + assert.equal(withBaseline.code, 1, withBaseline.out); + assert.match(withBaseline.out, /registry was reverted/); + + // The default ref has advanced to the rebuilt tree, so it cannot see the + // committed loss. It must say so instead of claiming a verification - which + // is exactly why CI passes the immutable pre-reset ref explicitly. + const withoutBaseline = runGuard([join(root, 'scripts/check-fork-only-patches.mjs')], root); + assert.equal(withoutBaseline.code, 0, withoutBaseline.out); + assert.match(withoutBaseline.out, /no older baseline/); + assert.doesNotMatch(withoutBaseline.out, /claimed by .* still present/); +}); + +test('--no-baseline states the skip instead of hiding it', (t) => { + const { root } = pushedRepo(t, { baselinePatches: [KEPT], afterPatches: [KEPT] }); + copyFileSync(GUARD, join(root, 'scripts/check-fork-only-patches.mjs')); + const { code, out } = runGuard([join(root, 'scripts/check-fork-only-patches.mjs'), '--no-baseline'], root); + assert.equal(code, 0, out); + assert.match(out, /rebuild-survival SKIPPED/); +}); + +// --------------------------------------------------------------------------- +// Default vs explicit baseline (PR #185 review round 4, 砚砚 P1). +// +// Once merged, a clean develop_base checkout has HEAD == origin/develop_base. +// Failing closed on that made the FIRST step of canonical `pnpm check` red on +// the very branch it protects, so nothing after it ever ran. +// +// The fail-closed rule exists for the CI case: a caller who explicitly names a +// baseline that turns out to be the tree under test has mis-wired the witness. +// The registry default resolving to HEAD means something else - there is no +// older tree here - so HEAD itself becomes the witness for the WORKING TREE. +// That keeps catching an uncommitted registry or patch reversion instead of +// skipping the survival half outright. +// --------------------------------------------------------------------------- + +function tipRepo(t) { + // A clean develop_base tip: the remote-tracking default equals HEAD. + const { root } = pushedRepo(t, { baselinePatches: [KEPT, DROPPED], afterPatches: [KEPT, DROPPED] }); + assert.equal( + gitCapture(root, 'rev-parse', BASELINE).trim(), + gitCapture(root, 'rev-parse', 'HEAD').trim(), + 'fixture must model HEAD == origin/develop_base', + ); + return root; +} + +test('a clean develop_base tip passes the default invocation', (t) => { + const root = tipRepo(t); + const { code, out } = runGuard([GUARD, '--repo-root', root], tmpdir()); + assert.equal(code, 0, `canonical pnpm check would be red on its own target branch:\n${out}`); + assert.match(out, /no older baseline/); +}); + +test('an explicit --baseline-ref HEAD still fails closed', (t) => { + // The CI defence must not loosen: naming the accused as the witness is a + // mis-wired caller, whatever the registry default happens to be. + const root = tipRepo(t); + const { code, out } = runGuard([GUARD, '--repo-root', root, '--baseline-ref', 'HEAD'], tmpdir()); + assert.equal(code, 1, out); + assert.match(out, /resolves to HEAD/); +}); + +test('at the tip, HEAD witnesses an uncommitted registry reversion', (t) => { + const root = tipRepo(t); + // Uncommitted: drop a claim from the registry and delete its file. + writeFileSync( + join(root, 'scripts/fork-only-patches.json'), + JSON.stringify({ baselineRef: BASELINE, patches: [KEPT] }, null, 2), + ); + rmSync(join(root, 'src/dropped.ts')); + + const { code, out } = runGuard([GUARD, '--repo-root', root], tmpdir()); + assert.equal(code, 1, 'the default path must not skip survival - HEAD is still a witness'); + assert.match(out, /f192-gate.*registry was reverted/); + assert.match(out, /src\/dropped\.ts/); +}); + +test('the workflow keeps passing an explicit immutable baseline', () => { + // The default path is honest that it cannot see a committed rebuild loss, so + // CI must keep naming the pre-reset commit. Pin that wiring as a registered + // anchor: deleting it turns the TREE check red even if CI falls back. + const registry = JSON.parse(readFileSync(join(REPO_ROOT, 'scripts/fork-only-patches.json'), 'utf8')); + const wf = registry.patches + .flatMap((p) => p.requiredAnchors ?? []) + .find((a) => a.file === '.github/workflows/fork-only-patches.yml'); + assert.ok(wf, 'workflow must be a registered anchor file'); + for (const needle of [ + 'check-fork-only-patches.mjs --baseline-ref', + 'github.event.before', + 'github.event.pull_request.base.sha', + ]) { + assert.ok(wf.mustContain.includes(needle), `workflow anchor must pin ${needle}`); + } +}); diff --git a/scripts/fork-only-patches.json b/scripts/fork-only-patches.json new file mode 100644 index 0000000000..cc57b8f43e --- /dev/null +++ b/scripts/fork-only-patches.json @@ -0,0 +1,65 @@ +{ + "$comment": "Single source of truth for fork-only patches that upstream main does NOT carry. Consumed by scripts/check-fork-only-patches.mjs (wired into `pnpm check`). See docs/fork-only-patches.md. Every develop_base rebuild from upstream main MUST re-apply these; the guard turns a silent loss into a red check.", + "baselineRef": "origin/develop_base", + "$baselineComment": "The pre-rebuild tip, read straight out of git. `git reset --hard upstream/main` replaces the working tree but does NOT move remote-tracking refs, so this ref still holds the previous tree's registry - and this guard - after a rebuild has erased both from disk. Run the guard as the last step of a rebuild to compare what the previous tree claimed to protect against what the new one actually has. NOT fork/optimizations: that branch is an orphan July snapshot (7 commits, no merge-base with upstream/main), so membership of it is not a survival predicate.", + "patches": [ + { + "id": "f192-evidence-prereq-gate", + "owner": "opus", + "why": "Skips scheduled eval-cat invocation when the domain's evidence source cannot produce evidence (OTel disabled). Without it every daily eval:a2a fire burns a full LLM session to re-conclude a known gap.", + "upstreamStatus": "rejected-as-fork-scope (zts212653/clowder-ai#1352 closed 2026-08-25; issue #1353 accepted the direction then closed as fork-specific)", + "lostInRebuilds": ["PR #91 (2026-08-11)", "PR #137 (2026-08-28)", "commit 70d79a2f2 (2026-09-01)"], + "requiredFiles": [ + "packages/api/src/infrastructure/harness-eval/domain/eval-domain-evidence-gate.ts", + "packages/api/test/harness-eval/eval-domain-evidence-gate.test.js" + ], + "requiredAnchors": [ + { + "file": "packages/api/src/infrastructure/harness-eval/domain/eval-domain-daily.ts", + "mustContain": ["evidencePrereqProbe", "evaluateEvidencePrereq", "buildEvidencePrereqSkippedMessage"], + "note": "execute-path ordering: the evidence gate must run BEFORE publishPrereqProbe, or it cannot prevent the invocation it exists to prevent. Anchored on CALL SITES, not bare symbols - imports and interface fields would satisfy a substring check while the executed gate was gone.", + "order": ["evaluateEvidencePrereq(config.evidencePrereqProbe", "if (config.publishPrereqProbe)"] + }, + { + "file": "packages/api/src/infrastructure/harness-eval/domain/eval-domain-nday.ts", + "mustContain": ["evidencePrereqProbe", "evaluateEvidencePrereq", "buildEvidencePrereqSkippedMessage"], + "note": "execute-path ordering: the evidence gate must run BEFORE publishPrereqProbe, or it cannot prevent the invocation it exists to prevent. Anchored on CALL SITES, not bare symbols - imports and interface fields would satisfy a substring check while the executed gate was gone.", + "order": ["evaluateEvidencePrereq(opts.evidencePrereqProbe", "if (opts.publishPrereqProbe)"] + }, + { + "file": "packages/api/src/index.ts", + "mustContain": ["createTelemetryEvidencePrereqProbe", "evidencePrereqProbe,"], + "note": "bootstrap: import + probe construction from telemetryHandle + inclusion in evalScheduleOpts" + } + ] + }, + { + "id": "fork-only-patch-guard", + "owner": "opus", + "why": "The guard that detects every other entry's loss. Its develop_base workflow is the only automatic enforcement point: all other workflows in this repo trigger on `main`, and develop_base is the branch rebuilt onto upstream main. Lose this and the guard degrades to 'someone remembers to run pnpm check' - the exact failure mode it replaces.", + "upstreamStatus": "fork-scope by construction - upstream has no develop_base branch and carries none of these patches", + "requiredFiles": [ + ".github/workflows/fork-only-patches.yml", + "scripts/check-fork-only-patches.mjs", + "scripts/check-fork-only-patches.test.mjs" + ], + "requiredAnchors": [ + { + "file": ".github/workflows/fork-only-patches.yml", + "mustContain": [ + "branches: [develop_base]", + "check-fork-only-patches.mjs --baseline-ref", + "github.event.before", + "github.event.pull_request.base.sha" + ], + "note": "the trigger that makes the guard automatic, AND the explicit immutable pre-reset baseline it passes. The default baseline cannot see a committed rebuild loss, so dropping --baseline-ref from CI must turn the tree check red rather than fall back silently." + }, + { + "file": "package.json", + "mustContain": ["pnpm check:fork-only-patches", "node scripts/check-fork-only-patches.mjs"], + "note": "local enforcement: wired FIRST in `check` so a rebuild fails fast; definition + chain wiring are separate revert cases" + } + ] + } + ] +}