docs(spec): DD-043 — native and reactive targets (design only) - #98
Conversation
Design for instrumenting a GraalVM-native, reactive Quarkus application: build-time attachment via a Quarkus extension, an event-loop-correct request boundary, and a replaced invariant set. Two axes the tool has assumed universal until now are unpacked separately: runtime-vs-build-time attachment, and thread-per-request vs event loop. Preserves closure's no-source-modification property by injecting through a Maven lifecycle participant (-Dmaven.ext.class.path), the build-time analogue of the operator's CATALINA_OPTS injection. Phase 0 spikes gate the rest: offline-JaCoCo survival under AOT and plugin-execution injection can each invalidate parts of the design. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ajors Fable's review (docs/superpowers/specs/reviews/) found the spec was not executable as written. All blockers and majors addressed: - BLOCKER: ./mvnw supplies Maven, not a JDK. Host is JDK 17, rest-heroes pins maven.compiler.release=25 (verified) — javac fails before augmentation. The whole Maven build now runs in a JDK-25 container, which is also where the injector jar and MAVEN_OPTS must cross a container boundary. - BLOCKER: Vert.x BlockedThreadChecker is log-only — no callback, metric or API. The headline reactive invariant had no transport, and this repo rejected log scraping twice (DD-019, DD-040). Replaced with an extension-owned watchdog measuring runOnContext scheduling delay. - MAJOR: "the DD-040 channel transplants unchanged" was false — its load-bearing element is the poll waiting on ITERATION_LOCK, the very lock removed here. Quiescence redesigned as a completion-parking poll (2s bound, 4s driver timeout). The X-Basquin-Cost header fast path cannot exist; every iteration polls. - MAJOR: driver-side concurrency-1 serializes only the driver. On Tomcat a health probe took ITERATION_LOCK too and so could not overlap; without it, probes and post-response reactive work land inside measurement windows. Added an in-flight counter that taints such samples UNMEASURED, with a reported taint rate. - MAJOR: the JFR cross-check compared throttled gross-allocation samples against net heap delta — unfalsifiable, and DD-004 already ruled JFR soft-only. Redefined as whole-run per-route rankings; the exact cross-check (getThreadAllocatedBytes) lives in the JVM-mode cells. - MAJOR: negative controls needed planted defects in unmodified apps — a thesis violation. Controls now ship as extension-owned defect routes. - MAJOR: Quarkus's bootstrap resolver may re-read poms from disk and bypass the mutated in-memory model, silently yielding an uninstrumented binary. S4's acceptance is now the Installed features banner, JVM and native. The Develocity precedent is withdrawn as oversold. - MAJOR: Agent.end()'s Thread.sleep(25) would block the event loop every iteration. The core extraction now splits evaluation from composition. Reactive invariants are soft-by-structure; Tomcat defaults to hard. - MAJOR: S1's stated failure mode was the least likely. Build-time class init captures probes into the image heap, inflating the coverage baseline; "must increase" cannot catch it. S1 rewritten around three signatures. - MAJOR: scope was 4-6 features in one unit. Split into six PRs in dependency order, per the project's PR-granularity rule. Also corrected: there is one shared boundary core today (the valve delegates to agent.RequestBoundary), not three copies; FilterBuildItem is the idiomatic router-wide filter, not RouteBuildItem; latency's population changes rather than merely improving; "cannot fail to attach" overclaimed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@claude please review this spec PR. This is design only — no product code. The artifacts are a design spec and the adversarial review that reshaped it. Useful things to be skeptical about:
Verified facts the spec relies on, if you want to spot-check: |
|
Claude finished @basquin-bot[bot]'s task in 3m 18s —— View job Review: DD-043 — native and reactive targets
This is a well-executed revision — the second draft is honest about where the first one overclaimed, and every load-bearing citation I checked against the actual code holds up. Verified facts (all confirmed):
1. Continuity claims (DD-040/DD-010/DD-004/DD-019). All check out against code, not just prose:
No fabricated continuity claims survived into this revision as far as I can tell. 2. Structurally-incapable-of-firing signals. The §6.3 watchdog (measuring 3. No-source-modification constraint. Holds throughout — §5's lifecycle participant and §7.3's extension-owned control routes are both genuinely non-invasive. The one place it can bend is §5.2's fallback: "the documented degradation is §5.1's pom edit … acceptable for a named app, never as the design." That's an honest, explicitly-flagged exception rather than a silent violation, and §5.2 already requires the benchmark row to record which mode was used — no action needed, just confirming it isn't a quiet backdoor. 4. Phase 0 gating logic. Found one real cross-reference gap. §7.1 states "S1–S4 share no state and run as concurrent subagents" — but S1 (offline JaCoCo under AOT), S2 (SubstrateVM GC behavior), and S4 (banner check "JVM and native") each require a native build of the quickstart. §7.2 later establishes the actual constraint: "native-image wants ≥4 cores and several GB each and this host has 8 cores / 15 GB: native builds are serialized on a mutex." §10 does generalize that mutex rule to "S1–S4 in Phase 0" too, so the design isn't actually inconsistent — but §7.1 itself, read on its own, would lead an implementer to kick off three concurrent native-image builds. A one-line cross-reference from §7.1 to §10's mutex rule (or restating it inline) would close the gap before someone reads only §7.1 and OOMs the host on the first spike. Everything else — the toolchain fix (containerized JDK-25 Maven build), the watchdog replacement for the log-only checker, the heap-taint mechanism, the |
Six tasks producing evidence for S1-S4, with no product code. S4's mechanism question is answered without building the injector or the extension: a throwaway lifecycle participant injects an already-published Quarkus extension and the acceptance is the Installed features banner. S1 tests three failure signatures rather than the one the first spec draft named, since build-time class init inflating the coverage baseline is both likelier than frozen probes and invisible to a must-increase check. Records one scope limit honestly: the fixture has no database, so S2's post-response quiescence question is DEFERRED to the rest-heroes cell in PR-2 rather than answered against a floor that lacks the polluter it worries about. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ve-build mutex Two gaps found in review of #98: - The §6.2 JFR cross-check was the only signal in §6 with no negative control, i.e. silently exempt from the rule that had just demoted it. It now carries a control (the alloc route must rank first by aggregated ObjectAllocationSample totals) with an explicit fallback to diagnostic-only-and-not-published if it cannot be made to fire. - §7.1 said S1-S4 'run as concurrent subagents' while S1, S2 and S4 each need a native build. §7.2 and §10 carry the serialization rule, but §7.1 read alone would have an implementer start three concurrent native-image builds and exhaust 15 GB. The mutex is now stated inline rather than by reference. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Both findings addressed in code, not replies — 4f109cc. 1. JFR cross-check had no negative control (§7.3). You're right that it was silently exempt, and the reason it matters is sharper than "a missing row": the cross-check was demoted in review precisely because an undesigned signal renders as a clean column, so exempting it from the rule that demoted it reintroduces the same defect one layer up. Added a control row: 2. §7.1 vs the native-build mutex. Also correct, and the failure mode is concrete — S1, S2 and S4 each need a native build, and three concurrent Thanks for independently verifying Note on checks: |
basquin-bot
Bot
added
the
ready-for-approver
Approver review — CHANGES REQUESTED (2 findings)Design-only PR; I read the spec, the plan, and the committed review, and verified the load-bearing citations against the code myself rather than trusting the prose. What holds up (independently checked): every in-repo code citation is verbatim-correct — Two findings, most serious first. 1. [blocking — invariant #3] §3.1 asserts external pom facts as "Verified 2026-07-24" with no committed evidence — and the PR's own review says they are NOT verified
Fix (either): commit the evidence (the three pom lines, under the spike dir) and cite it; or downgrade "Verified 2026-07-24" to "to confirm in S0 (see review NIT #19)", matching how the plan (line 119) and the review already treat it. (§6.2's "verified" native-JFR claim is the same category but better off — the review grounds it in cited GraalVM docs.) 2. [invariant #1 / §7.3's own rule] the negative-control table omits the §6 "5xx / crash" signal — the exact silent-exemption shape the last commit just fixed for JFR§7.3 states the rule ( Not hypothetical: crash detection here routes through a brand-new boundary (Vert.x Fix: add a crash control (the plan's Deliberately not verifiedExternal web/pom facts I have no network path to check (the actual pom contents, Vert.x hook semantics, native-JFR support). Finding #1 is about the "Verified" label lacking committed evidence, not a claim the numbers are wrong. And whether the spikes run — that's PR-0's evidence, separately reviewable. Both fixes are one-line-cheap; a round trip is cheaper than a "Verified"-that-wasn't propagating into the six downstream PRs that cite this spec. |
basquin-bot
Bot
added
agent-changes-requested
…red build state The plan's parallelization table had Task 2 and Task 3's JVM steps running concurrently. Both invoke env/build.sh, which mounts the same fixture/ as the container workdir and builds into fixture/target/, and both share one .m2 local repository that Maven does not lock across processes. Either collision yields plausible-looking but wrong spike output, which on a PR about trustworthy measurement is the worst available failure. The parallelism at stake was two short JVM builds; the three native builds are serialized regardless and dominate wall-clock. Records the safe form of parallelism (per-task fixture and repo copies) for later. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…trol-table gap Approver findings on #98, both upheld: 1. §3.1 asserted the target toolchain pins as 'Verified 2026-07-24' with nothing committed, while this same PR carries a review recording the Java-release claim as unverifiable (fable review NIT 19). A PR that both asserts a fact and commits a document calling it unverified fails the claims-trace-to-evidence invariant. Now backed by bench-results/dd043-target-pins-2026-07-24/ — the upstream pom.xml files with provenance. Both rest-heroes and rest-villains pin release=25, Quarkus 3.37.3, JaCoCo 0.8.15 identically, which the 2x2 needs: a toolchain difference between control and target would confound every cell. 2. §7.3's control table omitted the 5xx/crash signal — the same silent exemption just fixed for the JFR cross-check. Auditing §6 exhaustively found three more (worker-pool saturation, connection-pool pending acquisitions, FD count). Rather than add rows one review at a time, §7.3 is now a closed set: every signal in §6 either has a control row or is listed as explicitly unpublished. The three secondary reactive signals are declared unpublished — sketched, no thresholds, no transport — and promoting one is a spec change, not an implementation detail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Both findings upheld and fixed — 1. Unevidenced "Verified" claim (§3.1). You're right, and the contradiction was sharper than a missing citation: the PR asserted the pins as verified while committing a review that recorded the Java-release claim as unverifiable (fable review NIT 19). I had verified it in-session, which is not the same as committing it — and in-session verification is precisely what invariant #3 exists to distrust. Evidence now committed at One thing the artifact surfaced that I had not checked: 2. Missing 5xx/crash control (§7.3). Correct, and the same shape as the JFR gap fixed one commit earlier. Auditing §6 exhaustively rather than patching the named row found three more silently exempt: §6.3's worker-pool saturation, Hibernate Reactive connection-pool pending acquisitions, and FD count. Adding rows one review at a time was clearly not converging, so §7.3 is now a closed set: every signal in §6 either has a control row or is listed as explicitly unpublished. The three secondary signals are declared unpublished — sketched, no thresholds, no transport to the result store — and promoting one is a spec change rather than an implementation detail. The 5xx row also names why it is easy to zero silently: it reads Re-labelled |
basquin-bot
Bot
added
ready-for-approver
Task 1 of the DD-043 Phase-0 spike plan: the shared harness every later spike depends on. Candidate A (quay.io/quarkus/ubi9-quarkus-mandrel- builder-image:jdk-25) carries both JDK 25 and native-image, so no Maven step ever runs on the JDK-17 host and native builds later need no docker socket. env/build.sh package -DskipTests reaches BUILD SUCCESS and produces fixture/target/quarkus-app/quarkus-run.jar, confirming BLOCKER 1's fix. Deviations from the brief, all recorded in env/ENVIRONMENT.md: the image's ENTRYPOINT is native-image (not a shell), so every invocation overrides it; the image ships no system mvn, so the one-time archetype generation bootstraps a throwaway Maven distribution instead; $HOME resolves to "/" under the mapped host UID, so mvnw's own cache-dir lookup needed an explicit HOME=/m2; the archetype set quarkus.platform.version to 3.37.4 (the registry's current recommendation) despite the :3.37.3: create coordinate, hand-corrected to the pinned 3.37.3. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBabr7SFhkhqpHpV2tWZZ6
core.filemode=false on this WSL/NTFS checkout means local chmod +x never reached git — both scripts were committed 100644. Every later spike (and anyone checking this repo out on a normal Linux host) invokes them directly (env/build.sh ..., and build.sh's own ./mvnw inside the container), so a non-executable bit means "Permission denied" on a real POSIX checkout. Matches the existing convention for gradlew and the deploy/*/build.sh scripts, which are already 100755. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBabr7SFhkhqpHpV2tWZZ6
Review finding: build.sh defaulted IMAGE to the `jdk-25` tag while ENVIRONMENT.md claimed a digest was recorded "for exact reproducibility." Since later Phase-0 tasks (S1-S4) all build through this shared harness, a tag that moves between Task 1's validation and a later task's build would silently swap in a different image with no signal that anything changed — the exact drift this harness exists to prevent. Pin the digest that was actually probed and validated; BASQUIN_SPIKE_IMAGE still overrides it per-invocation. ENVIRONMENT.md now says the digest is what build.sh pins, not merely a value recorded alongside it. Also: document the EXTRA_DOCKER_ARGS unquoted-expansion constraint (word-splitting is intentional and load-bearing for Task 3, but it means callers can't pass paths with spaces or glob characters), and add a sentence to README.md's fixture-regeneration snippet explaining why it doesn't need -e HOME=/m2 (it shells out to a throwaway real mvn, not the $HOME-dependent mvnw wrapper, which doesn't exist yet at that point). Verified: env/build.sh package -DskipTests still reaches BUILD SUCCESS and produces fixture/target/quarkus-app/quarkus-run.jar; docker inspect confirms the digest actually resolved and was the image used. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBabr7SFhkhqpHpV2tWZZ6
The client-disconnect finding driving a DD-043 spec change rests on the /slow row: status=200 succeeded=false. Round 1's [PROBE] line carried no request path, so which line was /slow was established by curl ordering, id-matching, and timing — a sound inference, independently verified, but not something the raw evidence stated on its own. Evidence backing a spec change should be unambiguous on its face. - BoundaryProbe.java: capture the request path at filter entry and add it to the [PROBE] printf as path=%s, alongside the unchanged id/status/ succeeded/cause/ms fields. Drop the now-confirmed-unused RoutingContext import (the lambda's rc type was always inferred). - Re-ran the spike end to end with --entrypoint java on the docker run (the image's default entrypoint is native-image; omitting it silently starts a native build instead of the app). Committed build.log and startup.log, neither of which a prior review could verify from the diff. - Preserved round 1's probe.log/curl.txt as probe-round1.log/curl-round1.txt rather than overwriting reviewed evidence with unreviewed evidence. - Re-derived the verdict from round 2's output rather than carrying round 1's forward: the two rounds agree on every disposition (same statuses, same succeeded values, same cause on /slow) — this is a labelling fix, not a correction to the underlying finding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBabr7SFhkhqpHpV2tWZZ6
… a repository injection Closes the gap in Task 3's S4 verdict: that spike injected an already-published Central artifact (quarkus-smallrye-openapi), but the real mechanism must inject com.basquin:basquin-quarkus, which won't be on Central. Installed a throwaway artifact under a groupId absent from Central (com.basquin.spike.localonly:local-probe-dep:1.0) into the spike's local repo via `mvn install:install-file` in a container, extended InjectProbe behind -Dbasquin.inject.local=true (default behaviour, and Task 3's committed logs, unchanged), and rebuilt the fixture through the untouched env/build.sh with no repository declared and no pom.xml change. Result: BUILD SUCCESS, no resolution errors, and the jar landed in fixture/target/quarkus-app/lib/main/ — the local repository is consulted for injected dependencies same as any disk-declared one. Verdict CONFIRMED for the local-repo case; the repository-injection contingency spec §5 worried about is not needed for plain dependency resolution. Scoped explicitly: this tests resolution of a plain jar, not Quarkus extension discovery (META-INF/quarkus-extension.properties), which Task 3 already covers for a real extension. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBabr7SFhkhqpHpV2tWZZ6
…roject Review caught that the addendum's InjectProbe.java built a single mutable Dependency instance outside the per-MavenProject loop and added that same object to every project in the session. Maven's model objects are mutable, so a multi-module reactor would end up with every project's dependency list holding a reference to the *same* Dependency — any later in-place mutation (or logic relying on per-project object identity) on one project would silently bleed into all the others. The fixture here is single-module, so the bug was invisible in the committed evidence; still a latent bug worth closing before this mechanism generalizes to real multi-module targets. Fix: construct a fresh Dependency per project, inside the loop, with a comment explaining why the allocation must stay there. Both the Central (quarkus-smallrye-openapi) and local-only (local-probe-dep) code paths are otherwise unchanged. Rebuilt the probe jar and re-ran both injection scenarios in JVM mode only (native is unaffected by an in-memory-model change on the Maven side, and Task 3 already proved the native path). Results are unchanged: Central artifact still shows in Installed features, local-only artifact still lands in fixture/target/quarkus-app/lib/main/. Regenerated evidence files overwritten; findings.md and .superpowers/sdd/task-3-addendum-report.md note the re-verification. Also committed the previously-missing precondition evidence for the addendum: two curl checks confirming com.basquin.spike.localonly is absent from Maven Central (both 404), in addendum-central-absence.txt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBabr7SFhkhqpHpV2tWZZ6
… is a runtime line Steps 3-4 grepped the Maven build log for 'Installed features:'. That line is a runtime startup banner and never appears in build output, so the recipe could never match — the Task 3 implementer diagnosed it and ran the built artifacts instead, which is what Step 5 already did. Corrected both steps to run the artifact, including the --entrypoint java override the builder image requires (its own ENTRYPOINT is native-image, so without the override the container silently starts a native-image build instead of the app — a trap that has now caught three tasks). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…the measurement User asked whether a Redis server would help message passing, noting shared memory for input was very optimal in the original closure work. Records why it would not help explore's hot path: ITERATION_LOCK plus Agent.end()'s Thread.sleep(25) (agent/Agent.java:118) cap throughput near 1/(25ms + appTime) — measured 6.1/s Roller, ~11/s JSPWiki. Against ~91ms per iteration a loopback round trip is ~0.1-1ms, so the grace sleep alone is 25-250x the whole transport cost. Optimising transport recovers low single digits while the cost sits in one sleep and one lock. Also records why shared memory does not transfer from the original closure work: it cannot cross pods, and the HTTP request is the system under test rather than overhead — delivering input via shm would bypass the servlet/Vert.x stack where the defects actually live. The AFL analogy transfers to the coverage map, not the input path. Where it is the right question: DD-041's coordination plane (corpus, coverage map, cost ranking), which line ~307 already frames as partition-per-worker vs a shared work-queue. Weighed against the existing dashboard aggregator and the already-union-merged coverage before adding a new stateful dependency. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ures) REFUTED. /coverage throws NoSuchMethodException on every native request — native-image's default reflection policy strips reflective access to org.jacoco.agent.rt.internal_*.Agent#getExecutionData(boolean), a method javap confirms genuinely exists in the pinned 0.8.15 runtime jar. Identical code works under JVM mode (same JDK 25, same Quarkus 3.37.3), isolating the failure to native/AOT specifically rather than a fixture or version bug. None of the three failure signatures could be evaluated on real native coverage data: no valid .exec was ever obtained (signature i untestable), NeverCalled was eliminated from the image as dead code so its "zero" is static, not a RuntimeData reading (signature ii untestable via the live path), and jacoco-cli rejected all three native dumps outright as invalid execution data before any class-id matching could occur (prior to what signature iii was written to catch). Per spec 7.1: S1 failing voids 6.4 as written. Full evidence, root-cause analysis, and the JVM-mode diagnostic that isolates AOT as the cause are in s1-coverage/findings.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBabr7SFhkhqpHpV2tWZZ6
…(CONFIRMED) S1 found /coverage throwing NoSuchMethodException in every native request, root-caused to the reflective two-hop RuntimeData lookup, not JaCoCo-under-AOT generally. S1b tests the fix: CoverageProbe now calls RT.getAgent().getExecutionData(false) directly against JaCoCo's public IAgent type, no reflection at all. It works — real JaCoCo execution data at t0/t1/t2, all three failure signatures now testable and all three hold. Signature (ii) needed a new instrument since NeverCalled is eliminated from the native image by reachability analysis; added Probe.unused(), a JAX-RS route that's registered (so it survives into the image) but never requested. Its probe reads zero throughout, including after t1 when Probe's class record is demonstrably live (sibling methods flip to covered) — real evidence against the inflated-baseline worry, not just NeverCalled's weaker structural-absence zero. Also records the denominator finding: NeverCalled still counts toward jacoco-cli's denominator (it reads the preserved pre-native classfiles), so a native coverage percentage has a lower achievable ceiling than a JVM one from the same source tree — raw percentages across modes aren't directly comparable. Verdict: CONFIRMED. Spec §6.4 needs the direct-call form, not a rewrite. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GBabr7SFhkhqpHpV2tWZZ6
Phase 0 ran and the gate PASSED. Neither of the two outcomes §7.1 named as
spec-voiding occurred: §6.4 is amended rather than void, and §5's
no-source-modification mechanism holds.
REPORT.md consolidates all four spikes — verdict table, per-spike evidence
with file/line citations, and an explicit scope block. Two reporting hazards
handled deliberately: the ~3.10 MiB/min idle-drift figure is presented as
n=3 quantum events from one run rather than a measured rate, and the
non-enum `CONFIRMED (scoped)` tag is decomposed into a strict enum value
plus a separate scope line so the verdict stays machine-readable.
S1 is reported as REFUTED as specified, then CONFIRMED via S1b. The
reflective read path failing is a real finding about the method §6.4
prescribed, not something to collapse into a bare CONFIRMED.
Eight amendments, each traceable to committed evidence:
1. §6.4 coverage read is a direct typed call, never reflection —
also decouples the design from JaCoCo's shaded package name
2. §6.4/§7.4 the native coverage denominator is not the JVM's; the two
percentages are not like-for-like comparable
3. §6/§7.3 5xx/crash gated on ar.succeeded(); disconnect becomes a
third disposition, with a control asserting it does not
increment the crash counter
4. §6.1 the measurement floor: 524,288 B resolution, ~1 MiB
practical per-request minimum
5. §6.1 System.gc() works under SubstrateVM, so gcBeforeMeasure is
recommended on native, not merely noted as portable
6. §5 the nmt hedge is dropped; the fallback is withdrawn
7. §5 a fresh Dependency per MavenProject — a shared instance
passes every single-module test and bites on the real
multi-module targets
8. §7.1 S1's failure signatures rewritten; NeverCalled cannot be the
signature-(ii) instrument because reachability analysis
deletes it
A ninth edit records S4's result in §5.1, whose hypothesis the spike
refuted. Sections checked and needing nothing (§4.3, §6.5, §6.2, §6.3,
§4.4, §8.1) are named in the status block with reasons, so a silent absence
is not mistaken for an unchecked section.
ROADMAP gains a DD-043 ladder row recording that Phase 0 cleared PR-1.
No committed spike evidence was altered.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBabr7SFhkhqpHpV2tWZZ6
findings.md quoted 'Total time: 44.136 s' from the first addendum run, but addendum-build.log was regenerated by the Dependency-aliasing fix round and now reads 44.374 s at line 33 — so the document cited a log that said something else. Trivial in magnitude (a wall-clock duration, no bearing on any verdict) but it is exactly the drift that 'every figure comes from the artifact' exists to prevent, and a reviewer re-deriving figures would rightly flag it. Found by the Task 6 agent doing precisely that. Quotes the committed log and adds a provenance note recording the regeneration, so the discrepancy is explained rather than silently overwritten. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ts through to what depends on them
Round 1 rewrote the sections the Phase-0 evidence contradicted but not the
sections that depend on them, so the spec contradicted itself in four places
and shipped three controls that could not fail. All 1 Critical / 6 Important /
4 Minor findings from .superpowers/sdd/final-review-pr98.md are addressed; no
committed spike evidence was altered.
C1 §7.3's coverage control still named the class-level instrument amendment 8
disproved. It now uses §7.1's method-level instrument and has two named
ways to fail: no sibling flip (no live record), or the instrument's class
absent from the class-initialization report (structural absence).
I4 The replacement instrument had no §1.1-compatible home — a planted JAX-RS
route edits app source, and an extension-owned one is not in the app's
coverage denominator. Decision: a *withheld application route*,
pre-registered in the bench manifest, chosen from the contract-first route
set. Plants nothing. The -H:+PrintClassInitialization report is adopted as
the reachability precondition; the t0-confinement variant is recorded as
rejected-as-primary (it reports initialization kind, not startup execution
— 11,912 of 12,094 rows are BUILD_TIME) and retained as a diagnostic.
I1 §6.2's body said native JFR streaming was "verified" while the ledger
called §6.2 unverified. Body corrected, what-would-verify-it stated, and
§6.2 made a PR-5 entry gate.
I2 Thread leak added to §7.3's explicitly-unpublished paragraph; §6's
cross-reference repointed from §6.2 to §6.3. The closed-set rule now
covers dispositions as well as signals.
I3 Two §7.3 rows added that make taint and UNMEASURED *fire*; the positive-
noise row's pass-either-way disjunction restated as a threshold. §6.1 now
specifies the in-flight counter as process-global with its decrement on
every disposition, closing the 0%-forever mechanism at source.
I5 S3's JVM-only scope carried into the spec in five places, with an S3
re-check as an entry condition on both §7.2 native cells.
I6 "§5's mechanism holds" narrowed to the dependency half S4 measured, in all
seven places it appeared; plugin-execution injection is now §8.2, a PR-4
entry gate.
M1-M4 REPORT.md prose reconciled with its artifacts (the 9.454s/"3 seconds"
divergence named, the uncited Maven version dropped, banner-native.txt
disclosed as a 9-line run log); §6.2's com.sun.management claim labelled
uncited.
Left as-is, with reasons recorded in the ledger: the Phase-0 plan (a record of
what was executed — editing it would destroy amendment 8's provenance) and
REPORT.md's amendment numbering (1-8 are what evidence forced; this round is
what a review forced).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GBabr7SFhkhqpHpV2tWZZ6
Phase 0 executed — gate PASSED, spec amended against the evidenceSince the last review this PR grew from a spec into spec + executed evidence. All four Phase-0 spikes ran; consolidated report at
§7.1 named two outcomes that would have voided design sections. Neither occurred. The findings that changed the design
Review rounds on this branchA whole-branch review returned 1 Critical + 6 Important, all fixed in The hardest finding was that the replacement instrument had no thesis-compatible home: it must live in the app's coverage denominator, but §1.1 forbids touching app source. Resolved with a withheld application route — both targets are contract-first, so one route from Worth noting the fix agent refuted my own proposed resolution with evidence: Scope limits deliberately kept visibleS3 is JVM-mode only. S2 tested one allocation size and its Hibernate-Reactive quiescence half is DEFERRED to PR-2. S4 proved dependency injection and augmentation but not plugin-execution injection, which §5 also requires — recorded as an open question for PR-3/PR-4 rather than folded into "the mechanism holds". |
Approver review round 2 — APPROVED (ready for a human to merge, not merged)I re-judged the whole branch (design spec + Phase-0 spike evidence, 96 files, no product code) on its own evidence — verifying figures against raw artifacts and fixes against committed text, not on the PR description or the two committed self-reviews. Round-1 findings (2) — both fixed:
Round-2 whole-branch review (1 Critical + 6 Important + 4 Minor) — all fixed in committed text, independently confirmed:
Independently verified against raw artifacts: S1 Invariants: (1) no silent zero — the round-2 fixes exist precisely to eliminate controls that pass without measuring; structural-absence vs live zeros are distinguished honestly. (2) no token/secret in any artifact (only Gate "PASSED" is honestly scoped: S1 refuted §6.4's read path but S1b established a working one; S4 honoured only dependency injection — the plugin-execution half (§8.2) and native JFR streaming (§6.2) are carried as explicit PR-4/PR-5 entry gates, not claimed as passed. The gate clears PR-1 (pure refactor); the risky halves are flagged, not glossed. Did not verify: external web/JFR facts with no in-repo path (spec flags each unverified and gates it — correct for a design doc); quoted-not-captured Non-blocking note: Approved = ready for a human to merge. Only the human merges. |
basquin-bot
Bot
added
approved-awaiting-merge
…ledger true Approver review of #99 found 2 blocking + 1 moderate + 2 minor. Both blockers were claims I made. BLOCKING 1 — "nothing under that path is citable from any committed file" was an overclaim. I grepped DD-043's own three docs plus DESIGN-DECISIONS.md and then made a statement about the whole repo. A full `git grep` finds SIX citations across FIVE files. Narrowed the roadmap claim to what was actually checked (DD-043's documents), and recorded the real debt in TODO.md with the audit table. The debt is not a find-and-replace: in the inputpair-capture and nonce-and-3xx specs the dead pointer IS the stated "Motivation source", and one plan carries a bare "Proven by throwaway spike" whose proof is the missing file. Dropping those citations removes the only stated provenance for a design decision. Recorded per-site work rather than a blanket fix. BLOCKING 2 — the spec's Round-2 ledger claimed to reproduce every finding, and did not for three of eleven. M1-M3 were collapsed into one row that states none of them and whose scope column said "REPORT.md only" — wrong, because M2's fix also removed the unevidenced "Maven 3.6.3" from spec §3.1. Since #99's whole argument is that the ledger is the durable substitute for the uncommitted review report, a ledger that does not actually carry the findings makes the substitution hide a gap instead of closing one. Split into three accurate rows. Worth noting @claude reviewed this same substitution and passed it as "verified equivalent"; the approver caught what it missed. Two reviewers earned their keep here. MINOR 4 — the roadmap asserted "no open PRs" while #99 is itself open, against the precedent #98 set by listing itself. Now lists #99 in both places. MINOR 5 — the citation debt was recorded only in the PR body; ROADMAP:138 designates TODO.md for standing debts. Now there. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both blockers were again my own claims, and both were in the paragraph I had just rewritten to be "checkable": - It cited `git show --name-only 496311c` as the verification. 496311c is reachable from no ref (#98 was squashed into 6aa16fc, branch deleted), so the command resolves only from my local objects. A verification a reader cannot run is worse than none, because it looks sound. - "named ~60 of the 88 files" was wrong: the enumeration covered 48. I summed every `.txt` when the list named only `series.txt`. The lesson from four rounds is not "count more carefully". Every false claim on this PR has come from meta-commentary about my own drafting history — how many files an earlier wording named, what a previous draft said, which command proves it. None of that helps whoever implements PR-1. So it is deleted, replaced by a short statement of what Round 2 touched plus an honest note that per-round diffs are not recoverable post-squash. Kept, because they are a different thing: the three "an earlier draft claimed X, that was wrong" notes attached to *design* claims (the unevidenced pins, the toolchain claim, the DD-040 transplant). Those tell a reader not to reinstate a refuted claim. Archaeology about my drafting process does not. Also: ROADMAP no longer labels #99 "docs-only" 41 lines above calling that label untrue; TODO says three load-bearing citations (naming all three in a table) and three parenthetical receipts, which is what the audit found; and the [PROBE] fields are ids, not timestamps, matching REPORT.md's own wording. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…curring defect Approved with 4 findings (all Minor/Nit). Three are actionable here and all three are the same mistake in different clothing: a hand-written reference that does not track the thing it points at. F1 — test/agent/ResetLoaderParentFirstTest.java cited .superpowers/sdd/task-3-report.md, which exists in no tree. A committed file must not point at untracked scratch; a reader following it finds nothing. Replaced with the instruction that actually matters: repeat the mutation by moving a class out of `package agent`, not by changing the literal. F2 — worse than the drift the approver called it. Spec §4.1 cited the guard at build.gradle:175 "wired into check (:217)". The commit that moved it to :197 also changed the MECHANISM: it is now finalizedBy on the jar-producing tasks, precisely because the release path invokes jar/runnerJar directly and never runs check. So the spec described the wrong location and the wrong wiring. Now cites the task by name, states the finalizedBy mechanism and why, and notes the expected entry set is derived from basquin-core's jar rather than hand-listed. No line numbers left to drift. F3 — the plan attributed its baseline to "main (commit f6d143a)". f6d143a is not an ancestor of main (confirmed); it was the tip of an unmerged branch. The measurement is unaffected but the provenance was wrong. Corrected to main at 6aa16fc, which is this branch's actual merge-base. Not fixed here, deliberately: docs/ROADMAP.md:124 and spec:80 still cite .superpowers/sdd/final-review-pr98.md. Both are already fixed on the #99 branch, which is unmerged; this branch is based on 6aa16fc and predates it. Fixing them here would conflict with #99. The approver's fourth finding (#98 listed as open) is the same case. The older citations in DESIGN-DECISIONS.md and the 2026-07-22 specs are pre-existing and recorded as a standing debt in TODO.md by #99, with the reason they are not a find-and-replace: in two of them the dead pointer IS the stated motivation source. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…D-043 row #99 merged as 42790aa and touched the same two docs. Only docs/ROADMAP.md conflicted, on one table row, and it was a genuine merge rather than a pick-one: - main (#99) had the corrected opening "**merged** as #98" but the stale body "PR-1 is cleared to start". - ours (#100) had "PR-1 done" plus the full PR-2 entry requirement, but the superseded "(2026-07-24, PR #98)" opening. Resolved to main's phrasing plus our body, so neither side's correction is lost. Also fixed two statements #99 set that its own merge made stale: the roadmap said "One PR is open (#99…)" and described #99 under Open PRs. #99 is merged and #100 is the open one. Leaving that would have produced a roadmap wrong about repo state — the exact defect #99 existed to fix, and one an approver has now flagged twice. The spec auto-merged cleanly; verified by content rather than trusting the merge that both sides survived — #99's three separate M1/M2/M3 ledger rows, its removal of the untracked-scratch citation, and its "Scope of Round 2" paragraph, alongside #100's §4.1 package constraint, PR-2 entry requirement, stack-frame delta and packaging-guard record. Post-merge verification: ./gradlew clean check jar runnerJar green, 326 tests / 0 failures / 0 errors, and 6 core class entries in each of the two shipped jars — the guard's own subject, checked directly rather than inferred from the build passing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
What
Design spec for DD-043 — extending Basquin to GraalVM-native, reactive Quarkus applications. Design only; no product code.
Every existing target (JPetStore, JSPWiki, Roller) shares two properties the tool has silently assumed are universal, and a native reactive app violates both:
-javaagent, valve, JVMTI)Targets:
rest-villainsandrest-heroesfromquarkusio/quarkus-super-heroes, each built JVM-mode and native — a 2x2 where every cell isolates one variable from its neighbours.rest-villains(blocking) is a throwaway control that exists purely so a wrong number is localisable.Why it preserves closure's thesis
docs/LOCKFREE-LOAD-DESIGN.md:16states the thesis; the operative property is that the app's source is never modified. Runtime attachment was the mechanism, not the thesis. Build-time injection goes through a Maven core extension (-Dmaven.ext.class.path) that mutates the in-memory project model — the build-time analogue of the operator'sCATALINA_OPTSinjection. No file in the app tree is created or changed.Review history
The first draft was reviewed adversarially (
docs/superpowers/specs/reviews/2026-07-24-dd043-fable-review.md, committed here): 2 blockers, 8 majors, 6 minors, 3 nits. All blockers and majors are addressed in the second commit. The two blockers were real and would have failed in the first ten minutes:./mvnwsupplies Maven, not a JDK. Host is JDK 17;rest-heroespinsmaven.compiler.release=25(verified against the pinned pom).javacfails before augmentation, andcontainer-build=trueonly containerisesnative-image. The whole Maven build now runs in a JDK-25 container.BlockedThreadCheckeris log-only — no callback, metric or API. The headline reactive invariant had no path from signal to driver, and this repo has rejected log scraping twice (DD-019, DD-040). Replaced with an extension-owned watchdog measuringrunOnContextscheduling delay.The majors were largely places the spec claimed continuity it had not earned — most notably that "the DD-040 channel transplants unchanged" was false, since DD-040's load-bearing element is the poll waiting on
ITERATION_LOCK, the very lock this design removes.Notable design consequences
ITERATION_LOCKtoo, so it could not overlap a driver iteration. Without the lock, probes and post-response reactive work land inside measurement windows — so the boundary keeps an in-flight counter and taints such samplesUNMEASURED, with a reported taint rate, rather than emitting a number.Scope
Per the review and the project's PR-granularity rule, delivery is six PRs in dependency order (section 9), starting with PR-0: spike evidence only, no product code. Phase 0 genuinely gates the rest — if S1 (offline JaCoCo under AOT) or S4 (does augmentation honour the injected dependency) fails, parts of this design are void and return here before implementation.
Open question
Whether the Apicurio Registry server builds native is unverified (section 8.1) — its
-DcliSkipNativeflag appears to govern the CLI. Target 5 is gated on checking that directly.🤖 Generated with Claude Code
https://claude.ai/code/session_01GBabr7SFhkhqpHpV2tWZZ6