fix(ci): the host-pi canary reported a refactor as drift, in the wrong job (issue #102)

The canary's first run went red against pi 0.84.1, and it was wrong twice
over. Both faults were mine, introduced with the canary itself.

The drift was not real. 0.84.1 extracted `readPiManifest` into its own
module, `core/pi-manifest.js`, without changing what it means: it still
returns null when a package has no `pi` key, `collectPackageResources` still
falls through to the convention-dir loop, and RESOURCE_FIELDS is still the
same four names. So the behaviour `host-pi.mjs` depends on is intact at
latest. What broke was a needle pinned to the old function body, which is an
incidental line rather than the behaviour it stood for. The needle now
anchors the fallthrough itself (`let hasAnyDir = false;`), which is present
in both 0.80.7 and 0.84.1 and is the thing the mirror actually relies on.
Verified by running the canary against both installs: 13 of 13 needles match
each.

A canary that reports refactors as breakage is one people stop reading, so
this is recorded in OQ-018 rather than quietly corrected. The row already
anticipated the inverse failure (needles pass, behaviour moved); this is the
same family from the other side, and the row now says to check the needle
before believing the verdict.

The second fault was the signal's shape. The step lived on
`admin-extension-canary`, so a run named "the admin extension survives
latest pi" went red for a reason with nothing to do with the admin
extension, and the admin half had in fact passed. It is now its own job with
its own name and failure protocol, which also means neither canary can hide
the other's result.

And the exit code contradicted the message. The script said "ADVANCE
WARNING, not a build break" and then exited 1, which is a build break.
Drift is now a `::warning::` on a green job; a red run means the canary
could not execute at all. The two canaries differ in kind and the script
header now says how: a red admin canary means the PUBLISHED admin may
already be broken for anyone installing it against latest pi, because it
declares a `*` peer, while drift here means a FUTURE bump needs work and
nothing shipped is broken, because the worker pins pi and
host-pi.pinned.test.mjs gates that pin as a hard failure. Failing every
unrelated PR until someone does that future work buys nothing and costs the
credibility of the check.

Also added `.github/scripts/host-pi-canary.mjs` to both path filters, which
I had missed: a change to the canary did not trigger the workflow that runs
it.

No behaviour change to the worker. OQ-019 UNCHANGED, checked.

Signed-off-by: Rob Boerman <robboerman@live.nl>

Author: edgehero

.github/scripts/host-pi-canary.mjs

@@ -13,9 +13,20 @@
  * Usage: node .github/scripts/host-pi-canary.mjs <scratchDir>
  * where <scratchDir> holds a `npm install @earendil-works/pi-coding-agent@latest`.
  *
- * Exit 0 = the mirror still holds. Exit 1 = it moved, with the file and needle named. A missing install is
- * an INFRASTRUCTURE failure and exits 2, because reading it as either verdict would be a guess (the same
- * doctrine admin-pi-canary.mjs and release.yml's npm-view step follow).
+ * EXIT CODES, and why drift does NOT fail the build:
+ *   0  the mirror holds, OR it moved and that is reported as a `::warning::` annotation.
+ *   1  the canary could not RUN (no pi installed) -- an infrastructure failure, which must be loud,
+ *      because reading a canary that never executed as either verdict would be a guess (the same doctrine
+ *      admin-pi-canary.mjs and release.yml's npm-view step follow).
+ *
+ * Drift is a warning rather than a failure, and the distinction is deliberate rather than a softening.
+ * This canary and the admin one differ in KIND. A red admin canary means the PUBLISHED admin may already
+ * be broken for anyone installing it against latest pi, because it declares a `*` peer -- a live,
+ * user-facing problem. Drift here means a FUTURE pi bump will need work: nothing shipped is broken,
+ * because the worker pins pi and `worker/test/host-pi.pinned.test.mjs` gates that pin as a hard build
+ * failure. Failing every unrelated PR until someone does that future work would train people to ignore a
+ * red check, which costs more than the signal is worth. The annotation, the weekly scheduled run and
+ * `OQ-018` are what carry it instead.
  */
 import { existsSync, readFileSync } from "node:fs";
 import { join } from "node:path";
@@ -30,7 +41,7 @@ if (!scratch) {
 const piRoot = join(scratch, "node_modules", "@earendil-works", "pi-coding-agent");
 if (!existsSync(join(piRoot, "package.json"))) {
 	console.error(`::error::no pi installed at ${piRoot} -- an infrastructure failure, not a drift verdict. Re-run before reading anything into it.`);
-	process.exit(2);
+	process.exit(1);
 }
 const version = JSON.parse(readFileSync(join(piRoot, "package.json"), "utf8")).version;
 
@@ -52,12 +63,13 @@ if (misses.length === 0) {
 	process.exit(0);
 }
 
-console.error(
-	`::error::pi ${version} moved ${misses.length} internal(s) that worker/src/host-pi.mjs mirrors. This is ADVANCE WARNING, not a build break: the pinned version is unaffected until someone bumps it. ` +
-		"Before that bump, re-verify in this order: (1) discoverHostPackages against pi's getNpmInstallPath, INCLUDING the precedence that honours the managed path before the global one; " +
+console.log(
+	`::warning::pi ${version} moved ${misses.length} internal(s) that worker/src/host-pi.mjs mirrors. ADVANCE WARNING, and the build is green on purpose: the pinned version is unaffected until someone bumps it, and worker/test/host-pi.pinned.test.mjs is the hard gate on that pin. ` +
+		"Before the next bump, re-verify in this order: (1) discoverHostPackages against pi's getNpmInstallPath, INCLUDING the precedence that honours the managed path before the global one; " +
 		"(2) isEnabledByPatterns against isEnabledByOverrides, especially '-' beats '+' beats '!' and which patterns match exactly rather than by glob; " +
 		"(3) the pi-package predicate against collectPackageResources' fallthrough to the convention dirs; (4) PINNED_PI_NEEDLES itself. " +
-		"A silently wrong mirror stages a package the operator disabled, which is the failure this feature exists to remove.",
+		"Check (4) FIRST and honestly: a needle that pinned an incidental line rather than the behaviour it stood for fires on a pure refactor, and a canary that cries wolf is one people stop reading. " +
+		"If the behaviour really did move, a silently wrong mirror stages a package the operator disabled, which is the failure this feature exists to remove.",
 );
-for (const miss of misses) console.error(`  missing -- ${miss}`);
-process.exit(1);
+for (const miss of misses) console.log(`  missing -- ${miss}`);
+process.exit(0);

.github/workflows/pi-upgrade-check.yml

@@ -27,6 +27,7 @@ on:
       - "triggers.example.json"
       - ".github/workflows/pi-upgrade-check.yml"
       - ".github/scripts/admin-pi-canary.mjs"
+      - ".github/scripts/host-pi-canary.mjs"
   pull_request:
     paths:
       - "image/**"
@@ -39,6 +40,7 @@ on:
       - "triggers.example.json"
       - ".github/workflows/pi-upgrade-check.yml"
       - ".github/scripts/admin-pi-canary.mjs"
+      - ".github/scripts/host-pi-canary.mjs"
   schedule:
     # Weekly: pi ships breaking changes between minors and its HEAD moved within 24h of this
     # project's design being written. A pin that is never exercised rots silently.
@@ -460,10 +462,46 @@ jobs:
           cp admin/dist/index.mjs "$SCRATCH/admin-bundle.mjs"
           node .github/scripts/admin-pi-canary.mjs "$SCRATCH"
 
-      - name: "HOST-PI: pi's package-install paths and enablement grammar survive latest pi"
+  host-pi-canary:
+    # `worker/src/host-pi.mjs` MIRRORS private pi internals (issue #102): where `pi install` puts a
+    # package, and whether `pi config` disabled a resource. pi exports no public answer to either, which
+    # is the residual recorded as OQ-018. `worker/test/host-pi.pinned.test.mjs` gates the PINNED version
+    # inside contract-tests as a hard failure; this job is the advance warning against `latest`, and both
+    # read one shared needle list (PINNED_PI_NEEDLES) so the gate and the canary cannot drift apart.
+    #
+    # Its OWN job rather than a step on the admin canary, deliberately. Sharing that job made a run named
+    # "the admin extension survives latest pi" go red for a reason that had nothing to do with the admin
+    # extension, which is a mislabelled signal however accurate the annotation underneath it was. Separate
+    # jobs also mean an admin failure no longer hides this result, and vice versa.
+    #
+    # Failure protocol: drift here is a `::warning::` and the job stays GREEN, because nothing shipped is
+    # broken -- the worker pins pi, and the pinned test is what fails when that pin is wrong. A RED run
+    # means the canary could not execute at all (pi did not install), which is infrastructure and says
+    # nothing either way about drift. See the script header for the full reasoning.
+    name: host-pi mirrors survive latest pi (canary)
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: actions/setup-node@v4
+        with:
+          node-version-file: ".nvmrc"
+
+      # No `npm ci`: this job reads two source files and one scratch install, and never imports the
+      # repo's own pinned pi -- installing it would be the drift-hiding mistake the admin canary's own
+      # comment warns about.
+      - name: Install pi@latest into a scratch dir (never the repo)
         run: |
-          # worker/src/host-pi.mjs MIRRORS private pi internals (issue #102): where `pi install` puts a
-          # package, and whether `pi config` disabled a resource. pi exports no public answer to either.
-          # worker/test/host-pi.pinned.test.mjs gates the PINNED version in contract-tests; this is the
-          # advance warning against latest, sharing one needle list so the two cannot drift.
-          node .github/scripts/host-pi-canary.mjs "$SCRATCH"
+          SCRATCH=$(mktemp -d)
+          echo "SCRATCH=$SCRATCH" >> "$GITHUB_ENV"
+          cd "$SCRATCH"
+          npm init -y >/dev/null
+          # Same doctrine as the admin canary: an install error is infrastructure, NOT a drift verdict.
+          if ! npm install @earendil-works/pi-coding-agent@latest --no-audit --no-fund; then
+            echo "::error::npm install @earendil-works/pi-coding-agent@latest failed -- an infrastructure failure, not a pi-drift verdict. Re-run before reading anything into it."
+            exit 1
+          fi
+          node -p "'canary pi: ' + require('$SCRATCH/node_modules/@earendil-works/pi-coding-agent/package.json').version"
+
+      - name: "pi's package-install paths and enablement grammar still read the way host-pi.mjs assumes"
+        run: node .github/scripts/host-pi-canary.mjs "$SCRATCH"

specs/open-questions.md

@@ -631,6 +631,14 @@ adversarial passes did.
 - **What would reopen it louder**: a pi release where the pinned test passes and the behaviour changed
   anyway — i.e. the needles turn out to be checking the wrong lines. That is the failure the two-distance
   split cannot catch, and the only remedy is re-deriving the mirror against the source, not the needles.
+- **Calibrated once already, on the day it was written**: the canary's first run fired against pi 0.84.1,
+  and the drift was **not** real. 0.84.1 extracted `readPiManifest` into its own module without changing
+  what it means, and a needle pinned to its old body reported a behaviour change that had not happened —
+  while the fallthrough the mirror actually depends on was intact. The needle now anchors that fallthrough
+  instead. The lesson generalises and is the reason this row exists rather than a comment: **a needle
+  pinned to an incidental line reports refactors as breakage**, and a canary that cries wolf is one people
+  stop reading, which costs more than the drift it was watching for. When this fires, check the needle
+  before believing the verdict.
 - **Related risks**: `OQ-005` (the upstream-drift row this is the same species as, and the one whose
   correction records that a sha is not a version), `OQ-011`.
 

worker/src/host-pi.mjs

@@ -55,8 +55,12 @@ export const PINNED_PI_NEEDLES = {
 		"return this.getPnpmGlobalPackagePath(source.name) ?? join(this.getGlobalNpmRoot(), source.name);",
 		'this.runNpmCommandSync(["root", "-g"]).trim()',
 		'this.runNpmCommandSync(["list", "-g", "--depth", "0", "--json"])',
-		// A package with no `pi` key is still a pi package when it ships a convention dir.
-		"return pkg.pi ?? null;",
+		// A package with no `pi` key is still a pi package when it ships a convention dir. Anchored on the
+		// FALLTHROUGH rather than on `readPiManifest`'s null return, because the fallthrough IS the behaviour
+		// the mirror depends on while the manifest read is only how you arrive at it. pi 0.84.1 extracted
+		// that read into its own module without changing what it means, and a needle on its old body cried
+		// wolf on a pure refactor -- which is how a canary teaches people to ignore it.
+		"let hasAnyDir = false;",
 		// The enablement grammar: which prefixes are overrides, and the order they resolve in.
 		'return entries.filter((pattern) => pattern.startsWith("!") || pattern.startsWith("+") || pattern.startsWith("-"));',
 		"function isEnabledByOverrides(filePath, patterns, baseDir) {",