smoke: fix the snapshot canary that blocked v1.8.7, and make it bidirectional

Run 589 built the base cleanly and then failed both smoke jobs 81-passed/1-failed on 'mempalace skill snapshot is current'. That canary greps a phrase from the vendored mempalace skill to detect a stale snapshot, and the phrase it pinned was 'Attribute what you file yourself' — the heading of the hand-stamping instruction that THIS release withdraws. So it fired correctly: the snapshot changed and the expectation did not. Every publish job was skipped, so nothing reached the registry and v1.8.7 was never consumed. Rather than bump the string: * the assertion is now BIDIRECTIONAL — the new phrase must be present AND the withdrawn one absent. A one-way canary only catches half the drift: it cannot notice a re-vendored stale snapshot that happens to contain the pinned phrase. Verified against v1.8.6's snapshot, which now correctly fails. * the comment records the structural limit rather than just the fix: a phrase canary can only ever detect 'older than what I remembered to pin', never 'older than skillset main'. Only a diff against the skillset repo can do that, which is now a Still-open item — it needs a CI clone credential for a private repo, i.e. a policy decision, not a code change. Changelog consolidated: the SSH sidecar multiplexing default moves from Unreleased into v1.8.7, since the retag will sit on a commit that contains it, and the v1.8.7 summary now records the failed first attempt rather than quietly presenting the second one as the whole story.
2026-08-26 08:09:09 +02:00
parent 657b1ad856
commit f645e6654f
2 changed files with 105 additions and 62 deletions
@@ -11,67 +11,6 @@ Pre-v1.0.0 tags followed the pi npm version (`v{pi_version}[letter]`).
 ---
 ## Unreleased
 ### Added
 - **The SSH sidecar now defaults to connection multiplexing, without overriding
  anyone's explicit choice.** `~/.ssh-local/config` already forced `ControlPath`
  into the writable sidecar dir, but nothing supplied `ControlMaster` for targets
  coming from the user's own bind-mounted `~/.ssh/config`. An entry that never
  mentioned it therefore opened a **fresh TCP connection per `ssh` call** — and
  an agent doing a dozen calls in a few minutes is exactly the traffic shape that
  trips fail2ban or a CGNAT flow-table cap. Observed 2026-08-25 on this fleet:
  ~12 connections to one host in 15 minutes, after which port 22 stopped
  answering while HTTPS to the same estate stayed healthy in 0.44 s (that
  asymmetry is the tell for rate-limiting rather than an outage).
  **The fix is where the block sits, not what it says.** `ssh_config` is
  first-value-wins, so position encodes intent, and the two settings need
  opposite treatment:
  | Setting | Position | Meaning | Why |
  |---|---|---|---|
  | `ControlPath` | **before** `Include ~/.ssh/config` | override | the user's value points at read-only `~/.ssh`; it cannot work here, so it must lose |
  | `ControlMaster auto` + `ControlPersist 10m` | **after** the `Include` | default | an explicit per-host `ControlMaster no` must keep winning; we only supply an opinion where the user expressed none |
  *Force what is broken, default what is merely absent.* The first draft of this
  put both in the leading block, which would have silently overridden an explicit
  `ControlMaster no` — the counterfactual is in the test below.
  Verified with `ssh -G` (the resolved-config oracle) rather than by reading the
  man page, against a fixture with one host set to `no`, one silent, one set to
  `auto`: the explicit `no` resolves to `controlmaster false` **and** still gets
  the writable `ControlPath`, the silent host resolves to `auto`, and the same
  fixture under the rejected layout flips the `no` host to `auto` — so the test
  discriminates the *position*, not merely the presence of the block. Then
  end-to-end: the real script rendered in a sandbox `HOME`, block last, `bash -n`
  clean, `shellcheck -S error` clean (the gate added in v1.8.7).
  Measured effect on the author's own config (41 host aliases): **22 were silent
  about `ControlMaster` and gain `auto` + 10 m persist; 0 are overridden**, since
  the fleet contains no explicit `no`. Worth noting *how* the one deliberate
  exception is written — `proxmox002-vpn` carries `# No ControlMaster — VPN means
  direct route, no CGNAT flow cap`, i.e. the intent is expressed as **absence
  plus a comment**, which `ssh` cannot distinguish from "no opinion". That host
  does now get multiplexing; its comment says multiplexing is *unnecessary*
  there, not harmful. Anything that must stay unmultiplexed needs a literal
  `ControlMaster no`.
  Why the ordering matters beyond this one config: `~/.ssh/config` is
  **per-machine**, differs across the fleet, and future machines' versions do not
  exist yet to be audited. A default-not-override design is correct without
  needing to inspect any of them.
  `ControlPersist` is deliberately short (10 m idle, and each new session resets
  the idle timer — long enough to collapse an agent's burst, short enough that an
  abandoned socket ages out). A per-host entry that sets its own value keeps it:
  hosts already specifying `ControlPersist 4h` still resolve to 4 h. The known
  cost of multiplexing is the **stale master** — socket present, daemon gone,
  after a suspend or network change — which makes every later `ssh` to that host
  hang; recovery is `ssh -F ~/.ssh-local/config -O exit <host>`, now documented
  in the `pi-devbox-environment` skill along with `-O check`.
 ## v1.8.7 — 2026-08-25
 Patch release, and the fastest turnaround in the series (~9 h after v1.8.6) for
@@ -93,6 +32,16 @@ time as usual. **Base rebuild is forced twice over** — `Dockerfile.base` chang
 mempalace-toolkit SHA ("otherwise a toolkit-only fix never lands") — so expect
 ~67 min, and note that either cause alone would have sufficed.
 ⚠️ **The first tag of this version did not publish.** Run 589 built the base
 fine, then **both** smoke jobs failed 81-passed/1-failed on a single assertion —
 `mempalace skill snapshot is current`, a canary pinning a phrase from the
 vendored skill. The phrase it pinned was the heading of the very instruction this
 release *withdraws*, so refreshing the snapshot without re-pinning the canary
 made it fire correctly on a healthy image. Every publish job was skipped, so
 nothing reached the registry and the version was never consumed; the tag was
 moved to include the fix below. Fixing the canary is what this release is
 *for*, in miniature: the gate was right and the expectation was stale.
 ### Added
 - **Palace writes now carry the device that made them, and diary entries say so
@@ -191,8 +140,77 @@ mempalace-toolkit SHA ("otherwise a toolkit-only fix never lands") — so expect
  does not exist would repeat the shipped-false-claim mistake corrected below).
  Expect ~67 min, as for v1.8.5/v1.8.6.
 - **The SSH sidecar now defaults to connection multiplexing, without overriding
  anyone's explicit choice.** `~/.ssh-local/config` already forced `ControlPath`
  into the writable sidecar dir, but nothing supplied `ControlMaster` for targets
  coming from the user's own bind-mounted `~/.ssh/config`. An entry that never
  mentioned it therefore opened a **fresh TCP connection per `ssh` call** — and
  an agent doing a dozen calls in a few minutes is exactly the traffic shape that
  trips fail2ban or a CGNAT flow-table cap. Observed 2026-08-25 on this fleet:
  ~12 connections to one host in 15 minutes, after which port 22 stopped
  answering while HTTPS to the same estate stayed healthy in 0.44 s (that
  asymmetry is the tell for rate-limiting rather than an outage).
  **The fix is where the block sits, not what it says.** `ssh_config` is
  first-value-wins, so position encodes intent, and the two settings need
  opposite treatment:
  | Setting | Position | Meaning | Why |
  |---|---|---|---|
  | `ControlPath` | **before** `Include ~/.ssh/config` | override | the user's value points at read-only `~/.ssh`; it cannot work here, so it must lose |
  | `ControlMaster auto` + `ControlPersist 10m` | **after** the `Include` | default | an explicit per-host `ControlMaster no` must keep winning; we only supply an opinion where the user expressed none |
  *Force what is broken, default what is merely absent.* The first draft of this
  put both in the leading block, which would have silently overridden an explicit
  `ControlMaster no` — the counterfactual is in the test below.
  Verified with `ssh -G` (the resolved-config oracle) rather than by reading the
  man page, against a fixture with one host set to `no`, one silent, one set to
  `auto`: the explicit `no` resolves to `controlmaster false` **and** still gets
  the writable `ControlPath`, the silent host resolves to `auto`, and the same
  fixture under the rejected layout flips the `no` host to `auto` — so the test
  discriminates the *position*, not merely the presence of the block. Then
  end-to-end: the real script rendered in a sandbox `HOME`, block last, `bash -n`
  clean, `shellcheck -S error` clean (the gate added in v1.8.7).
  Measured effect on the author's own config (41 host aliases): **22 were silent
  about `ControlMaster` and gain `auto` + 10 m persist; 0 are overridden**, since
  the fleet contains no explicit `no`. Worth noting *how* the one deliberate
  exception is written — `proxmox002-vpn` carries `# No ControlMaster — VPN means
  direct route, no CGNAT flow cap`, i.e. the intent is expressed as **absence
  plus a comment**, which `ssh` cannot distinguish from "no opinion". That host
  does now get multiplexing; its comment says multiplexing is *unnecessary*
  there, not harmful. Anything that must stay unmultiplexed needs a literal
  `ControlMaster no`.
  Why the ordering matters beyond this one config: `~/.ssh/config` is
  **per-machine**, differs across the fleet, and future machines' versions do not
  exist yet to be audited. A default-not-override design is correct without
  needing to inspect any of them.
  `ControlPersist` is deliberately short (10 m idle, and each new session resets
  the idle timer — long enough to collapse an agent's burst, short enough that an
  abandoned socket ages out). A per-host entry that sets its own value keeps it:
  hosts already specifying `ControlPersist 4h` still resolve to 4 h. The known
  cost of multiplexing is the **stale master** — socket present, daemon gone,
  after a suspend or network change — which makes every later `ssh` to that host
  hang; recovery is `ssh -F ~/.ssh-local/config -O exit <host>`, now documented
  in the `pi-devbox-environment` skill along with `-O check`.
 ### Fixed
 - **The vendored-snapshot canary was one-way, and pinned a phrase the same
  release deleted.** `mempalace skill snapshot is current` grepped for
  *"Attribute what you file yourself"* — the heading of the hand-stamping
  instruction withdrawn above. It therefore did its job (snapshot changed,
  expectation did not) and blocked an otherwise-green build. Two changes rather
  than a string bump: the assertion is now **bidirectional** (the new phrase must
  be present **and** the withdrawn one absent, so a re-vendored *stale* snapshot
  fails as loudly as a forgotten bump — verified by running it against v1.8.6's
  snapshot, which correctly fails), and the comment now states the structural
  limit: a phrase canary can only detect *"older than what I remembered to pin"*,
  never *"older than skillset main"*.
 - **Four build-provenance smoke assertions verified the presence of a manifest
  field name and never looked at its value.** The originals were literally:
@@ -262,6 +280,17 @@ mempalace-toolkit SHA ("otherwise a toolkit-only fix never lands") — so expect
 ### Still open
 - **Make the vendored-snapshot check automatic instead of a remembered string.**
  Tonight's failure is the third iteration of the same maintenance burden (v1.8.4:
  phrase present in both copies; v1.8.7: phrase deleted by the release that
  refreshed the snapshot). A phrase canary structurally cannot answer *"is this
  snapshot older than skillset main?"* — only a diff can. Proposed: a lint job
  that clones the skillset repo and compares
  `rootfs/usr/local/share/pi-devbox/skills/mempalace/SKILL.md` against it,
  failing with the diff when they drift. Open question first: the skillset repo is
  **private**, so this needs a CI clone credential, which is a policy decision
  rather than a code change.
 - **Provenance stops at Chroma's metadata.** The hourly reconciler on the palace
  host stamps `device`/`agent_kind` in `chroma.sqlite3`, but knowledge-graph
  triples and coordination events live in *separate* SQLite files
@@ -527,7 +527,21 @@ exec_test "mempalace skill linked (fallback)"      'test -L $HOME/.agents/skills
 # multiple harnesses", a phrase present in BOTH the stale and the fresh copy.
 # A snapshot canary must pin the NEWEST section, so update this string whenever
 # the snapshot is refreshed — that is the point of it.
-exec_test "mempalace skill snapshot is current"    'grep -q "Attribute what you file yourself" $HOME/.agents/skills/mempalace/SKILL.md && echo ok'
+#
 # v1.8.7: this fired for real, and on the release that changed the snapshot. The
 # pinned phrase was "Attribute what you file yourself", the heading of the
 # instruction telling agents to hand-stamp added_by — which that same release
 # WITHDREW (RFC 001 §7.3.2 ranks agent-side stamping worst-possible; the bridge
 # now does it). So the canary correctly reported "snapshot changed, expectation
 # did not", and blocked publication of an otherwise-green build (81 passed, 1
 # failed, twice). Two lessons kept in the assertion itself:
 #   * it is now BIDIRECTIONAL — the new phrase must be present AND the withdrawn
 #     one absent, so a re-vendored stale snapshot fails just as loudly as a
 #     forgotten bump. A one-way canary only catches half the drift.
 #   * a phrase canary can only ever detect "older than what I remembered to pin",
 #     never "older than skillset main". The real fix is a CI job diffing this
 #     file against the skillset repo — see the Unreleased changelog note.
 exec_test "mempalace skill snapshot is current"    'f=$HOME/.agents/skills/mempalace/SKILL.md; grep -q "Provenance is stamped for you" "$f" && ! grep -q "Attribute what you file yourself" "$f" && echo ok'
 # Link TARGETS, not just link existence: with no skillset mounted (as here) the
 # baked tree must be what resolves, for all three vendored skills.
 exec_test "vendored skills resolve to the baked tree (no skillset mounted)" \