Skip to content

fix(natives): heal better-sqlite3 in place so npm can't prune the heal away - #27

Merged
pacphi merged 1 commit into
mainfrom
fix/natives-heal-survives-npm-reconciliation
Jul 17, 2026
Merged

pacphi merged 1 commit into
mainfrom
fix/natives-heal-survives-npm-reconciliation

Conversation

@pacphi

@pacphi pacphi commented Jul 17, 2026

Copy link
Copy Markdown
Owner

The symptom

ak sync contradicted itself in a single run:

✓ natives: …/ruflo/node_modules/agentdb: native installed
✗ still failing: [natives] 1/1 agentdb location(s) on WASM fallback (data-loss writes)

Both reports were true, ~30 seconds apart. The verifier was right; so was the installer.

Root cause

  1. The ruflo@latest upgrade leaves the shared ruflo/node_modules/better-sqlite3 half-built — sqlite3.a compiled, no .node linked.
  2. healNatives rung 1 installed better-sqlite3 into the agentdb location with --no-save. That produced a working local copy and verified true — honest at that instant.
  3. Nothing in the tree declares that copy. When healAidefence ran npm install into the ruflo root, npm reconciled the tree and pruned it as extraneous.
  4. Resolution fell back to the half-built shared copy → WASM fallback.

The same prune also removed agentic-flow/node_modules/agentdb — which is why the count read 1/1 where sync had just patched two locations. That 1/1 was the tell.

Worth noting: tests/kit/natives.test.mjs already had a test hypothesizing "an in-process npm install (aidefence) dedupes it away". That chased the resolver-cache symptom; this is the pruning root cause underneath it.

The fix

Heal the copy resolution already finds, in place, via the package's own prebuild-install || node-gyp rebuild — npm run is user-invoked and never gated by npm >=11.17 allow-scripts. Installing a copy is now the last resort, only when better-sqlite3 isn't resolvable at all. The heal lands on the declared copy, so a later reconciliation has nothing to prune.

Ordering alone would have been a half-measure: it fixes this sync, but leaves a landmine for any future npm install in that tree (ak x mcp, a later aqe install, or a user running npm by hand). Fixing the strategy makes the heal durable; the reorder is now defense in depth.

Also:

  • Reorder natives after aidefence/aqe-solver in sync. Runs on security too — an aidefence install can wipe the binding even when the plan never flagged natives.
  • Timeout 300s → 600s. A node-gyp sqlite3 compile is slow, and a half-built build/ is what a truncated one leaves behind. (Defensive — not proven to be a trigger here.)
  • Inject runner into ensureNativeBsq3 so the ladder is testable without npm or a network.

Verification

Reproduced the original broken state (half-built shared copy + aidefence absent) → identical 1/1 … WASM fallback. Then with the fix:

  • ak sync → ✓ natives: … native built in place → ✓ converged — no failing subsystems
  • Binding lands on the shared declared copy; no extraneous copy planted
  • Survives a subsequent tree-reconciling npm install — the exact operation that broke it before
  • Second ak sync is a clean no-op (idempotent)

An earlier hypothesis (ordering alone) was falsified by testing — the binding survived. It only reproduced once natives ran first to create the extraneous copy. That's what pointed at pruning rather than ordering.

Tests

tests/kit/heal-natives.test.mjs — hermetic (synthetic fixture + injected runner; no npm, no network, runs on the full CI matrix). It simulates npm's prune-the-undeclared behavior and fails on the old strategy:

✖ heal survives an npm tree reconciliation (the sync convergence regression)
  AssertionError: STILL native after reconciliation — a heal that a later
  npm install prunes away is not a heal

3 of 5 fail on the old code, all 5 pass on the new. Gates: 87 mjs + 43 cjs tests pass, eslint exit 0, typecheck exit 0.

…l away

`ak sync` reported "natives: native installed" and then failed its own
convergence proof with "1/1 agentdb location(s) on WASM fallback". Both
reports were true, 30 seconds apart.

The ruflo upgrade leaves the shared ruflo/node_modules/better-sqlite3
half-built (sqlite3.a, no .node). healNatives then installed
better-sqlite3 INTO the agentdb location with --no-save, which produced a
working local copy and verified true — honest at that instant. But nothing
in the tree declares that copy, so when healAidefence ran `npm install`
into the ruflo root, npm reconciled the tree and pruned it as extraneous.
Resolution fell back to the half-built shared copy → WASM fallback. (The
same prune also removed agentic-flow/node_modules/agentdb, which is why
the count read 1/1 where sync had just patched two locations.)

Heal the copy resolution ALREADY finds, in place, via the package's own
`prebuild-install || node-gyp rebuild` — `npm run` is user-invoked and
never gated by npm >=11.17 allow-scripts. Installing a copy is now the
last resort, only when better-sqlite3 is not resolvable at all, so the
heal lands on the declared copy and survives any later reconciliation.

Also:
- Move the natives step after aidefence/aqe-solver in sync, so nothing
  reshapes the tree after the heal. Defense in depth: with the in-place
  fix, ordering alone no longer decides correctness. Runs on `security`
  too — an aidefence install can wipe the binding even when the plan
  never flagged natives.
- Raise the build timeout 300s → 600s. A node-gyp sqlite3 compile is slow
  and a half-built build/ dir is what a truncated one leaves behind.
- Inject `runner` into ensureNativeBsq3 so the ladder is testable without
  npm or a network.

Verified by reproducing the original broken state (half-built shared copy
+ aidefence absent): same 1/1 WASM fallback, then `ak sync` converges, the
binding lands on the shared declared copy, survives a subsequent
tree-reconciling `npm install`, and a second sync is a clean no-op.

The regression test simulates npm's prune-the-undeclared behavior and
fails on the old strategy with "a heal that a later npm install prunes
away is not a heal".
@pacphi
pacphi merged commit abad428 into main Jul 17, 2026
11 checks passed
@pacphi
pacphi deleted the fix/natives-heal-survives-npm-reconciliation branch July 17, 2026 14:15
pacphi added a commit that referenced this pull request Sep 27, 2026
…live (#246)

* docs(plan): Branch 4 upstream watch live code-level plan

* feat(upstream): split lastCheckedAt from conformance-verified dates (schema 6)

The registry now records two dates. lastCheckedAt is the last state
re-read (issue states, released versions) and is the tests' clock;
lastVerifiedAt is the last date every constraint's retest was re-run.
The staleAfterDays rule keys on lastCheckedAt, a per-constraint retest
moves only that constraint's nextRetestAt, and a re-check past a retest
date reports stale evidence while the registry stays valid.

Schema 5 -> 6: lastCheckedAt is required, a date, never in the future
and never before lastVerifiedAt. Data migrated in place (lastCheckedAt
2026-09-27; lastVerifiedAt left as recorded). The watch report, its
plain-text header, the idle ledger line and the hook healing plan's
upstream block carry lastCheckedAt. No test pins a literal plan digest.

docs/UPSTREAM-WATCH.md now separates re-checking from re-verifying.

* feat(upstream-watch): read fixing pull requests and tag containment

createFetcher() gains two read-only calls. fixingChanges(id) asks the
GitHub GraphQL API what closed a thread and keeps only pull requests
merged into the repository's default branch (falling back to the
ClosedEvent's closing commit); an unmerged closing reference such as
ruflo#3373, an off-branch merge or a hand close yields no change.
contains(repo, refs, sha) compares each tag spelling with the commit:
behind or identical is contained, ahead or diverged is not, a tag
missing for every spelling is unknown, and any other failure (rate
limit, auth, 5xx, an unexpected status) throws so it surfaces as
"Could not check" rather than "not contained".

Fixtures recorded read-only on 2026-09-27: ruflo#3167/#3194/#3415 are
closed by PRs #3434/#3421/#3423, each contained in v3.46.0.

* feat(upstream-watch): confirm releases from the merged fixing change

Without a recorded minVersion, a release now counts only when its tag
contains the merged pull request (or closing commit) that fixed the
thread. collect() asks the fetcher for the fixing changes and walks
the releases after the fix, oldest first and at most five, stopping at
the first tag that contains the change or has no tag to check.

releaseState() returns released (confirmed, with the change and tag),
not released (every checked release lacks the change), or unconfirmed
(no fixing change, or no tag). The new report group "Released, fix not
confirmed" holds the unconfirmed ones; they get no dispatch and no
ledger line. The released ledger line drops candidate= and carries
pr=<n> or commit=<sha7>. A confirmation failure is "Could not check".

Release gates may carry tagPattern (containing {version}); the four
openai/codex gates use rust-v{version}. The loader now also rejects an
unknown key in a release gate, matching the schema's
additionalProperties: false.

Tests changed on purpose: the "candidate" test is replaced by the
confirmation tests; the merged-PR test (ruflo#2986) asserts
release-unconfirmed without a confirmation and released-actionable
with one; fixtureFetcher() gains fixingChanges/contains stubs.

* feat(upstream-watch): count AgentDB fixes released only when Ruflo bundles them

ak gets AgentDB through Ruflo, so an agentdb publish alone must not
read as released. A release gate may now carry bundledBy, the carrier
chain from the package ak installs down to the parent of the gated
package; the three ruvnet/agentdb gates use ["ruflo",
"@claude-flow/cli"].

fetcher.bundled(chain, name) takes npm latest of the first carrier
(the newest Ruflo, hence the newest inside the support window), walks
each manifest's dependencies then optionalDependencies, and resolves
every range to its highest published match by semver (maxVersion
accepts npm's bare string for one match and an array otherwise). A
missing dependency or a range with no match is reported, not thrown;
any other npm failure is "Could not check". collect() resolves each
chain once.

releaseState() then counts the fix released only when the carrier's
resolved version is at or after the fixed one; the released version is
the carrier's (what ak installs) and fixedVersion the package's. Live
today: ruflo 3.46.1 -> @claude-flow/cli 3.46.1 -> agentdb
3.0.0-alpha.20; agentdb#26/#27/#28 remain open and waiting.

* feat(upstream-watch): record the ledger issue #243

watchPolicy.ledger now requires issue (integer >= 1); the registry records
243, the pinned and locked "Upstream watch" issue. The routine prompt opens
that issue directly instead of searching by title.

* feat(upstream-watch): let a reviewed thread leave the reply queue

A new history event, reviewed, records that the maintainer read every
comment up to that day and none needs a reply. It clears earlier comments
from "Needs our reply" (and the reply ledger lines) the way a status change
does, never later ones, and does not re-date reopened or retire-proposed
lines. ruvnet/ruflo#3153 records it for sparkling's four comments.

* feat(upstream-watch): register every upstream thread user-facing docs cite

The citation guard now also scans README.md and every top-level docs/*.md
guide (userFacingDocs); CLI help already lives in src/. ADRs, audits, plans
and research sit in subfolders and are never scanned; three top-level
history files are exempt by name (USER_DOC_EXEMPT). ruvnet/ruflo#1234 is a
placeholder in an example command and joins SYNTHETIC.

Registers the 26 threads the docs cite: 17 open as watching (mapped to the
doc caveat they back) and 9 closed as retired. Adds the claude-code and
opencode dependency policies those threads need. New entries are appended
as text in the file's own style; no existing line moved.

* chore(upstream-watch): migrate the #213 and #240 upstream remainder into the registry

#213 now tracks ruvnet/ruflo#3196, #3446 and #3450, records our 2026-09-27
reply, and names the route/peek API request still to be filed. ruflo#3196's
adjustment no longer waits for a unified path: the two stores are
deliberate (ruflo#2786), so ak waits for a tested preservation or migration
outcome. The threads #213 cites are registered: #2786 and #3195 retired,
#3143 and #2889 watched as unmapped.

#240's adjustment names agentic-qe#574 as the remaining upstream work and
records that #719 was released in 3.14.4 on 2026-09-27; its kit file is
src/lib/aqe-readiness.mjs.

* chore(upstream-watch): map, retire or query the three stale threads

openai/codex#16045 is mapped to the connected host check, which disables
each MCP server by name because -c mcp_servers={} is still a no-op
(reproduced on codex-cli 0.157.1); the code comment now cites it.
openai/codex#16921 is retired: the same request is watched through #17827,
which carries the same adjustment; the two duplicate entries now point at
#17827 only. ruvnet/ruflo#952's adjustment records that --tools /
CLAUDE_FLOW_MCP_TOOLS (3.46.1) narrows only advertised schemas while
execution stays registered, so the permissions.deny gating stays; the
mcp.mjs comment says the same.

* docs(upstream-watch): record Branch 4 decisions and the live watch

The audit record gains a Branch 4 decisions section (B4-G1, B4-G2, B4-Q1,
B4-Q2, B4-Q3) in the decision format, with implementing commits, guarded by
a test. ADR-0041 section 7 now states schema 6's two dates, release
confirmation from the merged fixing change, AgentDB gated through Ruflo, the
wider citation guard and the ledger issue number. UPSTREAM-WATCH.md says the
routine is created after this reaches main and that #240 and #213 are
registry entries. The glossary adds Confirmed release and Reviewed thread.

* fix(upstream-watch): keep an AgentDB released line stable across Ruflo releases

The released line for a bundled fix put the newest Ruflo in version=, so
every Ruflo release produced a new line and the routine posted a repeat,
breaking the rule that an exact recorded line is never acted on twice.
version= is now the fixed agentdb version; the carrier version stays in
the report (carrierVersion and the basis).

* fix(upstream-watch): name the first release not ruled out as unconfirmed

When the first release after a fix was shown not to contain it and the
next had no tag, the unconfirmed result still named the first release.
It now names the first release whose check was unknown (or the first
never checked), and the report says "not ruled out" to match.

* fix(upstream-watch): report a failed bundle resolution as could not check

An AgentDB entry without a recorded minVersion (all three live ones)
stayed "Released, fix not confirmed" when resolving what Ruflo bundles
failed, contrary to the collect comment. A failed resolution now gives
released: null, so the entry is "Could not check" and keeps its thread
facts, as the minVersion case already did.

* refactor(upstream-watch): define the package and owner/repo patterns once

The package-name pattern lived in both the registry validator and the
fetcher, and the fetcher repeated the ledger owner/repo check inline.
Both are now exported from src/lib/hook-audit/upstream-watch.mjs,
spelled as the schema spells them, and a test holds the schema to the
same source. The older future-dates test uses the withDocument helper.

* fix(upstream-watch): start the release walk when the fixing change merged

An issue closed after the release that already shipped its fix left that
release out: the walk started at the close time, so the entry read 'no
release since the fix' or named a later version. The fixing-change query
now reads mergedAt, and both the walk and the classification start at the
first fixing pull request's merge. A closing commit keeps the close time,
which is when it landed on the default branch.

The recorded GraphQL answers were re-recorded read-only with the new query.

* fix(upstream-watch): look past the first five releases through the newest

The confirmation window never moved: when the first five releases after a
fix did not contain it, the entry stayed fixed but unreleased even after a
later release shipped it, and parallel release lines can fill those slots.
After five releases without the fix the walk now checks the newest release
(latest, not a later-published backport); when it has the fix, the releases
in between are walked oldest first, so the released version is the oldest
containing one and a newer release never changes the ledger line.

* fix(upstream-watch): keep the thread when its release confirmation fails

A failed fixingChanges or contains call set the entry's error, which
discarded the thread already read: the entry fell to 'Could not check'
only and check lost its closed, reply and acknowledged lines, which are
limited to --since and so were lost for good on a day the routine posted
other events. The failure is now recorded on the release (released: null,
the error in its basis) and the thread's facts are kept.

check --json now lists fetchErrors, and plain check writes each one to
stderr as 'Could not check <id>: <error>' so stdout stays ledger lines.

* fix(upstream-watch): name #213 and #240 our tracking issues, not issues to migrate

Both are already registry entries (relation: tracking), yet the report
grouped them under 'Tracking issues to migrate' and the guide and skill
still said tracking issues migrate here. The group is now 'Our tracking
issues', and the guide and both skill copies say what a tracking entry is.

* fix(upstream-watch): keep the partial fix agentic-qe#719 from dispatching

#719 was mapped with an adjustment, so its release (3.14.4) produced a
released line and a dispatch to remove the temporary busy rule while
agentic-qe#574 is still open and #240's own adjustment says it is not yet
verified that 3.14.4 clears FsyncFailed under a live lock. #719 is now
context only (unmapped, with a note), and #574 drives the dispatch.

* docs(upstream-watch): state that a reviewed line covers its whole UTC day

History lines carry dates only, so a reviewed line treats every comment of
that day as read, including one posted after the review. Say so, and when
to record a review so that no comment is missed.

* docs(upstream-watch): record that the watch runs on POSIX only

On Windows npm is a .cmd file that Node's execFile refuses to start
without a shell, so every npm-gated release would read 'Could not check'.
A shell is no fix: cmd.exe treats the caret in version ranges passed to
npm view as an escape. The routine runs on Linux; the guide and the script
header now say macOS and Linux only.

* docs(audit): record Branch 4's implementation status and open items

The audit record had Branch 4's decisions but no implementation status or
open items, and the program plan's Branch 4 checkboxes were all unchecked.
Add a Branch 4 status subsection in the voice of Branch 1's, open-items
bullets for what is left (routine, #617 rehearsal, #240, unposted drafts,
five untriaged stale threads, three released Ruflo items), tick the code
item in the plan and annotate the rest. Commit ids in the decisions
section are refreshed after the rebase onto main.

* docs(upstream-watch): keep the previous checked-at when a thread could not be checked
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant