Phase 1 / Core / ECS access enforcement - #78
Merged
Merged
Conversation
`registerSystem`'s conflict pass refuses two writers of ONE id and nothing more, so two systems whose declarations CROSS — `Reads(T), Writes(U)` against `Writes(T), Reads(U)` — both registered and closed a two-node DAG cycle. `computeLevels` then reported it at the FIRST dispatch as `error.WriteWriteConflict`, permanently for that phase, under a name `RegistrationError` documents as something else, with the offending descriptor already committed. Pre-existing and byte-identical at the merge base, so not a regression — but this branch makes it reachable for Forge, whose `step_spec` now declares a read where it declared none. `RegistrationError` gains `DependencyCycle`. `registerSystem` gains a reachability walk from the new system's successors over the existing edges, asking whether any reaches one of its predecessors, placed ahead of the commit so the refusal touches nothing of the scheduler. The two codes stay distinct: a write-write conflict is a property of ONE component, a cycle a property of a SET and of no single member. Two colours suffice where `registry.zig`'s `@requires` closure needs three — the question is REACHABILITY in a graph already known acyclic, not cycle-finding. `computeLevels` KEEPS its branch rather than gaining an `unreachable`: the invariant is held by a sibling function and does not survive a registration that failed on `OutOfMemory`. Counter-factual, the pass removed: `2263/2284 tests passed (2 failed)`, both reading `expected error.DependencyCycle, found void`, with the non-vacuity test — a crossing declaration closing NO cycle — staying green. Four corners green at 2265/2284. Floor 2281 → 2284 / 2282, re-derived from the suite. Two enumerations swept with it, neither false and both short: `root.zig` listed three shapes broken by the protocol bump (four — the new variant breaks an exhaustive `switch`), and `README.md` listed one registration refusal (two).
Three corrections from Guy's closing review, plus the journal of the DAG cycle fix. The Recorded deviation read 55 sites across 11 files; the like-for-like figure against what the FROZEN SECTION actually names — `tests/` and `bench/` — is 52 across 9. The 55 reached that number by adding the `src/` sites, counted at the branch HEAD where three of the five in `scheduler.zig` are inline tests THIS BRANCH added: a migration count that counts what the migration created measures the wrong thing, and mixing a second perimeter into a refutation blunts it. The frozen text is untouched; it stays a Claude.ai round-trip. `FrameContext.user` was called "the path every system in the tree actually uses to reach its query". Measured: zero readers under `src/`, and the eight files that read it are all under `tests/` and `bench/`. True of the test corpus, false of production, and the difference is the whole weight of the residual. The Tier 1 seam takes `anytype` and that was unsaid. Eight functions accept either a view or a `*World` under one spelling, which is what lets one body serve both regimes — and is also the price: nothing at the call site says which arrived. The enforcement holds because the ONE registered caller passes `ctx.view`, upstream of the seam and not at it. And one correction of my own, found before the review returned it: the contract said the refusals run "before the first mutation". False — `access.resolve` calls `ensureComponentRegistered`, so the WORLD is mutated before either check. It is benign, being idempotent and monotone, and it is now stated rather than denied.
Five independent lenses over the cycle fix raised 27 findings; 20 survived a verifier told to refuse anything the source did not confirm, and they deduplicate to seven classes. Every one is mine. `computeLevels`' branch carried TWO reasons for staying an error rather than an `unreachable`, and the second was FALSE: it claimed the branch covers a scheduler half-mutated by a failed registration. It does not — such a scheduler holds an edge naming an index the rollback popped, and the in-degree count twenty lines above faults on it, `in_degree[target] += 1` with `target == n`, before one level is built. The scenario cannot reach the line it justified. Deleted; the first reason was always sufficient alone. The walk's TRANSITIVITY was unpinned — its whole reason for existing. In both the two-node and three-node cases the predecessor sits ONE hop from a seed, so a walk that checks its seeds, expands one level and stops passes them. Measured: with the walk replaced by that bounded form, `310/312 steps succeeded; 2266/2286 tests passed (1 failed)` — exactly one red, the four-node ring added for it, every other test green. A diamond joins it, the only shape in the file that reaches a node twice, written for termination and the answer and NOT for necessity. `RegistrationError` is not the set `registerSystem` returns: the resolver is `anyerror!ComponentId`, so the inferred set is `anyerror` and a caller annotating against the alias does not compile. Pre-existing; both the declaration and the re-export now say what it IS for. `forge/sync.zig`'s preflight enumerated two deterministic failures where there are three, and the third is reachable: `wouldConflict` compares write against write only, and any later `fixed_update` system reading `Transform` and writing `RigidBody` closes a cycle with `step_spec`. The omission is argued, not papered — predicting it means a second copy of the scheduler's edge construction inside a Tier 1 module. Two contracts were wider than what holds. "A scheduler that has seen an allocation failure here is unusable" reasoned about the commit and was scoped to the function, false for every pre-commit allocation. "NOTHING WAS COMMITTED" promised four things and asserted a count of descriptors; it now walks edges and both tracker maps. The `topologicalLevels` check beside it was deleted rather than strengthened — dominated by the assertion above it, and its comment named a failure it could not see. Floor 2284 -> 2286 / 2284, re-derived from the suite. Four corners green.
Two of the three findings a Codex review refused the closure on. Both are one class — a guarantee affirmed in a comment that the code does not give. P1-2. `View.world_erased` was `*anyopaque`, the same type for every declared set, so `View(&write_spec).fromErased(ctx.view.world_erased)` promoted a read declaration to a write one with no cast, no builtin and no diagnostic. The file header claimed the escape cost "an explicit `@ptrCast(@aligncast)` at the call site — a deliberate and greppable act", which was true of recovering a `*World` and false of the promotion, which is the bypass that is actually useful. Closed at the type. `ErasedFor(spec)` is an opaque generated per declared set; the field and `fromErased`'s parameter are spec-typed; the one narrowing left is generated inside the trampoline, where the spec is known. The design rests on comptime memoisation keying on the slice's IDENTITY — one `const spec` names one type wherever it appears, two separate declarations of an identical set name two — which was measured before the design was written and is now pinned in the suite rather than left in a throwaway program. Counter-proof both ways, as required. `case_view_promotion.zig` must NOT compile and fires on the compiler's own type error, not the view's marker: a marker would mean the second view had been built and was objecting afterwards. A positive test builds a view the way the trampoline does and reaches what it declared, because a `fromErased` that accepted nothing would satisfy the refusal perfectly. P2. `WELD_API_VERSION_MAJOR` sat at 0 under a doc comment prescribing the increment on every binary break, while `query_create` went from five parameters to seven and `WeldQueryChunk` changed layout. Same C0.5 rule that took the ECS surface to 2, applied on one axis and missed on the axis the loader enforces. Bumped to 1, minor reset. The bump is what made the missing half of the check live: the loader refused only `api_version_min > host`, so a plugin built against major 0 would have loaded on a major-1 runtime and called entries whose signatures had moved. A major is a break, so the admissible band is one value; `error.ApiVersionTooOld` is its lower bound, with a fixture that could not have existed before — nothing is older than the first major. Counter-factual, the bound removed: the legacy stub reaches `state = .loaded`, one red of 2288. Floor 2286 -> 2290 / 2288, re-derived from the suite. P1-1 is next and is not in this commit.
The third finding, and its general form is worse than the witness the
review names. Refusing the empty set would have closed one value; what
was open is that `registerSystem` took a `SystemDescriptor` — a struct
with public fields — so nothing tied `accesses` to `run`. Take the
descriptor `of(&.{Writes(A)})` produces, assign `.accesses =
&.{Reads(Z)}`, and the body writes A while the DAG is told it reads Z.
That is ARCH-030's original defect rebuilt by field assignment, and no
check inside `registerSystem` could see it: both halves are individually
well-formed.
Zig gives no way to seal a pair, and three forms were refused before the
fourth. A non-pub witness field does not help — a caller copies an
`of`-produced descriptor and overwrites the field beside it. A
`*const Bound` indirection does not either: the child type is reachable
through `@typeInfo` and a literal can then be written. Field access
through a non-pub TYPE is legal, measured. So the closure is not to hide
the pair but to refuse to ACCEPT one: the entry is generic on the
declared set, derives the descriptors AND the body's context type from
it, and `SystemDescriptor.of` is private. The type stays public because
the scheduler stores it and `systemsInPhase` returns it; nothing
consumes one from outside.
What that bounds is REGISTRATION, stated rather than rounded up:
`phases` is a public field, so a holder of the scheduler can rewrite a
stored descriptor after the fact. That is tampering with a DAG already
built, not a registration that lies.
80 call sites across 11 files. The scripted pass ran dry first, which is
what caught a defect in ITSELF: it keyed each generated declaration on
the BODY name, and one `nopSystem` serves twenty-five registrations
declaring twenty-five different sets. Keyed on the system name it parses
clean, and a body used at several sites becomes generic over the spec —
the consequence of a body's parameter type being its declaration. The
identity rule then earned its keep: two tests registering "the same
system in another phase" through two separately declared identical sets
were refused, and now share one declaration.
`case_missing_accesses.zig` is DELETED, not repaired. It omitted the
field and matched `missing struct field: accesses`, measuring that Zig
refuses a literal missing a field without a default — a fact owing
nothing to this milestone. A counter-proof that tests what the compiler
does anyway is green for a reason unrelated to the invariant, and that is
how the general form stood unclosed with a fixture guarding it.
`case_mismatched_pair.zig` replaces it.
The empty-set rule is NOT implemented and is raised, not skipped: the
refusal was written and fires on twelve registrations that are all
honest — five drive the phase pipeline and touch no component, the rest
mutate through `ctx.cmd`, deferred to a flush the DAG does not order.
Journalled under Blockers encountered.
Four corners 314/314, 2271/2290, zero assertions. Determinism on all
four: eight witnesses, four invariants OK, no divergence. Zero cost still
ONE body of 8 instructions. C0.1 3.36 ms, S1 52750 ns, both GO.
`build-and-test (windows-2025, ReleaseSafe, f32)` failed at `e054b26`
and failed again on its first same-SHA re-run. Cleared on the second,
which is the only admissible clearance for this class and which this
class has needed twice before.
Discriminants, both attempts: signature `test runner failed to respond`
present once, sibling class `failed without output` absent, zero
`error: '...' failed:` lines, and 53.0 then 48.6 minutes against a
55-minute budget with conclusion `failure` rather than `cancelled` —
which excludes the cold-cache timeout class on both its own
discriminants at once rather than on duration alone.
Both attempts lost exactly ONE test, where every previous pair of
failures at one SHA had lost different counts. Two identical losses is
the shape a specific implicated test produces, so the reading was tested
rather than repeated — and it is refuted by a measurement the debt's own
note calls impossible.
That note says naming the hung step from the log does not work because a
hung step emits no output. True of the step's SOURCE name; false of its
identity. The `failed command:` line immediately following the hang
message carries the step's build-cache hash, and the two attempts differ:
`01a580472aec240c02328baaccfbc5f8` against
`c810df3f34968acc6a2110b9fd5002da`. Two different executables hung at one
SHA. The equal loss then explains itself with no culprit — both sit in
the contiguous family whose neighbours all report `0 pass, 1 skip
(1 total)`, where a hang costs exactly one test whichever member it hits.
The count was never the discriminant; the identity is, and it is one grep
away in every log this class has already produced.
Collateral, because two debts met on one job: the run carries `Zig cache
is 11222302720 bytes, over the 10737418240 cap - SKIPPING the final
save`. A skipped final save leaves the next run on that cell colder,
which is the cold-cache class's trigger. Neither debt is new; their
interaction is recorded nowhere else.
And a measurement error of my own, reported for the same reason: the
first read of the failing job said every step was green. The filter
`select(.name|contains("ReleaseSafe, false"))` matches three jobs -
ubuntu, ubuntu-arm and windows - and jq concatenated their step lists, so
a green ubuntu job was read as the windows one. Querying by numeric job
id gave the truth at once.
20/20 checks pass at `e054b26`.
The closing review ruled on the one question this milestone raised, and
the ruling does not amend the rule — it reads it.
ARCH-030's object is the IMPLICIT empty set, the one an omission
produces. Since `spec` became a mandatory comptime parameter, omitting it
is a COMPILE error, which is stronger than the registration error the
invariant asks for. A hand-written `&.{}` is a declaration its author
made, not an omission that happened to them.
The decisive half removes the question instead of arbitrating it: the
pairing closure made an empty declaration self-verifying. The body
receives `SystemContextOf(&.{})`, hence `View(&.{})`, whose `get` and
`getMut` refuse at comptime for every `T` — already pinned by view.zig's
"an empty declaration grants nothing". A system that declares nothing
cannot reach a column, so it has no edge to place. Before the pairing was
closed an empty set could lie; after it, it cannot.
The comment at the site carried the measurement and the wrong reason.
"Twelve sites protest" is an argument from the tree; "the type guarantees
it" is an argument from the design, and only the second survives someone
deleting the twelve. Rewritten on the second, citing the pin.
`SystemScheduler.phases` stays public and stays named: rewriting a stored
descriptor needs the scheduler itself, and ARCH-030's threat model is the
system, which receives a context and nothing else.
Comment-only. 314/314, 2271/2290, counter-proof 6/6.
`weld_no_job_body` sat on `View` and not on `ErasedFor`, so `job_bound` refused a view in a dispatched body's arguments and admitted `ctx.view.world_erased` — a `*ErasedFor(spec)`, which `fromErased` takes directly. A worker holding one rebuilds the view with no cast and reaches any entity of the world by handle, which is word for word the reach the marker's own text declares impossible. Measured before anything was written, in both forms: `declaresMarker(ErasedFor)` false, `carriesMarked(*ErasedFor)` false bare AND false wrapped in a struct, against `carriesMarked(View)` true. The guard had nothing to find. What matters more than the fix is that the previous remedy opened this. `ErasedFor` did not exist before 9223a8d; it was minted to close the promotion between views and was born without the guarantee its twin carried, because that guarantee is implemented in another file and nothing at the point of creation recalled it. Second time in this milestone a remedy opened the next breach. So the site carries the RULE and not just the constant: any type through which a `*World` can be recovered must declare this marker, whatever else it is for — written where the next such type will be written, since the file that enforces it is not the file that will forget it. Census measured before, as required, and unchanged after: 6 arg-passing entries derived, 4 dispatch across workers, 4 of those guarded, 5 guarded in total. It could not have moved, and that was measured rather than assumed: nothing in the tree transports an `ErasedFor` into a job. Counter-proofs in both forms, diagnostics measured not predicted. Bare, the refusal carries the marker's text in full, which is what says the reason travelled. Wrapped, it reads `refuses that: no reason declared` — `reasonOf` walking only pointers and optionals where `carriesMarkedIn` enters everything, which is a debt this milestone named and left to the bound's owner. The wrapped fixture's mark deliberately avoids that tail and anchors on the type name: a fixture anchored on a defect's symptom goes red the day it is repaired, and the repair then reads as a regression. The property is pinned in the suite as well, with a non-vacuity control, because the harness runs on one CI cell and the suite on thirteen. Counter-factual, the marker removed: exactly one test reddens. Four corners 314/314, 2271/2290, zero assertions. Determinism on all four, four invariants OK. Zero cost still ONE body of 8 instructions. Also raised and NOT fixed: the plugin entry is called before the version check, so a superseded-major plugin receives the current table. Bounded by measurement - the table handed over is all-default stubs and one site hands it - so the consequence is latent until a real table is handed over. The remedy is a loading-protocol change; closing it hastily is what produced a type born without its twin's guarantee.
`build-and-test (windows-2025, ReleaseSafe, f64)` at `565012c`, its f32 twin cancelled behind it, cleared on the first same-SHA re-run. Signature present TWICE — two hangs in one run, the shape the tenth occurrence had — sibling class absent, zero `error: '...' failed:`, `2253/2285 tests passed (32 skipped)` against a declared floor of 2288 giving three tests lost across two hung steps, and 52.6 minutes against a 55-minute budget with conclusion `failure`, which excludes the cold-cache class on both its discriminants. The discriminant found at the twelfth paid immediately. The two `failed command:` lines name two different executables in ONE build — `fecde19354ea9ccbd9ecb5d20cdb00ac` and `2fe9c3b586059afa4881744abc5715ef` — which is the within-run twin of the within-SHA comparison the twelfth produced. Four distinct identities are now recorded across three attempts and no two agree. Where the tenth occurrence could only say "at least one of the two cost no test at all" and left that asymmetry unexplained, the identities say plainly that two unrelated steps hung, so three tests across two steps needs no theory of a culprit. 20/20 checks pass at `565012c`.
`build-and-test (windows-2025, Debug, f32)` failed at `0aff532` and is NOT the hang class: the hang signature is absent, zero tests are lost (2288 collected, exactly the floor), and one assertion genuinely fires — `error.Win32ThreadSafetyTimeout`, a 30-second wall-clock bound on three threads doing 100 createWindow/destroyWindow pairs each. The job ran 9.65 minutes against the ~50 a hang costs. Cleared on the first same-SHA re-run. The discriminant that settles it is general to every flake class and costs one command: a diff with no Zig in it. That commit's diff is two Markdown files and seven lines, on top of a commit where this same cell had passed — so no code changed between the green and the red, and the commit cannot be the cause. The sibling Debug/f64 cell passing at the same SHA corroborates and is weaker, Win32 windowing having nothing to do with the physics scalar. It must not be confused with the class M1.1.9 retired from `crash_recovery.zig`. Those assertions ran after the blocking receive had returned and guarded nothing — a duration asserted post hoc. This loop bounds a wait and abandons it, which is a correctly shaped hang guard; what fires is its budget on a runner the suite saturates. Removing it removes a real guard and raising it weakens the bound it gives, so it is recorded rather than touched. Stated because it is not free: four of this session's CI runs went on documentation-only commits recording flakes, and two of those runs themselves flaked.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brief:
briefs/M1.A-ecs-access-enforcement.mdARCH-030closed in Zig, and its Tier 3 form aligned. The declared access set becomes the type of what a system receives.Closing notes
What worked.
specremoved the failure the milestone exists to close, rather than detecting it.SystemDescriptor.of(phase, name, spec, body)produces the descriptors AND the type of the context; there is no second list, so nothing can disagree. It also retired a hazard nobody asked it to:registerSystemstores the caller'saccessesslice without duplicating it, andforge/sync.zigdocuments a real crash from an inline&.{ … }temporary dangling — a derived set is a comptime constant and cannot.*anyopaque, so the bypass costs an explicit cast at the call site instead of a field access. Pairing that with the job-bound marker was necessary rather than decorative: the erasure makescarriesMarkedblind, and a view in a worker's arguments is exactly the hazard the marker exists for.World— were caught by reading the diff pair by pair, not by the compiler, which was happy with several of them.What deviated from the original spec.
WELD_ECS_PROTOCOL_VERSION1 → 2. Not named by the brief; prescribed byroot.zig's own doc comment for exactly this case.Worldaccessors (changedTick,resourceBytes,currentTick) outside the brief's file list, andobservers.ziglikewise — both justified in the Execution log.zig build ecs-access-zero-cost, not atestblock: a test cannot disassemble the binary it lives in, and the repository's only precedent for reading a claim in emitted code is a build step feeding a Zig scanner (forge-asm-inventory).What to flag explicitly in review.
root.zignames as versioned changed, all breaking for a Tier 1 caller. If the intent was to keep the surface at 1, that is a decision to take before merge.FrameContext.useris the residual the view cannot close, and it is the path every system in the tree actually uses to reach its query. The enforcement is real on the view's own surface and silent on that one.weld_api.his generated this header should be deleted rather than reconciled.registerOn*family lost itsworldparameter at 42 call sites. Mechanical, but wide.Final measurements.
2261/2280 tests passed, 19 skipped) and 2278 onwindows-2025(derived from the guard's twoonly_on = .windowsentries; the CI cell confirms it). Re-derived from the suite four times as blocks landed — 2263, 2271, 2275, 2279, 2280 — the closure agreeing independently at each.312/312 steps.zig build forge-determinismon all four corners: eight committed witnesses byte-identical, four discrete invariants OK,divergence frame: none within K=60— on a DAG whose edges changed, the forge system having gainedReads(RigidBody).251db05through a worktree: 3.66 ms against 3.66 ms, delta 0.0 %. S1 at--workers=4over eight interleaved rounds: 52 666 ns against 52 646 ns, +0.0 %.getMuttakes it to 23 against 8.authorityOfreadsRigidBodyfrom inside the registered system and the declared set named it nowhere. Second catch in the bench:bench_integratedeclared nothing while its worker body mutates two columns.Residual risks / tech debt left intentionally.
M1.Dentry has M1.A as its cause, which is the milestone's own exit rule. The residuals are named inCLAUDE.mdunder M1.A residuals and none is deferred work:FrameContext.user, the absentview.query(no consumer),services/physics.zig's*Worldon an Etch-callable path (not a system entry point, so outside the invariant's object),M1.D.23neither fixed nor promoted, and the Tier 3 guarantee bounded to a compilation witness.M1.D.23andM1.D.24.Validation
zig build testgreen on the four corners (Debug + ReleaseSafe × f32 + f64) —2261/2280bench/ecs_benchmark.zigC0.1 and S1: no regression beyond 5 % against251db05, measured interleavedzig build ecs-access-counterproof,ecs-access-zero-cost,c-api-read-column-constness,forge-determinismzig build,zig build test,zig fmt --check,zig build lintgreenCLAUDE.mdupdated (current-state table, +1 Tags row, residuals, date) — and its current-state row corrected, it still described M1.E as active after that milestone's mergeengine-ecs-internals.mdcited by the brief, plus Englishplusand the Zig identifierun. Zero French prose. Run with Python, not agrepbracket class.sync.zig"carries theWorld",scheduler.zig"the cmd buffer borrowsworld")Status: CLOSED,Closed: 2026-09-12docs(brief): close M1.A🤖 Generated with Claude Code