Archive — history, not state. Kept for its reasoning and its evidence; its plan is closed.

Review queue — ARCHIVED 2026-07-25

Retired. This existed because John was asleep and decisions had to accumulate somewhere for later review. He now answers as things come up, per plans/method.md, so rulings go straight into plans/edicts.md or plans/roadmap.md and open questions get raised in conversation rather than queued.

Kept for the reasoning behind decisions already made. Everything still actionable was lifted into plans/roadmap.md §Answered before archiving — do not work from this file.

His answers to the open items, for the record:

  • D7 — after the rework, get a full green suite everywhere. The bar that matters is that the VM runs and logs come out; vm_suite must pass.
  • D8 — likely dissolves with the platform rework rather than needing its own fix.
  • D11 — the right move, and worth another pass now that recent work makes that class of hardcoded target handling even more obsolete.
  • D12 — yes to make check-arms; we need a full test of every axis.
  • D13 — fix all six pool-reader defects. For the PGO one, the counters should exist only when $pgo does. And a build needs to be able to assume an endowment: even with no actor in a cart claiming it, the runtime should be buildable such that it can provide $pgo and $hook.
  • D18 — answered by D13: assumed provisioning, not optional claims.

Decisions

D1 — Parallelism is by isolated worktree, source-merged

Agents that change code run in their own git worktree, verify green there, and I merge source files only, then reseed once and re-verify in the main branch. Two agents cannot share a worktree: the daemon is process-global per shop and every change to a boot input regenerates boot.qop, boot/*.cart and boot/content/**, so parallel reseeds would produce conflicting binaries. Read-only audit agents run in parallel freely. If you disagree: the alternative was strict sequencing, which is slower but has no merge step. Nothing about the result differs, only the throughput.

D2 — Test-deletion staging error, corrected

tests/cli/content_mode.ce’s deletion was swept into a worklist commit by my git commit without a pathspec while an agent had it staged. I reset the commit and moved the deletion into the shop-modes commit where it belongs. Branch history is now clean. Look at: 0072b52e.

D3 — Worklist K3 named a mechanism that does not exist

K3 said to keep profile.powers.native_load and drop the mode gate. That field is docs-only — it appears in docs/spec/artifact-formats.md and nowhere in code. The real mechanism was already in force and is the ruling’s own argument: pit-shop’s manifest swaps native_build per platform, and native_build_none refuses. I did not invent the powers field. Note: that manifest is itself a target-name table, flagged by the L audit as a missing development-powers axis. Correct for now, but it is the same pattern one level down.

D4 — SUPERSEDED IN PART by the delete-don’t-fall-back edict

The decision below — take fresh opcode slots rather than reusing MACH_RESERVED_OP0/OP1 — stands, and its reasoning about PIT_MACH_ABI being owed to the import opcodes rather than the pool format is unaffected.

But its premise is now weaker than it needs to be. I preserved slots 43/44 because committed artifacts might contain them. Under plans/edicts.md §1 there is no compatibility to keep: those artifacts are regenerated on every seed, nothing outside this repo runs this code, and the handlers’ own comment says they exist for “legacy boot compatibility”. So the right end state is delete slots 43/44 outright, which frees them and removes two dead VM cases, rather than routing around them.

Taking fresh slots is still correct for the new opcodes — an opcode’s numeric value should not be a recycled one — but the legacy pair should go, and that is now a work item rather than a constraint to respect.

D4 (original) — New opcodes take FRESH slots, not the “reserved” ones

This is the most consequential call of the session and it changes the arc.

The planning pass found that PIT_MACH_ABI is bumped to mach-v3+pool-import1 because of two import opcodes, not because of the pool format — Pit_MachPoolOpen takes the expected ABI hash as a caller-supplied parameter and never reads the constant (9466791a:source/mach.c:412-415). So the image layer does not need the stamp bump at all. That is correct and it is the key insight.

It then proposed taking opcode slots 43/44 (MACH_RESERVED_OP0/OP1), reasoning that the green toolchain never emits them. I checked, and the premise is wrong. Those slots are live legacy handlers, and source/mach.c:5719,5730 says why:

/* Legacy boot compatibility: former delete-by-constant-key opcode. */

They implement property-delete and exist precisely because committed artifacts from an older toolchain may still contain them. The emitter not producing them today proves nothing about blobs already on disk. Reusing those slots would silently reinterpret old bytecode as a module import — the worst failure mode available: no error, wrong behaviour.

Decision: take fresh slots above the enum instead. The opcode field is 8 bits (MACH_GET_OP(i) = (i) & 0xFF, source/mach.c:16) and roughly 145 of 256 slots are used, so there are ~111 free. New opcodes at the top cannot collide with anything ever emitted, because the enum never had those values.

Consequences, and why this is better than the conditional plan:

  • No scan required. The proposed Experiment 0 (dump every committed artifact hunting for opcodes 43/44) is unnecessary. Its result would have gated the whole arc on a negative that is hard to prove and easy to get wrong.
  • No conditional flag day. The stamp stays mach-v2+fuse1 through the entire image arc regardless of what is on disk.
  • The bump moves to where it belongs. Adding an opcode is backward-compatible for reading old blobs but not forward-compatible for old binaries reading new ones, so the stamp should bump when the emitter starts producing the new opcodes — not when the handler is added. By then M1 (cart carries engine mcode, C serializes at mount) should have landed, which makes the bump a non-event forever.

If you disagree: reusing 43/44 saves two enum slots out of 111 and costs a silent miscompile risk on artifacts we cannot fully enumerate. I do not think that trade is close. The legacy handlers should eventually be deleted, but that is a separate decision with its own evidence requirement, and it should not be smuggled in as a side effect of this arc.

D5 — CORRECTED. Record-shape interning is not kept — the image layer deletes it.

My first answer here was wrong and John caught it. I wrote that shape interning should stay because it is “the constant pool doing its job”. The right answer is that the runtime shape table is a pre-image mechanism and the image work is precisely what removes it.

The design already says so. 9466791a gives shapes two kinds:

PIT_RECORD_SHAPE_IMAGE    "Direct-image descriptor. `location` is the signed byte
                           distance to its first immutable PitMachPoolShapeKeyRow."
PIT_RECORD_SHAPE_DYNAMIC  "Legacy decoded-Mach/native descriptors ... remain
                           transition-only until those execution lanes are gone."

plus _Static_assert(sizeof(PitRecordShape) == sizeof(PitMachPoolShapeRow)) — the runtime prefix and the pool row are the same layout, so an image shape row is a PitRecordShape and a record points straight into the pool’s SHAPES section. No copy, no table, no interning. The kind that needs the table is labelled transition-only by its own author.

Where I went wrong: I claimed deleting the table loses dense records. That is true only today, and for an uninteresting reason — on this base a mach blob is parsed into runtime memory, so there is no mmappable pool for a record to point into, and deleting the table leaves shapes nowhere to live but per-record copies. Density comes from records sharing one descriptor; the table is not what provides sharing, it is only where the descriptor currently sits.

So the sequencing is: shapes move into the image, then the table dies. That is roadmap phase A1, not a standing exception. Nothing about it is a judgment call any more.

Kept below for the record, since the assessment of what it does is still accurate — only the conclusion was wrong.

D5 (superseded reasoning) — why it looked like it should stay

Your ruling named text interning. Record shapes are a different mechanism sharing a similar silhouette, so I had it assessed rather than deleted, and I am keeping it.

The case for keeping it, which I find convincing:

  • It is not a cache over runtime-created values. pit_record_shape_intern (source/runtime.c:1034) is populated at code load time from shapes the compiler already emitted into an image’s constant pool, and it rejects actor-heap texts — keys must already be constant-pool texts or the call returns NULL. Its hit rate is high by construction, not by accident.
  • Its size is bounded by a build-time quantity (distinct declared shapes across loaded code), not by how long the program runs. That was the actual defect in text interning: an unbounded process-wide table accumulating runtime garbage.
  • It costs the collectors nothing comparable. The GC only reads shapes to size and scan records; there is no weak sweep like the one text interning forced into both collector profiles.
  • Deleting it is expensive and not obviously in the spirit of anything: records lose the dense representation, nan32 needs an ABI change (PitRecord carries shape_ref under a _Static_assert(sizeof(PitRecord) == 24)), and it is the substrate for MACH_FUSE_LOADF/STOREF field fusion and the PGO record-field hints.

So: shapes live in the image and are shared once on load. That is precisely the arrangement your ruling said to rely on — “99% of text and record shapes can simply be in the stone pool of a mach image” — rather than the thing it said to remove.

If you disagree: the honest counter is that it is still process-wide mutable state behind a lock, immortal, never freed. If “no shared mutable structure” is meant absolutely rather than as a statement about unbounded runtime caches, this goes too — but that is a much larger change and I did not think it was what you meant.

D6 — Text interning deleted; two subtleties worth checking

Net −603 lines across 16 files, PitContext 16 bytes smaller, and the real saving is the per-actor 2 KiB minimum table plus its per-collect rebuild and sweep in both collectors. Language suite unchanged at 0.04s; fuzzer timings overlap before and after.

Two places this could have gone silently wrong, both handled — verify these first if anything looks off later:

  1. pit_key_probe_ready changed from “is the interned instance” (objhdr_i) to “is a non-empty text”. Safe because the gate never fed a pointer comparison — it hands the key to rec_find_slot, whose hash and equality are content-based, and the in-tree comment said outright that the gate existed for allocation avoidance rather than correctness. More keys take the fast path now, not fewer. I read this hunk myself.
  2. The empty-text fold. pit_key_hash gives the immediate empty key a fixed hash of 1 but hashes a heap text by content. Interning used to make that unreachable by folding every zero-length text to PIT_KEY_empty. That fold is preserved in pit_key_from_string and empty texts are excluded from the probe gate — otherwise an empty heap text and PIT_KEY_empty would land in different record slots.

pit_key_equal still opens with if (a == b) return TRUE, which used to be an almost-certain hit and is now a genuine same-object check falling through to a length + memcmp. That is the one place the deletion costs time rather than saving it, and it was already correct.

D7 — make check-web is RED at baseline on this branch (pre-existing, not ours)

The platform-leaks agent found make check-web fails its boot smoke:

pit-shop/shop_actor: stack overflow — depth 1024, limit 1024
fleet-booted: false  eval-ran: false  => FAIL

It stashed its changes and re-ran to confirm the failure is identical at baseline, so nothing in this session caused it. The wasm target builds fine; it is the fleet boot under node that overflows. This is a real defect on the green base that predates the whole arc, and it means make check-all cannot currently be green. I have not chased it — it is out of scope for the worklist and deserves its own investigation.

Also: emcc needs EMSDK_PYTHON=/opt/homebrew/bin/python3.14 or it dies on Xcode’s python 3.9.

D8 — The macOS .app sniff is load-bearing; kept behind a provider predicate

The audit suggested host.c’s strstr(binary, ".app/Contents/MacOS/") might be dead because PIT_APP already gives a portable path to the same answer. Nothing in the tree sets PIT_APP — host.c:388 is its only reference, and internal/bundle.ce writes no such env into the .app it builds. tests/cli/app_bundle.ce launches Contents/MacOS/HelloApp bare and asserts the Application Support shop and the absence of a development universe, both gated on app_mode. Deleting the sniff would have broken it.

So it moved rather than died: sys_is_app_bundle() in source/sys_os.h, implemented in platform/posix/ under __APPLE__ and in platform/windows/ returning 0, and deliberately absent from the embedded providers since host.c is outside their manifests. The agent checked before building the hook, which is the right order.

If you disagree: the cleaner end state is for the .app builder to write PIT_APP and for the sniff to go entirely. That is a change to internal/bundle.ce plus a test update, and it was outside this task’s scope.

D9 — One platform “leak” was a no-op and got deleted rather than hooked

internal/vm.c’s #ifdef PIT_PLATFORM_PLAYDATE called pit_playdate_seed_memory_stats(), which writes 0,0,0 unconditionally, into a struct pit_runtime_memory_snapshot had already memset to zero. The call did nothing. Deleted it and the now-callerless provider rather than building a hook to preserve it — prefer deletion to abstraction. The three platform_seed_* fields stay, since the JSON report and $vm.memory still emit them, now with a comment saying no provider writes them.

Bonus deletion in the same pass: platform/ps1/ carried a fake pipe() documented as existing only for crash.c. With the sys_pipe shim in place, both it and its header declaration are gone.

D10 — RESOLVED by John: use the module-arm mechanism, not a branch or a fact

John’s ruling: there should be one generic build-time mechanism for target-varying things, and a constant like “where the seed is” should use it. It already exists — package.json modules arms swap modules per target (file_provider → file_darwin/file_playdate/…). The seed path becomes a module with per-target arms that returns it.

That is strictly better than either option I was weighing: no branch in shop_actor, no startup-fact plumbing, and no C edit at all, so the thing I declined to do blind disappears rather than waiting. Folded into roadmap phase F.

Original entry kept below for context.

D10 (superseded) — the playdate content path as a startup fact

shop_actor.ce branched on facts.system == "playdate" → "seed.qop" and == "emscripten" → "/seed.qop". The emscripten arm was already dead and is deleted: platform/web/main_web.c:21 already does setenv("PIT_SEED", "/seed.qop", 0), and select_content_path() consults that before reaching the branch. The fact was already arriving from the platform; only the redundant deduction is gone.

The playdate arm is still there, and this is the one item in the whole session I left knowingly unfinished. The fix is a single line in platform/playdate/main_playdate.c: push --content seed.qop into startup_argv, exactly as ps1/n64/psp/dc already push --cart <path>. Playdate cannot use the other lane — it has no environment at all (platform/playdate/os_playdate.c:183-188 returns PIT_NULL from os.getenv unconditionally and says so).

Why I did not just do it: I cannot build for Playdate. It needs the Playdate SDK, and nothing in make check or make check-all compiles that target, so a mistake would not surface here — it would surface on your device. Handing you a documented one-liner seemed better than an unverifiable edit to a target I cannot exercise. A comment at the site names the exact fix.

Related, worth fixing when you do: platform/playdate/main_playdate.c:180 already documents the wrong contract — “The root selects seed.qop from the target facts” — and the facts record has no such field.

D11 — Deleted a derivation rather than adding a lookup table

target_catalog.cm had two halves of one violation: a prefix parse (starts_with(value, "macos_"/"ios_"/…)) and a closed system list (darwin || linux || windows || …). Both are gone. selector_for_target now does only clean_selector(record), {} for empty, and {target: value} for a bare string — a name meaning itself, the one honest reading.

The four callers hand over the structured fact instead, which is correct because --target is documented as a system name everywhere it appears (internal/cement.ce:40 “Target system for the locator arms”; internal/bundle.ce:54 “Packaging target system”) and every in-tree use is a system.

Best moment of the change: pit-shop/shop_source.cm:709-711’s derivation was deleted outright, because select_dependency_arm already matches dep[effective_target] three lines above — once the selector carries {system: …} the second pass is provably the same lookup. Two lines removed, nothing added.

Deliberate behavioural delta: --target n64|ps1|psp|dc used to fall off the end of the incomplete closed list and yield {target: "n64"}; it now yields {system: "n64"}. Both match at run time because those targets build with PIT_TARGET_NAME == PIT_TARGET_SYSTEM, and it makes the console carts consistent with playdate, which already got a system-keyed arm. Every other target is byte-identical.

pit-linker/toolchains.cm was deliberately NOT wired in as the lookup source: target_catalog loads in the root shop on every platform, and pit-shop/package.json firewalls the toolchain table off non-desktop systems, so importing it would drag hard-coded Xcode SDK paths into every cart.

Follow-up noticed: arch_for_named_target is now unreferenced in-tree — selector_for_target was its only caller. Worth deleting.

D12 — make check does not build the kim8 text arm, and I broke it

The interning deletion removed pit_alloc_string_bytes from source/pit_text_kim.c while leaving nine call sites. The kim8 profile had not compiled since. It passed every gate because meson’s text option selects exactly one text-body file and nothing in make check configures kim8 — the utf32 arm compiles pit_text_utf32.c and never sees the error.

Caught only because the pool-reader agent happened to build both arms. Restored the function verbatim with a comment saying what it is for, so a future deletion pass does not mistake it for intern machinery. Verified by configuring a kim8 build: 178 targets link, 14/14 meson tests pass.

The general lesson, which matters beyond this bug: the default gate covers one point in a multi-axis profile space. text=kim8, gc=compact, value=32, and the console reps are all real configurations that make check never compiles. Any change to a profile-conditional file can break an arm silently. Worth a cheap make check-arms that does a configure-and-build (not test) sweep of the axes — that would have caught this in seconds.

D13 — Six defects in the ported pool reader, knowingly left in place

The pool reader is a faithful port of code that was never built, so I had it ported as-is rather than fixed in flight, and the findings recorded instead. None are reachable today — nothing calls the reader. Fix them when the reader gets its first caller:

  1. An unreadable prior shape key reports as “duplicate pool shape key” — two distinct failures share one message.
  2. Arity bound is off by one: fn.arity > 256 accepts 256, but Mach A/B/C register fields are 8 bits, so the bound should be 255.
  3. disruption_pc == 0 doubles as the no-handler sentinel, so a handler genuinely starting at pc 0 is inexpressible. Related: entry_pc/disruption_pc are uint16_t against a uint32_t instruction count.
  4. Validation is quadratic — the stone-text scan re-derives a fash64 per value, and the shape-key duplicate check is O(n²) full-text compares. Bounded (keys ≤ 255) but it is paid at open, which matters if opening lands on the boot path.
  5. A required section with length == 0 satisfies the presence check — the offset and alignment checks are both guarded by if (length), so a pool declaring STONE at offset 0 length 0 passes. Not exploitable; every stone ref then fails its range check.
  6. PitMachPoolPGOSiteRow is declared but unused — the PGO loop hand-decodes bytes, so there is no accessor to match the other eight sections.

One deliberate deviation from the original: it validated a shape kind against an enum from the record-shape refactor, which is not in this increment and whose struct differs on this branch. Rather than squat on that name with a partial definition, the pool-scoped constant carries the identical value "IMG1". When the refactor lands, tie them with a _Static_assert rather than renaming.

D14 — make budget is RED at baseline, like check-web. Second stale gate.

20 violations, and identical on the clean tree — verified by stashing, rebuilding and re-running. The violating metrics are pit-compiler output counts (slots, raw_instrs, final_instrs for tokenize/fold/parse/mcode/streamline) that no C-side change can influence, so tools/compiler_budget_baseline.json is simply stale on this branch.

That is now two gates red before we started: make check-web (D7) and make budget. Neither is caused by this work and neither is fixed by it. Worth a dedicated pass to re-baseline budget and diagnose the web stack overflow, because between them make check-all cannot be green, which removes the strongest single signal available.

D15 — Delete the dead opcode handlers, but DO NOT renumber. No ABI bump.

The compatibility sweep found two dead opcode families — MACH_RESERVED_OP0/OP1 (slots 43/44, a former delete-by-key opcode) and MACH_RESERVED_LOOKUP0/1/2 (29/30/31). Nothing in the toolchain can emit any of them: internal/mach_emit.cm jumps OP_GE = 28 → OP_GETUP = 32 and OP_POP = 42 → OP_REGEXP = 45, so the slots are unnameable. ~54 lines of enum entries, name/kind/dispatch table rows, and VM handlers.

The sweep then claimed an ABI bump is free because make smoke reseeds automatically on a PIT_MACH_ABI change. That is wrong, and it is the same trap that cost four agents a session. seed: depends on all (Makefile:188), so the binary is rebuilt first; the new binary cannot read the v2 artifacts it needs in order to regenerate them. The stamp does fold PIT_MACH_ABI, so drift is detected — but detection is not regeneration, and regeneration needs a booting daemon. Bumping the ABI is still a flag day, gated on M1.

Decision: delete the handlers and table rows; leave the enum slots as holes. No renumbering, so no ABI bump, so no flag day, and the dead code still goes. Renumbering would recover five slots out of roughly 111 free — worthless on its own terms and not worth coupling this cleanup to M1.

If you disagree: the tidiness argument for a dense enum is real but small, and it can be done for free later, after M1 makes stamp changes routine.

D16 — Rulings on the endowment names (John, direct)

$hook, not $trace, for the call/return hook. $trace is already the live distributed trace-id endowment ($trace(), $trace("with", id, fn)); renaming the newcomer costs nothing and renaming the incumbent would touch working code.

$runtime comes back, meaning “set a fact in the runtime.” set_logger_actor, set_courier_actor, set_runtime_info and shutdown — held chiefly by boot or an early actor wiring the machine up. That is one coherent idea, and it reuses the name for something real rather than the grab-bag the old single $runtime was.

ref stays in $inspect. Actor tokens have three sources — creation, delivery, and an $inspect snapshot — and ref sets nothing, so it does not fit $runtime’s definition. $inspect already implies token access because snapshot enumerates every id; granting that while withholding ref would be theatre. The coherent reading, which docs/architecture/security.md already states: $inspect is how you name an actor you were not handed.

D17 — The $inspect work is NOT committed: a real regression

Backed out at 26791a64 rather than landed. make check reported a baseline failure that looked like gate flakiness and is not:

- heap: disrupted
passed: 1952   failed: 1

internal/heap.ce is one of the files that change touched — its dead use('actor') import was deleted. The failure is intermittent, which is why a bare ./pit test showed 1953/0 while the gated run showed 1952/1. That intermittency is the interesting part and wants understanding before the change lands; an ordering or lifetime dependency is the likely shape.

The work is preserved (patch plus internal/inspect.c and internal/pgo.c in the session scratchpad, and the agent’s worktree holds a complete state). Everything else about it was sound: the token-forgery hole is genuinely closed — use('actor') exports 48 names with snapshot, count, ref, desc and actorsym all absent — and dropping actorsym was the load-bearing part, since removing ref alone would have left any importer able to build {[actorsym]: {id}} by hand.

Also learned: landing it needs a three-phase crossing, because the committed cart is cemented against the old actor module, so a binary with the surface removed cannot boot the cart that regenerates the cart. Same shape as the ABI problem, and the same fix applies.

D18 — OPEN: there is no optional endowment claim

The root shop now requires $inspect, because it hands out actor tokens. A build without that provider cannot start the shop at all. What is missing is “grant if present, otherwise the feature is absent” — without it, any component offering introspection as one service among many must hard-require it or forgo it entirely. Worth deciding whether that mechanism should exist before more surfaces move behind endowments.

Source: plans/archive/review-queue-2026-07-25.md