erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases 6 Packages Wiki Activity Actions

fix: prevent launcher exit disposal deadlock

Browse source
This commit is contained in:
Erik 2026-08-18 08:43:29 +02:00
parent 15539a22a6
commit 0a934cf578
9 changed files with 2596 additions and 15 deletions
Download patch file Download diff file Expand all files Collapse all files

538
docs/plans/2026-08-18-release-stabilization.md Normal file
View file

@ -0,0 +1,538 @@
# Release stabilization and human-maintainability campaign
**Status:** PROPOSED — plan recorded; implementation not started
**Created:** 2026-08-18
**Audit baseline:** `15539a22a67f8d915d88f8b1d8126cd55eedda6e`
**Evidence:** [`../reviews/2026-08-17-release-maintainability-audit.md`](../reviews/2026-08-17-release-maintainability-audit.md)
**Findings:** [`../reviews/findings-ledger.md`](../reviews/findings-ledger.md)
**Coverage proof:** [`../reviews/coverage-ledger.md`](../reviews/coverage-ledger.md)
## 1. Goal
Prepare acdream for a responsible public release and for maintenance by human
developers who do not have access to prior AI conversations, private worktrees,
or campaign memory.
The campaign succeeds when a new maintainer can clone the repository, identify
the current architecture and supported release, reproduce the build and test
gate, understand why non-obvious retail behavior exists, and publish or roll
back an authenticated release using repository-owned instructions.
This is a stabilization program, not a rewrite. The existing Runtime/App/
Headless architecture remains the foundation unless a slice proves a specific
boundary is wrong.
## 2. Binding principles
1. **Protect behavior before cleanup.** Establish a deterministic complete gate
before broad refactors, comment cleanup, or file decomposition.
2. **Distill knowledge; do not erase it.** No note, comment, issue history,
diagnostic, capture, or raw artifact is removed until its durable value has
a verified destination.
3. **One current truth.** Stable architecture and release state must not depend
on choosing between README, roadmap, milestone, campaign, memory, or
tool-specific instruction copies.
4. **Separate evidence from contracts.** Source comments explain the current
invariant. Dated research records preserve investigation history. Raw
captures live in an explicit artifact tier.
5. **No count-only gates.** A test total is meaningful only when the report says
which hermetic, installed-DAT, live, visual, manual, and diagnostic lanes
actually ran.
6. **Bound every external interaction.** Process waits, network operations,
test runs, and release steps require timeouts, cancellation, and diagnostic
artifacts on failure.
7. **Small reversible slices.** Each slice gets focused tests, a complete gate,
a reviewable commit, a rollback description, and a plan-ledger update.
8. **No opportunistic feature work.** New gameplay features wait unless needed
to prove or repair a release blocker.
## 3. Knowledge-preservation protocol
Every cleanup candidate is classified before it moves:
| Class | Durable value | Destination |
|---|---|---|
| Current invariant | Required behavior, ordering, ownership, threading, or retail rule | Short source comment and/or maintained architecture contract |
| Decision rationale | Alternatives considered, failed attempts, tradeoffs, gate outcome | Dated decision/research record linked from the current contract |
| Reproducible evidence | Minimal fixture, retail symbol/address, script, checksum, expected result | Versioned fixture/research record in Git |
| Raw evidence | Large logs, captures, Ghidra state, screenshots, dumps | Approved versioned artifact store with manifest, hash, provenance, and retention policy |
| Superseded or incorrect claim | Historically useful but no longer operative | Marked `SUPERSEDED` with successor link; archive after references are migrated |
| Duplication/noise | Repeats a preserved fact and adds no independent evidence | Delete only after destination/link validation |
Before deleting or rewriting historical material, all of these must be true:
- its current invariant is recorded at the owning code or architecture seam;
- useful retail provenance, failed approaches, and acceptance evidence remain
searchable under stable identifiers;
- inbound links and source comments point at the surviving destination;
- any raw artifact has an approved distribution, privacy, and licensing status;
- the replacement was reviewed by someone other than its author;
- the complete gate passes after the move.
Git history alone is not the preservation mechanism. History may later be
rewritten to remove large or legally restricted artifacts.
## 4. Campaign dependency map
```text
R0 baseline/authority
-> R1 launcher deadlock
-> R2 reproducible complete gate
-> R3 truthful test lanes
-> R5 documentation authority
-> R6 dead surfaces and tools
-> R7 plugin/config hardening
-> R8 bounded decomposition
-> R10 release candidate
R4 licence/provenance/release governance ---------------------> R10
R9 artifact migration (depends on R4 decisions) --------------> R10
```
R4 starts in parallel because it requires owner/legal decisions. It blocks a
public release but does not block the technical safety work in R1–R3.
## 5. Slice map
| Slice | Outcome | Depends on | Release blocking |
|---|---|---|---|
| R0 | Baseline and durable campaign authority accepted | — | yes |
| R1 | Launcher shutdown lock inversion fixed and deterministic | R0 | yes |
| R2 | Pinned clean build and complete bounded CI gate | R1 | yes |
| R3 | Test results truthfully distinguish executed, skipped, and diagnostic work | R2 | yes |
| R4 | Licence, provenance, credential disclosure, and release ownership decided | R0; parallel | yes |
| R5 | One current documentation authority; public docs match the product | R2–R3 | yes |
| R6 | Dead presentation/backend/probe surfaces removed; supported tools reproducible | R3–R5 | normally yes |
| R7 | Plugin compatibility/lifetime and diagnostic configuration hardened | R3 | yes for advertised plugin release |
| R8 | Highest-risk giant owners decomposed only at proven seams | R3, R6–R7 | selective |
| R9 | Large/generated research artifacts moved under an approved policy | R4 | yes if repository is distributed |
| R10 | Clean-clone release candidate and rollback rehearsal | all blocking slices | yes |
## 6. R0 — Baseline and authority
### Work
- Review and accept this plan and the six documents under `docs/reviews/`.
- Record the exact starting commit, SDK, package sources, operating systems,
supported release platforms, and current external prerequisites.
- Decide who owns legal/provenance decisions, CI/release credentials, and final
release approval.
- Pause unrelated feature campaigns until R1–R3 establish the safety net.
- Preserve the audit baseline before any cleanup; do not rewrite artifact
history in this slice.
### Exit criteria
- Plan and audit artifacts are tracked in the repository.
- One named owner exists for technical release approval and one for
licensing/provenance approval; the same person may hold both roles.
- The ledger in §17 identifies R1 as the only active implementation slice.
## 7. R1 — Fix the launcher shutdown deadlock first
### Confirmed failure
`LauncherProcessSupervisor.Dispose` currently holds the supervisor `_gate`
while disposing a child. `WindowsSystemChildProcess.Dispose` enters
`System.Diagnostics.Process.Dispose`; concurrently, the process-exit callback
can enter `OnProcessExited` and try to publish state through the same supervisor
gate. The captured wait cycle is:
```text
shutdown: supervisor _gate -> Process internals
exit callback: Process internals -> supervisor _gate
```
The complete solution test process hangs on
`ANullStderrLogPathBehavesExactlyAsBeforeForBothChildProcessKinds`; the
Launcher.Core project can pass alone because the race timing changes.
### Design constraints
- Never call child/process operations that may wait, dispose, raise callbacks,
or execute external code while holding `_gate`.
- Under `_gate`, make only the minimal state transition and snapshot the exact
children/work to retire.
- Perform unsubscribe, stop, kill, wait, and dispose work outside `_gate`.
- Exit callback and explicit disposal must converge idempotently regardless of
which arrives first.
- Preserve exact terminal-state ordering, status publication, graceful-stop
behavior, and child ownership; do not solve the hang by dropping callbacks.
- A failed cleanup must remain observable without starving later cleanup.
### Required tests
- Add a barrier-controlled race test that deterministically pauses the child
exit callback while disposal begins. Do not use `Thread.Sleep` as the oracle.
- Cover exit-before-dispose, dispose-before-exit, simultaneous exit/dispose,
repeated dispose, stop timeout/kill fallback, and callback failure.
- Assert one terminal publication, no resurrection, no orphan child, no held
supervisor lock during process disposal, and bounded completion.
- Run the focused race test repeatedly after its deterministic single pass.
- Run all Launcher.Core tests.
- Run the complete Release solution twice in fresh processes under a documented
timeout and retain hang dumps if either run fails to terminate.
### Exit criteria
- No process/child disposal occurs under the supervisor gate.
- The deterministic race test fails against the baseline mechanism and passes
against the fix.
- Two complete bounded solution runs finish with zero failures.
- F-009 and T-001 receive exact fix commit and gate evidence.
## 8. R2 — Reproducible build and complete CI gate
### Work
- Pin the accepted .NET 10 SDK feature band in `global.json`.
- Centralize common compiler/analyzer settings and package versions; enable
locked restore for release/CI.
- Eliminate all 26 clean-rebuild test warnings or make a narrowly justified,
centrally documented exception fail-safe.
- Build every supported product and every supported tool from a clean checkout.
- Make CI run the complete solution, not only portability subsets. Give each
project/process a timeout and collect test logs plus managed dumps on hangs.
- Preserve focused Windows/Linux portability lanes, but do not represent them
as the complete gate.
- Record restore sources, SDK/runtime, RID, commit, executed/skipped counts, and
artifact hashes in every release report.
### Exit criteria
- A clean clone restores and builds with the pinned toolchain and no warnings.
- Complete CI runs every default release test project and fails on timeout.
- The launcher hang cannot silently consume the CI job indefinitely.
- Package resolution and the gate command are reproducible from repository
instructions alone.
## 9. R3 — Make test reporting truthful
### Required lanes
1. **Hermetic release lane:** default CI; unavailable local data is never a
silent passing return.
2. **Installed-DAT/prepared-package lane:** explicit prerequisites and per-suite
skip identity; result published separately.
3. **Live/connected/visual/listening lane:** operator-owned, dated evidence;
never counted as ordinary unit coverage.
4. **Diagnostic/manual lane:** probes, dumps, fixture generators, and
characterization programs invoked explicitly outside default test totals.
### Work
- Replace the 271 asset/environment empty-return tests with explicit lane
requirements, truthful skips, or hermetic fixtures.
- Move or give stable assertions to the 51 output-only diagnostic methods.
- Delete/replace the three confirmed useless entire cases and the tautological
assertions catalogued in the audit.
- Remove the duplicate theory row and make the clean rebuild warning-free.
- Remove or re-home at least 52 tests with the unreachable panel stack.
- Retire temporary source-shape freezes once a semantic architecture/behavior
guard exists; retain whole-tree dependency guards that express real rules.
- Assign and stabilize the seven documented load-sensitive tests. Do not hide
them with generic retries.
- Inject controllable time into double-click and real-time transport tests.
### Exit criteria
- Default test success means every discovered default contract executed.
- Reports give exact reasons and prerequisite identity for every skip.
- Diagnostics and manual generators do not inflate release regression totals.
- No known duplicate row, literal tautology, or permanent empty scaffold remains
in the default suite.
## 10. R4 — Licence, provenance, security, and release ownership
This slice requires explicit project-owner decisions and, where appropriate,
qualified legal review. The implementation agent records evidence but does not
invent a redistribution basis.
### Work
- Select the project licence and establish contributor/code ownership.
- Audit WorldBuilder-derived code, dependency notices, named-retail/decompiler
exports, PDB-derived data, Ghidra databases, captures, images, and DAT-derived
fixtures for provenance and redistribution status.
- Decide which research artifacts may be public, private, regenerated, or
deleted from distributable history.
- Add SECURITY, CONTRIBUTING, changelog/version authority, disclosure and
deletion behavior for plaintext launcher credentials, and a vulnerability
response path.
- Define release artifact contents, supported platforms, SBOM/provenance,
signing/attestation policy, checksums, update manifest generation, rollback,
and release approval.
### Exit criteria
- Publicly distributed source and artifacts have an approved licence and
complete notices/provenance inventory.
- Users are told exactly how credentials are stored and removed.
- The launcher updater's production manifest and archives are generated and
verified by a repository-owned release process.
## 11. R5 — One current documentation authority
### Work
- Correct public README claims to the actual Vulkan-only client and retained UI.
- Make `docs/README.md` stable navigation plus a generated current-status block
sourced from one structured milestone/release ledger.
- Keep architecture documents limited to durable boundaries, ownership,
threading/lifetime, and data flow. Move commit/test-count/rollback chronology
to dated closeout records.
- Replace duplicated `AGENTS.md`/`CLAUDE.md` product truth with one maintained
tool-neutral source and generated thin adapters; fail CI on drift.
- Mark every memory/plan/spec as current, active, superseded, or historical.
- Normalize active issue/divergence indexes and validate their IDs, statuses,
paths, and links mechanically.
- Apply the knowledge-preservation protocol before shortening any campaign or
issue record.
### Exit criteria
- A new maintainer receives the same current answer from README, documentation
map, architecture, milestone/status ledger, and agent instructions.
- No current authority links missing private `claude-memory`, absent skills, or
developer-local paths.
- Historical records remain searchable but cannot override current truth.
## 12. R6 — Dead surfaces, diagnostics, and tools
### Work
- Decide and then remove or explicitly support the unreachable
`IPanelRenderer`/old panel stack and its tests.
- Remove stale OpenGL/framebuffer/ImGui apparatus and failed temporary-cleanup
markers from shipping assemblies after preserving useful evidence.
- Stop including the smoke plugin in release output by default.
- Classify every tool/script as supported, diagnostic, research-only, or
archived. Repair the five broken C# tools chosen as supported; remove
developer-home/old-worktree paths and document exact prerequisites.
- Centralize environment/diagnostic configuration at composition roots.
### Exit criteria
- Shipping assemblies and package contents contain no abandoned presentation
backend or sample plugin by accident.
- Every supported tool builds from the pinned clean checkout.
- Research-only tools are clearly invoked outside the release build.
## 13. R7 — Plugin and configuration contracts
### Work
- Enforce supported plugin API versions before loading code.
- Implement manifest dependencies or remove the unsupported promise.
- Publish/version `AcDream.Plugin.Abstractions` if plugins are advertised.
- Report registration cleanup and callback failures without preventing
best-effort teardown; prove collectible load-context release under failures.
- Replace absent/null allow-list ambiguity with one explicit production default.
- Replace direct hot-path environment reads/process-static mutable diagnostics
with immutable session-scoped configuration and typed sinks.
### Exit criteria
- An incompatible plugin fails before activation with an actionable message.
- Disable/dispose reports all cleanup failures and cannot silently retain host
registrations.
- Graphical and headless hosts have the same documented plugin/config default.
## 14. R8 — Bounded structural decomposition
This slice begins only after R1–R3. File size alone does not authorize a split.
### Priority candidates
- `RuntimeSetPositionState`
- `TransitionTypes`
- `RetailUiRuntime`
- `LiveEntityRuntime`
- `WorldSession`
- `WbDrawDispatcher`
### Rules
- Identify one ownership/lifetime or pure-algorithm seam at a time.
- Preserve a single state owner; do not replace a large class with mirrored
mutable state or a service graph of aliases.
- Prefer partial-file navigation when a state machine must remain one owner.
- Establish behavior/sabotage tests before extraction and remove corresponding
temporary source-text freezes afterward.
- Replace Chorizite/GL vocabulary in prepared-content DTOs with versioned
acdream-owned semantics at a separately reviewed boundary.
### Exit criteria
- Each extraction reduces change coupling or improves ownership clarity; line
count reduction alone is not success.
- Runtime behavior, retail evidence, allocations, and teardown ledgers remain
equivalent under the relevant focused and complete gates.
## 15. R9 — Repository artifact migration
### Work
- Inventory the approximately 575 MiB of Ghidra state, 299 MiB research tree,
and 151 MiB tracked logs by provenance, sensitivity, reproducibility, and
ongoing value.
- Keep compact fixtures, scripts, tool versions, summaries, and checksums in
Git. Move approved raw bundles to a versioned artifact store.
- Add a manifest/bootstrap command that verifies artifact identity.
- Define retention, redaction, access, and backup policy.
- Treat Git-history rewriting as a separately approved migration with backup,
contributor coordination, remote replacement, and verification. Never do it
as an incidental cleanup command.
### Exit criteria
- A normal clone contains what build/test/maintenance requires without opaque
generated databases or raw logs.
- Authorized researchers can retrieve exact approved evidence by manifest and
hash.
- Restricted or non-redistributable material is absent from public history.
## 16. R10 — Release-candidate gate
From a fresh clone on every supported release platform:
- restore with the pinned, locked toolchain;
- build every shipped product and supported tool with zero warnings;
- run the complete hermetic test lane with no failures, silent no-ops, hangs,
or generic retries;
- run and report the applicable DAT, connected, visual/listening, updater,
installer, graceful-shutdown, and rollback gates;
- generate versioned per-RID packages, plugin abstraction package if supported,
SBOM/provenance/checksums, and updater manifest;
- install/update/rollback using only public release instructions;
- verify package contents contain no credentials, developer paths, smoke
plugin, raw probes, or unapproved research artifacts;
- obtain technical and licensing/provenance approval.
Public release remains **NO-GO** until every blocking slice is closed.
## 17. Cross-session execution ledger
This table is the resume authority. Update it in the same commit as every slice
checkpoint. Do not infer status from chat history.
| Slice | Status | Commit(s) | Evidence/gates | Exact next action |
|---|---|---|---|---|
| R0 | PLAN ACCEPTED; tracking/owner assignment pending | — | Audit complete at `15539a22`; user started R1 | Track plan+audit artifacts; assign R4 owner |
| R1 | IMPLEMENTED + REQUIRED GATES PASS; review/commit pending | — | 2026-08-18 checkpoint below; F-009/T-001 resolved in working tree | Review diff, then commit the coherent R1 checkpoint |
| R2 | NOT STARTED; technically unblocked after R1 review/commit | — | F-010/F-014/F-019 | Start only after R1 checkpoint is accepted |
| R3 | NOT STARTED | — | T-002–T-016; F-015/F-021/F-022/F-030 | Wait for R2 |
| R4 | NOT STARTED; may run parallel | — | F-001/F-026/F-031/F-034 | Assign owner/legal provenance decision path |
| R5 | NOT STARTED | — | F-002/F-003/F-006/F-007/F-011/F-020/F-027/F-029/F-032 | Wait for R2–R3 |
| R6 | NOT STARTED | — | F-004/F-005/F-012/F-016/F-023/F-028 | Wait for R3/R5 |
| R7 | NOT STARTED | — | F-017/F-018/F-024/F-025 | Wait for R3 |
| R8 | NOT STARTED | — | F-008/F-013/F-033 | Wait for R1–R3 and cleanup decisions |
| R9 | NOT STARTED | — | F-034 plus R4 provenance decisions | Wait for R4 |
| R10 | NOT STARTED | — | All blocking findings | Wait for blocking slices |
### R1 implementation checkpoint — 2026-08-18
**Working-tree base:** `15539a22a67f8d915d88f8b1d8126cd55eedda6e`
**Commit:** pending; do not describe this checkpoint as merged or durable until
the source, test, plan, and audit records are committed together.
Implementation:
- `LauncherProcessSupervisor.Dispose` now transfers `_process` ownership to a
local and clears the field under `_gate`, then performs stop, event removal,
and child disposal outside the gate.
- Public `Stop` and disposal share one `StopProcess` implementation, preserving
graceful-stop, close-window, timeout, kill, and post-kill observation order.
- `OnProcessExited` reads the event sender's optional exit code without holding
`_gate`, then commits the terminal transition through the existing ordered
state publisher. A callback already captured during teardown may therefore
finish instead of forming the supervisor/Process lock cycle.
- `DisposeAllowsAnAlreadyCapturedExitCallbackToComplete` uses explicit barriers:
the fake captures the exit delegate before unsubscription; its disposal
releases the callback and waits for it to return. There are no timing sleeps
in the oracle, and an emergency release keeps failure against the old code
bounded rather than wedging the test host.
Sabotage and focused evidence:
- With the old lock shape temporarily restored and the new test retained, the
test failed with its expected five-second timeout. The fake's cleanup barrier
then released the old cycle, so the test process exited normally.
- With the fix restored, the same test passed in 21 ms.
- The race test passed 25/25 times in fresh `dotnet test` processes.
- All `LauncherProcessSupervisorTests` passed: 22/22.
- Complete Launcher.Core passed under a 180-second hard process bound:
339 passed / 0 skipped / 0 failed in 52 seconds.
Complete-solution evidence:
- Release build completed inside a 300-second bound with 0 warnings / 0 errors
in the evaluated incremental build.
- The exact serialized command was
`dotnet test AcDream.slnx -c Release --no-build --no-restore --nologo -m:1`,
launched in a fresh child process for each run. An outer process watchdog
allowed 900 seconds, killed the complete process tree on expiry, and treated
timeout as failure.
- Run 1: 12 assemblies, 14,748 passed / 77 skipped / 0 failed, 1:28.822,
bounded exit code 0.
- Run 2: 12 assemblies, 14,748 passed / 77 skipped / 0 failed, 1:30.241,
bounded exit code 0.
Adjacent evidence, deliberately not folded into R1:
- One default-parallel whole-solution run terminated normally in 58.148
seconds—important negative evidence for the former hang—but failed one
`AcDream.Launcher.Tests` Avalonia headless cleanup because a compositor was
accessed from a non-owning thread. The Launcher test project then passed
67/67 alone. R1 makes no Avalonia changes; R2/R3 must decide the supported CI
scheduling and ownership of that pre-existing parallel-run failure.
- The known duplicate Core theory row and 77 skip classifications remain
unchanged and belong to R3. R1 does not use their headline count as proof of
test quality.
Changed implementation/test files:
- `src/AcDream.Launcher.Core/Launching/LauncherProcessSupervisor.cs`
- `tests/AcDream.Launcher.Core.Tests/Launching/LauncherProcessSupervisorTests.cs`
Rollback before commit is the exact reverse of those two file diffs. After a
future commit, record its hash here and use `git revert <R1-commit>` rather than
rewriting history.
## 18. Session start protocol
Every implementation session begins by:
1. reading this plan, the executive audit, and the findings for the active
slice;
2. running `git status --short`, `git rev-parse HEAD`, and checking the ledger's
recorded commit against the working tree;
3. reading all files/tests named by the active finding before editing;
4. confirming there is no overlapping uncommitted user work;
5. restating the bounded slice outcome and gates in the session update;
6. working only the first non-blocked active slice unless the plan explicitly
allows parallel work.
## 19. Session handoff protocol
Before ending any session, record in §17 or a linked dated closeout:
- exact commit/worktree state and every file changed;
- decisions made and alternatives rejected;
- invariant/evidence destinations for anything removed;
- exact commands, pass/fail/skip counts, timeouts, and artifact paths;
- review findings and whether they were closed;
- remaining risks, blockers, and user/legal decisions;
- rollback command or precise reversal procedure;
- one exact next action that can be started without chat context.
A slice is not `DONE` because its code compiles or a focused test passes. It is
done only when its exit criteria, complete required gate, evidence update,
review, and cross-session ledger entry are all complete.
## 20. Immediate next action
Review the R1 diff and checkpoint evidence, then commit the source, regression
test, plan, and audit updates together if accepted. After that, begin R2's
pinned complete gate; assign the R4 decision owner in parallel. Do not begin
bulk comment, artifact, or giant-file cleanup first.

191
docs/reviews/2026-08-17-release-maintainability-audit.md Normal file
View file

@ -0,0 +1,191 @@
# acdream release maintainability audit
Status: **complete — public-release no-go**
Audit baseline: commit `15539a22a67f8d915d88f8b1d8126cd55eedda6e`
Baseline tree: `4de2634ee7f528fe149fc0817247988f8b6bf67f`
Started: 2026-08-17
Completed: 2026-08-18
## Objective
Determine whether a human development team can safely maintain and release
acdream without relying on undocumented AI conversations, campaign history, or
machine-local context. This is a review-only audit. It does not modify owned
source, tests, configuration, or existing documentation.
## Deliverables
- [`coverage-ledger.md`](coverage-ledger.md) records the inspected scope and
completion evidence.
- [`findings-ledger.md`](findings-ledger.md) is the deduplicated evidence-backed
finding register.
- [`architecture-documentation-assessment.md`](architecture-documentation-assessment.md)
compares the documented ownership model with the repository and evaluates
the documentation hierarchy.
- [`comment-reference-inventory.md`](comment-reference-inventory.md) classifies
comments and historical/external references by maintainability value.
- [`test-quality-audit.md`](test-quality-audit.md) records the complete
test-source review and every test-quality exception worth acting on.
## Review boundaries
Deep review covers all source-controlled code under `src/` and `tools/`, all
source-controlled test code under `tests/`, the public entry points, active
architecture and planning authorities, and their links into research/history.
Generated binaries, build output, retail data, and external repositories are
not judged as owned code. Their discoverability, licensing, reproducibility,
and use as required evidence are in scope.
## Evidence and classification rules
Each finding records an exact path/line or a reproducible repository command,
impact, confidence, and recommended direction. Severity means:
- **P0 — release blocker:** responsible public release should not proceed.
- **P1 — high:** likely to mislead maintainers or make important changes unsafe.
- **P2 — medium:** material recurring maintenance cost or localized false confidence.
- **P3 — low:** cleanup that improves clarity but does not materially impair work.
Tests are not called “useless” merely because they are small or repetitive.
That label is reserved for a test that cannot detect a meaningful regression.
Other low-signal tests are classified as misleading, redundant, tautological,
implementation-coupled, obsolete, weakly asserted, or misplaced.
## Executive conclusion
acdream is not architecturally hopeless or uniformly “AI spaghetti.” Its
assembly graph has meaningful boundaries, the Runtime/App split is real, and
several difficult lifetime, updater, transport, and retail-fidelity mechanisms
have strong tests. A human team could maintain it after a focused stabilization
program.
It is not ready for a responsible public release at this baseline. The audit
records 34 findings: **four P0, 25 P1, and five P2**. The four blockers are:
1. no project licence and unresolved redistribution/provenance for committed
reverse-engineering artifacts (F-001);
2. public setup documentation advertising the deleted OpenGL/ImGui stack
instead of the shipping Vulkan client (F-002);
3. a reproducible launcher supervisor/process-exit lock inversion that hangs
the official complete test process and can affect production disposal
(F-009); and
4. CI does not execute the complete solution gate and omits Core plus most App
tests (F-014).
The dominant maintainability risk is not one bad algorithm. It is the absence
of a trustworthy release truth: current state is duplicated across enormous
campaign journals, comments depend on missing/private context, tools and SDK
resolution are machine-specific, and headline test counts include hundreds of
contracts that did not execute.
## What is already solid
- Production Release compilation succeeds with no production-code warnings.
- The project-reference graph is acyclic; Platform, Headless, Runtime, and
Plugin.Abstractions satisfy their most important declared dependency rules.
- Per-project execution produced 14,747 passes and 77 reported skips. That is a
substantial safety net even after discounting the misleading cases.
- Updater extraction, hashing, transaction/rollback, path safety, launcher
credential redaction, generation gating, and teardown ownership receive
unusually strong adversarial coverage.
- The primary named-retail algorithm corpus is committed and searchable.
- A current NuGet vulnerability lookup reported no known vulnerable direct or
transitive packages, and the repository-wide common token/private-key scan
found no matching secret prefix. These are point-in-time positives, not
substitutes for pinned restore or artifact review.
## Test verdict
The test corpus is valuable but its headline number is not a reliable release
claim today. The review found:
- three entire cases with no meaningful runtime contract (the literal smoke
tautology, the compile-only camera-interface test, and the empty skipped PVS
scaffold), plus useless tautological assertions inside otherwise useful
tests;
- 51 output-only diagnostic methods in the default suite;
- 271 asset/environment-gated facts that report success without exercising
their named contract;
- at least 52 tests for an unreachable deleted presentation stack;
- 30 App source-reading files containing 560 literal-fragment assertions and
86 ordering-helper calls, including an explicitly temporary architecture
freeze;
- one duplicate theory row silently removed during discovery, 26 clean-rebuild
warnings, seven open load-sensitive tests, and real-time sleeps in timing
contracts; and
- a precise table of contract-like test names whose assertions do not establish
the behavior in their name.
The exact method/file catalog and classification is in
[`test-quality-audit.md`](test-quality-audit.md). Diagnostics are not called
useless merely because they print useful evidence; they are classified as
misplaced unless they also have a stable oracle.
## Recommended stabilization order
### 1. Establish legal and release authority
Resolve project and research-artifact licensing/provenance; decide which
decompiler/Ghidra/capture artifacts may be distributed; add the public licence,
NOTICE coverage, security policy, contribution policy, changelog/version
authority, credential-storage disclosure, and release runbook.
### 2. Make one bounded gate truthful
Fix the launcher deadlock with a deterministic regression test. Pin the .NET
SDK and restore graph, make a clean warning-free build the gate, and run the
complete solution in CI with per-project/process timeouts and hang artifacts.
Turn unavailable DAT/live/environment prerequisites into explicit lanes and
reported skips/failures; remove output-only apparatus from the release count;
own the seven flakes rather than retrying them generically.
### 3. Repair current documentation before historical cleanup
Correct the Vulkan/UI/readiness README, generate current status from one
ledger, synchronize or generate the agent instruction wrappers, retire stale
“current truth” memory, and separate durable architecture from campaign
history. Then validate active links and normalize the issue/divergence indexes.
### 4. Remove abandoned and ambiguous shipping surfaces
Delete or explicitly support the dead panel stack and stale OpenGL/probe
apparatus; stop embedding the smoke plugin in release output; enforce plugin
API/dependency/lifecycle contracts; centralize diagnostic configuration. Split
giant owners only at proven ownership/lifetime seams, preserving retail
algorithms and sabotage-verified behavior.
### 5. Make the repository reproducible for another human
Repair or archive the five broken tool projects, publish exact reference
bootstrap revisions/licences, remove developer-home paths, and move opaque
Ghidra projects/raw logs to an approved versioned artifact system. Keep compact
fixtures, checksums, scripts, and evidence summaries in Git.
## Scope limitations
This is a maintainability/release review, not a proof that all retail behavior
is correct and not legal advice. Installed-DAT, live-server, visual, listening,
physical-Linux, and manual generator gates were classified from their source
and recorded evidence but not all re-executed. Package vulnerability results
reflect the configured sources on 2026-08-18. The launcher hang was reproduced
and captured; the seven other documented flakes all passed in the single
per-project audit baseline and remain findings because their own issue records
show recurrence.
The audit changed only the new records under `docs/reviews/`; it made no edits
to owned source, tests, configuration, existing documentation, commits, or
external systems.
## Post-audit stabilization update — 2026-08-18
R1 has been implemented and fully gated in the current uncommitted working
tree. The launcher child is detached under the supervisor gate and stopped,
unsubscribed, and disposed outside it; a deterministic captured-callback race
test fails against the old mechanism and passes against the fix. Launcher.Core
passes 339/339, and two fresh serialized complete Release runs each finish
inside a 900-second hard bound with 14,748 passes / 77 skips / 0 failures.
This resolves F-009 technically but does not change the baseline audit's
historical count until the coherent R1 checkpoint is reviewed and committed.
The public-release decision remains no-go because F-001, F-002, F-014, and the
remaining high-priority release-governance findings are untouched.

172
docs/reviews/architecture-documentation-assessment.md Normal file
View file

@ -0,0 +1,172 @@
# Architecture and documentation assessment
Status: **complete at the recorded baseline**.
## Declared authority order
The repository declares milestones authoritative for the active outcome, the
roadmap authoritative for ordering, issues authoritative per defect, and
architecture documents authoritative for implementation shape. Research and
dated plans are historical evidence. This is a reasonable model, but the
current documents duplicate enough mutable state that the order does not
resolve contradictions reliably.
## Initial architecture-to-repository observations
| Subject | Documented current contract | Repository evidence | Initial assessment |
|---|---|---|---|
| Graphics backend | Vulkan only; OpenGL deleted | App references `Silk.NET.Vulkan`; Vulkan/RHI source exists | architecture matches code; public README is stale |
| Gameplay UI | one retained stack; ImGui deleted | no `AcDream.UI.ImGui` project; retained UI in App | architecture matches code; README and some rule wording are stale |
| Runtime boundary | Runtime may depend on Core, Core.Net, Content, Platform, Plugin.Abstractions; never App/UI/backend | project/source graph matches; dependency guards pass | confirmed |
| Headless boundary | Headless directly references Runtime only | project graph and executed loaded-assembly guards match | confirmed; operability exceptions recorded separately |
| Platform boundary | Platform has zero references | project graph and complete two-file source review match | confirmed |
| Reference workflow | six vendored reference projects | only uninitialized WorldBuilder gitlink exists | documented process is not reproducible |
| GameWindow shell | architecture checkpoint says 1,622 lines | current file is 1,860 lines | still primarily a composition/callback shell, but checkpoint and navigability claim have drifted |
| UI abstraction | backend-neutral panel/input seam | directly references Runtime and Silk input; old panel renderer has no implementation | boundary name and owned concerns no longer match |
| Presentation stacks | one retained retail UI | retained retail UI ships, but unreachable `IPanelRenderer`/ImGui-era panels remain compiled and tested | dead second design obscures the shipping stack |
| Build authority | .NET 10 solution with warnings-as-errors | no pinned SDK/locked restore/central build settings; policy copied inconsistently across 44 projects | a commit is not a reproducible toolchain definition |
| Issue authority | one authoritative status per defect | 380 headings/378 IDs, about two-thirds closed history, 29 broken links | tactical queue is also an archive and research journal |
## Documentation-system concerns under review
1. `README.md` currently contradicts the architecture on the most basic user
prerequisite: Vulkan versus OpenGL and the existence of ImGui.
2. `docs/README.md` is presented as a current-state guard but predates completed
work linked from the same page.
3. `AGENTS.md` is 1,615 lines and embeds a long commit-by-commit status ledger,
rollback commands, local credentials, machine paths, tool instructions,
architectural rules, and reference tutorials. Its normative rules are hard
to distinguish from stale operational history.
4. The architecture document similarly combines stable dependency rules with
current phase state, historical campaign checkpoints, test totals, exact
commits, and future target designs.
5. Required links into `claude-memory/` and `.claude/skills/` are absent from
the baseline worktree.
6. The current milestone, roadmap, and world-interaction plan each preserve a
different pre-closeout state even though the plan itself has a later closeout
section. Authority precedence cannot resolve a contradiction inside every
claimed authority.
7. The mandatory WorldBuilder inventory still gives OpenGL and ImGui design
direction after both were deleted.
8. The issue ledger's status convention is not mechanically enforceable: two
duplicate IDs, nine missing/nonstandard status blocks, and broken links sit
inside a 19,073-line file every session is instructed to scan.
9. The active-document link check found three broken links even before the
historical corpus: the missing `claude-memory/MEMORY.md` map and two missing
Phase O documents linked from roadmap line 505.
10. The retail-divergence authority has correct headline row counts but is not
maintainable as a review surface: 809 KB in 525 physical lines, 243 lines
longer than 1,000 characters, a 45,240-character heading, and 11 unique
unresolved source/test paths among 843 path citations. Its title is six
weeks behind its own latest entries.
11. The approved plugin architecture reads as an implemented contract but is
mostly aspirational. The manifest accepts every positive API version,
declared dependencies are unused, the promised abstractions package has no
pack/version metadata, and callback-fault/hot-reload behavior differs from
the design.
12. The launcher consumes a production GitHub Release manifest, but the
repository contains no production release workflow or manifest generator,
no tags/central product version, and no contribution, security, changelog,
or code-ownership documents.
13. `AGENTS.md` explicitly claims to be synchronized from `CLAUDE.md`, but the
files differ by 255 insertions and 27 deletions. The stale copy restarts a
completed interaction slice and disagrees about reference count, retired
AC2D status, and streaming-radius configuration. This makes the executing
assistant, rather than the repository commit, select project truth.
14. Tracked `memory/` documents labeled “current truth” still prescribe ImGui
frame submission, the deleted UI.ImGui project, OpenGL bindless capability
gates, and resuming the now-complete interaction Slice 4. Because root
instructions route work into these files, they are active contradictions,
not harmless archived notes.
15. A repository-wide scan of all 1,014 tracked Markdown files found 397
broken relative-link occurrences, representing 357 distinct file/target
pairs. Historical records account for most of the debt: 197 pairs in
`docs/research` and 130 in `docs/superpowers`. The current authority layer
has a much smaller but material set: `docs/README.md` points at the absent
`claude-memory/MEMORY.md`; the roadmap points at two absent Phase O
records; one old handoff points at a deleted VFX test; and one tracked
memory index points at a missing phase audit. `docs/ISSUES.md` contributes
25 distinct broken Markdown-link pairs. Historical link rot should be
repaired or frozen by support tier, but it is not equivalent in severity
to a broken current-authority link.
16. The dated plans/specs/research system is now 80 top-level plans, 100
`superpowers/plans`, 96 specs, and 700 research Markdown files. Recent
campaign plans do generally put status near the top, which is good, but
several preserve contradictory status prose in-place (for example an
`IN FLIGHT` label followed immediately by a later `CAMPAIGN CLOSED`
clause). There is no generated active/archive index or schema validation;
readers must interpret narrative amendments to decide which paragraph is
operative.
## Source-structure census
The project-reference graph is acyclic and the primary declared assembly
boundaries are present: Platform has no references, Headless references Runtime
only, and Runtime has no App/UI/backend reference. The maintainability risk is
inside several of those correct graph boxes. Source hotspots are highly
concentrated: App has 37 files above 1,000 lines, Runtime 20, Core 13, and the
five largest owned files range from 3,754 to 6,345 lines. Campaign/history
language also remains densest in those owners (2,349 rough source occurrences
in App, 698 in Core, and 491 in Runtime).
| Assembly | Primary assessment at this checkpoint |
|---|---|
| `AcDream.App` | Layer role is valid, but composition/render/UI/network sidecars remain too concentrated; dead presentation/backend seams, source-shape tests, static diagnostics, and the unconditional smoke-plugin publish make the shipping boundary hard to reason about. |
| `AcDream.Runtime` | Project boundary is real and well guarded, but `RuntimeSetPositionState` (6,283 lines) and several 2,500–3,200-line gameplay/entity owners combine algorithms, state machines, ledgers, and diagnostics beyond a human-scale change unit. |
| `AcDream.Core` | Contains valuable faithful ports with strong oracle citations, but giant physics units, process-static diagnostics, external-type leakage, and campaign/issue narrative obscure the stable algorithms. |
| `AcDream.Core.Net` | Protocol/session ownership is coherent; `WorldSession` remains a 3,754-line transport/lifecycle owner with dense historical comments, direct environment probes, and synchronous shutdown risk that warrants focused refactoring only after behavior is pinned. |
| `AcDream.Content` | Prepared-content boundary is conceptually clean, but public mesh DTOs still expose Chorizite types and GL-ABI vocabulary despite a Vulkan-only consumer. |
| `AcDream.Headless` | Presentation dependency guard is strong. Silent plugin callback failure and process-static/config conventions weaken operability; three files exceed 1,000 lines. |
| `AcDream.Launcher.Core` / `Launcher` | Update/install security and transaction design are unusually thorough, but the full-suite lock inversion is a release blocker and there is no owned production release publisher. Several view-model/orchestrator/update owners are oversized. |
| `AcDream.Platform` | Small, cohesive, and dependency-free as documented. |
| `AcDream.Plugin.Abstractions` | BCL-only and small, but not yet a versioned/published compatibility contract; the approved design substantially overstates the implemented surface. |
| `AcDream.UI.Abstractions` | Input/keybinding pieces remain useful; the project also retains a dead ImGui-era panel stack and directly depends on Runtime/Core, so its name no longer describes one neutral abstraction layer. |
| Bake / CLI / tools | Bake's transaction boundary is understandable though its runner is 1,067 lines. CLI and most forensic tools are campaign utilities without a support tier; many are excluded from the solution or require developer-local paths. |
## Recommended documentation information architecture
1. Keep `README.md` public and release-oriented: supported platforms/backend,
install/run path, actual maturity, credential-storage disclosure, licence,
and links to release/security/contribution documents. Do not duplicate
campaign state or rolling test totals there.
2. Make `docs/README.md` a stable navigation page plus a generated, dated
status block sourced from one structured milestone ledger. A CI check should
fail when the block, project graph, backend, links, or reported gate command
disagrees with the repository.
3. Restrict architecture documents to durable component ownership, dependency
rules, threading/lifetime contracts, data flow, and intentional seams. Move
commits, rollback recipes, test counts, and campaign closeout narrative into
dated decision/closeout records.
4. Replace duplicated `AGENTS.md`/`CLAUDE.md` bodies with one tool-neutral
maintained instruction source and generated thin adapters. Agent-only tool
syntax should not carry product truth.
5. Normalize issues and retail divergences into one record per stable ID with
machine-readable status, owner, current rationale, oracle, live symbol/path,
and review date. Generate compact active indexes; archive amendment history
separately.
6. Treat plans, specs, research, probes, raw captures, and memory as explicit
support tiers: active, superseded, immutable evidence, or external artifact.
Validate relative links only against the promises of each tier and prevent a
historical document from calling itself “current truth.”
7. Add one owned release runbook and automation for pinned clean restore/build,
the complete bounded test matrix, installed-DAT/live/manual lanes, per-RID
packages, manifest/checksum/SBOM/provenance generation, rollback, and public
publication.
## Positive architecture findings
- The project-reference graph is acyclic and the most important assembly
boundaries are real: Platform is dependency-free, Headless references
Runtime only, Runtime does not reference App/UI/backend assemblies, and
Plugin.Abstractions is BCL-only.
- `GameRuntime` ownership, generation gating, teardown ledgers, updater
transaction design, path traversal/hash validation, and credential redaction
have unusually substantial adversarial test coverage.
- The named-retail oracle is committed and searchable, so the core retail-
fidelity workflow does not depend entirely on the missing external reference
checkouts. The weakness is provenance/licensing and surrounding reference
reproducibility, not absence of the primary algorithm corpus.
- Production code builds without warnings at the baseline; all 26 rebuild
warnings are confined to test projects. Current package-source vulnerability
lookup reported no known vulnerable direct or transitive packages, although
the unpinned restore prevents treating that as a permanent commit property.

110
docs/reviews/comment-reference-inventory.md Normal file
View file

@ -0,0 +1,110 @@
# Comment and reference inventory
Status: **complete at the recorded baseline**.
## Classification standard
A durable source comment should make the local invariant, retail behavior, or
non-obvious constraint understandable without requiring a private conversation.
External provenance is useful when it points to a stable, obtainable source.
Campaign chronology is useful in research/decision records but usually not as
the primary explanation inside production code.
Comments will be classified as:
- **Durable invariant/provenance:** explains why/order/math and cites a stable
retail symbol, public document, or owned design contract.
- **Useful but context-dependent:** technically valuable, but requires an issue,
campaign, commit, or unavailable reference to understand.
- **Historical residue:** describes how an older implementation changed rather
than what the current code guarantees.
- **Diagnostic/apparatus residue:** names a temporary probe, rejected attempt,
manual gate, or one-off capture in maintained code.
- **Misleading/stale:** contradicts current code or an active authority.
- **Noise:** restates code, uses phase labels as structure, or preserves no
decision a maintainer needs.
## Initial inventory
A Roslyn trivia scan over all 1,325 owned C# files found 37,734 actual comment
trivia nodes, including 9,516 XML-documentation comments. Of those comments,
2,172 contain a campaign/slice/phase/checkpoint or commit-hash reference, 1,016
contain an issue-number reference, 1,049 name an external/reference oracle,
617 contain probe/temporary/workaround/diagnostic language, 143 contain
OpenGL/ImGui or GL-owner terminology, and 16 contain TODO/FIXME/HACK/XXX
markers. Categories overlap and occurrence does not by itself make a comment
bad; the figures identify the review surface without matching strings or
identifiers.
| Area | Comment trivia | Campaign/history | Issue refs | External refs | Old graphics | Probe/workaround |
|---|---:|---:|---:|---:|---:|---:|
| App | 18,009 | 1,215 | 480 | 121 | 107 | 244 |
| Core | 8,723 | 399 | 322 | 452 | 4 | 224 |
| Runtime | 5,813 | 227 | 123 | 112 | 1 | 46 |
| Core.Net | 2,545 | 156 | 37 | 328 | 0 | 23 |
| UI.Abstractions | 899 | 69 | 12 | 11 | 22 | 18 |
| Headless | 534 | 42 | 21 | 13 | 0 | 29 |
| All remaining source/tools | 1,211 | 64 | 21 | 12 | 9 | 33 |
High-signal examples already queued for review include:
- `src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs`, whose comments mix retail
symbols and durable draw invariants with issue numbers, campaign slice
migrations, deleted GL-arm history, and named throwaway probes across a
3,773-line class.
- `src/AcDream.Headless/Policies/HeadlessBotPolicy.cs`, whose production XML
documentation contains campaign contracts, temporary evidence gathering,
an open issue, and disabled diagnostic behavior.
- Tool entry points that identify themselves primarily by the issue/campaign
that created them, without a maintained support/archival classification.
- `src/AcDream.Core.Net/WorldSession.cs` contains 69 campaign-history comments
and 47 external-reference comments inside a 3,754-line transport/session
owner; `VendorUiController.cs` contains 71 campaign-history comments.
- `AcDream.UI.Abstractions` still documents ImGui capture and panel behavior in
interfaces used by the retained client after the ImGui backend was deleted.
The exact project counts and confirmed high-risk classes above are the final
inventory. It intentionally does not propose mass deletion: durable retail
symbols and local invariants are valuable, while stale backend claims,
personal paths, unavailable-reference paths, campaign narration, and failed
temporary-cleanup markers require targeted correction.
## Confirmed reference-context problems
- “holtburger” appears 163 times across 52 owned source files and 50 times
across 23 test files. Twenty-three source/test files contain a literal
`references/holtburger/...` path, but the only tracked reference root is the
uninitialized `references/WorldBuilder` gitlink. The citations are often
valuable independent corroboration, but a new maintainer cannot inspect the
cited code, commit, or license from this checkout. Examples include
`Core/Chat/CombatChatTranslator.cs:15-16`,
`Runtime/Chat/ChatInputParser.cs:6-7`, and numerous Core.Net wire-layout
comments. Stable public URL+revision or an owned evidence extract is needed;
a private/missing working-copy path is not durable provenance.
- Current source comments still describe deleted behavior. Examples include
`UiDatElement.cs:24-29` claiming `TextureCache.UploadRgba8` sets
`GL_REPEAT`, `FrameScreenshotController.cs:7-10,61,123-247` documenting a GL
thread/default-framebuffer path no production caller uses,
`FrameProfiler.cs:11-30` retaining an ImGui timing stage, and the dead panel
interfaces described in F-012. These are misleading, not merely historical.
- `RenderingDiagnostics.cs` and `PortalVisibilityBuilder.cs` contain multiple
comments explicitly ordering future maintainers to strip “throwaway” or TEMP
apparatus after May/June investigations that are already closed. The code
remained through the Vulkan cutover, so those comments are failed cleanup
markers rather than actionable ownership. F-016 records the executable debt.
- Campaign/issue references are useful as secondary provenance, but many source
comments make the chronology the primary explanation (for example the long
“Night-round review F14” house-marker note in
`MapPageController.cs:449-481`). A durable local invariant and current
reachability/support status should lead; campaign labels and dates belong in
linked research or decision records.
- Personal-machine paths are also embedded as provenance. The literal
`C:\Users\erikn` occurs in 146 documentation files, 43 tool files, eight
production-source files, two test files, and both root instruction files.
Production examples in `RetailCommandHelpTable`,
`RetailClientCommandCatalog`, `WeenieErrorMessages`, `OptionsPanelText`,
`ClientTextRefusals`, `AudioSettings`, `ConfigOptionsPageController`, and
`VendorUiController` cite a binary in the maintainer's Downloads directory.
Several comments also give a version or CodeView identity, which is the
durable evidence; the personal path adds no reproducible provenance and
implies access that another maintainer does not have.

147
docs/reviews/coverage-ledger.md Normal file
View file

@ -0,0 +1,147 @@
# Coverage ledger
Audit baseline: `15539a22a67f8d915d88f8b1d8126cd55eedda6e`.
The baseline commit and path-set fingerprints make the reviewed file universe
reproducible without copying thousands of paths into this document:
| Scope | Fingerprint (`git ls-files ... | git hash-object --stdin`) | Files/lines at baseline |
|---|---|---:|
| `src/**`, `tools/**` | `b79a0390fad9a3e86de765e49f869720045bbc94` | 1,325 C# files / 416,628 lines |
| `tests/**` | `34b425e434a7a3980e556a1ce445f2a40ae196fd` | 1,260 C# files / 402,131 lines |
| `docs/**`, root authorities, `memory/**` | `46b6c109b8b986a9425c46e66cbe6967e5ae3d46` | 1,127 tracked paths |
The repository has 4,018 tracked paths: 1,365 under `src`, 1,352 under
`tests`, 1,109 under `docs`, 152 under `tools`, 15 under `memory`, one
WorldBuilder gitlink, and 24 root/editor/CI paths.
## Owned production and tool code
“Inventory complete” means the file set, project references, package
references, size distribution, and solution membership were collected.
“Reviewed” means every file in the row participated in the all-file structural,
comment/reference, configuration, failure-handling, and dependency scans, with
manual review of entry points, public boundaries, exceptions, and size/
complexity hotspots.
| Area | C# files | Lines | Inventory | Deep review |
|---|---:|---:|---|---|
| `src/AcDream.App` | 628 | 205,466 | complete | complete |
| `src/AcDream.Bake` | 11 | 1,884 | complete | complete |
| `src/AcDream.Cli` | 6 | 1,736 | complete | complete |
| `src/AcDream.Content` | 31 | 7,533 | complete | complete |
| `src/AcDream.Core` | 275 | 78,848 | complete | complete |
| `src/AcDream.Core.Net` | 98 | 21,781 | complete | complete |
| `src/AcDream.Headless` | 35 | 8,859 | complete | complete |
| `src/AcDream.Launcher` | 15 | 3,342 | complete | complete |
| `src/AcDream.Launcher.Core` | 50 | 13,661 | complete | complete |
| `src/AcDream.Platform` | 2 | 206 | complete | complete |
| `src/AcDream.Plugin.Abstractions` | 8 | 147 | complete | complete |
| `src/AcDream.Plugins.Smoke` | 1 | 34 | complete | complete |
| `src/AcDream.Runtime` | 112 | 62,894 | complete | complete |
| `src/AcDream.UI.Abstractions` | 37 | 6,294 | complete | complete |
| `tools/*` | 16 | 3,903 | complete | complete |
The non-C# owned tooling surface contains 84 tracked executable scripts: 38
WinDbg/CDB command files, 30 PowerShell scripts, and 16 Python scripts across
the repository root and `tools/`. These are included in the tool review even
though they are not represented by the C# path-set count above. Initial
execution/readability triage is complete; support classification and exact
prerequisites are classified in F-004. All 13 tracked tool projects were also built
individually: eight succeed and five fail (three absent relative
DatReaderWriter references and two source-API drift errors). Two of the eight
passes depend on a developer-specific absolute project path that happens to
exist on the audit machine. Script triage found absolute Windows paths in 55 of
84 files and explicit developer-home paths in 43.
Size is already an audit dimension: 83 owned C# files are at least 1,000
lines, 24 are at least 2,000, and nine are at least 3,000. Size alone is not a
finding; cohesion, ownership, and change coupling will determine which files
are flagged.
## Test source
| Test area | C# files | Lines | Inventory | File-by-file review |
|---|---:|---:|---|---|
| `AcDream.App.Tests` | 515 | 174,271 | complete | complete |
| `AcDream.Bake.Tests` | 5 | 665 | complete | complete |
| `AcDream.Cli.Tests` | 1 | 178 | complete | complete |
| `AcDream.Content.Tests` | 29 | 6,120 | complete | complete |
| `AcDream.Core.Net.Tests` | 115 | 26,976 | complete | complete |
| `AcDream.Core.Tests` | 398 | 99,802 | complete | complete |
| `AcDream.Headless.Tests` | 18 | 9,573 | complete | complete |
| `AcDream.Launcher.Core.Tests` | 27 | 8,615 | complete | complete |
| `AcDream.Launcher.Tests` | 7 | 2,123 | complete | complete |
| `AcDream.Platform.Tests` | 2 | 224 | complete | complete |
| `AcDream.Runtime.Tests` | 91 | 64,102 | complete | complete |
| `AcDream.UI.Abstractions.Tests` | 46 | 8,583 | complete | complete |
| Fixture/helper projects and `tests/Fixtures` | 6 | 899 | complete | complete |
The exact baseline set is every path returned by
`git ls-files 'tests/**/*.cs'`; its fingerprint is recorded above. There are
68 test C# files at least 1,000 lines, 16 at least 2,000, and nine at least
3,000. Parameterized rows and generated/live-DAT gates will be audited through
their data sources as well as their declaring methods.
## Documentation and repository infrastructure
| Area | Inventory | Review |
|---|---|---|
| Root `README.md`, `NOTICE.md`, `AGENTS.md`, `CLAUDE.md` | complete | complete |
| `docs/README.md`, milestones, roadmap, issues | complete | complete |
| Architecture and divergence authorities | complete | complete |
| Active M4 plans/specifications | complete | complete |
| Historical plans, research, and audit records | complete | complete classification/link scan |
| `memory/` and documented `claude-memory/` links | complete | complete |
| Solution/project graph and CI | complete | complete |
| External/generated/reference material | complete | complete |
## Initial inventory facts
- `AcDream.slnx` contains all 14 owned `src` projects, 16 test/fixture
projects, and only one of the 13 tracked tool projects (`RetailTimeProbe`).
- `references/WorldBuilder` is the sole tracked reference gitlink and is not
initialized in this worktree. No other documented reference checkout is
present.
- Five tracked tool projects reference a nonexistent
`references/DatReaderWriter` project; two encode a developer-specific
absolute path.
- The repository-instruction audit skill named at `AGENTS.md:725` is absent,
as is the `claude-memory/` tree linked by the documentation map.
- The solution Release build was executed at the baseline: it succeeds with
26 warnings, all emitted by test projects.
- Every test assembly was executed. Eleven complete normally in the combined
run; Launcher.Core passes 338/338 alone but deadlocks in the combined run.
Current reproducible totals are 14,747 passing and 77 reported skipped, plus
one duplicate theory row dropped during discovery.
- All 11,458 declared fact/theory-like methods were syntax-inventoried for
assertion signals, diagnostic naming, literal tautologies, conditional
execution, skips, and timing primitives. The conditional-execution pass
found 314 method-level empty returns in 294 methods across 107 files; 271 of
those methods silently pass when an asset or opt-in environment gate is not
available. This cross-file triage supplied the exception set for the
completed semantic follow-up below.
## Completion evidence
The semantic follow-up is complete. It manually reviewed every exceptional
class produced by the all-file matrix: no-oracle methods and their helpers,
empty returns, explicit/custom skips, environment and installed-DAT gates,
source-text assertions, sleeps/real-time deadlines, random/concurrent tests,
literal tautologies, duplicate data, diagnostic naming, dead-stack tests, and
the open flake ledger. The exact actionable catalog is in
`test-quality-audit.md`; valid no-throw and deterministic-repeatability tests
were deliberately excluded from the “useless” label.
Documentation coverage used a full 1,014-file Markdown relative-link scan and
manual reading of the authority chain, current campaign/status documents,
repository instructions, memory entry points, issue/divergence ledgers, CI,
release/update paths, and representative historical records. Historical
research was classified and mechanically checked rather than line-edited as
if it were current architecture.
Build/test execution, package-vulnerability lookup, project/tool builds,
repository-size/provenance census, secret-pattern scan, and managed-stack
capture for the launcher hang provide executable corroboration. No source,
test, configuration, existing documentation, commit, or external system was
modified by this audit.

844
docs/reviews/findings-ledger.md Normal file
View file

@ -0,0 +1,844 @@
# Findings ledger
This ledger is complete for baseline
`15539a22a67f8d915d88f8b1d8126cd55eedda6e`. It contains 34 deduplicated
findings: four P0, 25 P1, and five P2. Absence from the ledger does not certify
behavioral correctness; it means the repository-wide maintainability review
did not find an actionable exception beyond the documented classes.
## F-001 — No top-level source licence
- **Severity:** P0 release blocker
- **Confidence:** high
- **Category:** release/legal packaging
- **Evidence:** `README.md:289-291` explicitly states that acdream has not
been assigned a top-level licence and is not ready for public redistribution.
`git ls-files` contains no `LICENSE` or `LICENSE.md`. The repository also
commits roughly 113 MB of decompiler output in
`docs/research/named-retail/acclient.c` and
`acclient_2013_pseudo_c.txt`, plus PDB-derived symbols/types; `NOTICE.md`
covers only WorldBuilder-derived MIT code and records no provenance or
redistribution basis for these research artifacts.
- **Impact:** the repository describes itself as open source but its own public
entry point says it cannot yet be redistributed. A public release cannot meet
the project's stated goal until ownership and licensing are resolved.
- **Recommended direction:** select and add an approved project licence, audit
provenance of extracted/adapted code and bundled artifacts, and make NOTICE
and dependency attribution consistent with it before release.
## F-002 — Public setup documentation describes a deleted renderer and UI
- **Severity:** P0 release blocker
- **Confidence:** high
- **Category:** user documentation / reproducibility
- **Evidence:** `README.md:24-37`, `README.md:67`, `README.md:100`,
`README.md:212`, and `README.md:237` advertise mandatory OpenGL 4.3 and
ImGui developer tools, including a nonexistent `src/AcDream.UI.ImGui/`.
`AGENTS.md:51-53` and
`docs/architecture/acdream-architecture.md:67-107` say OpenGL and ImGui were
deleted and Vulkan is the only backend. `src/AcDream.App/AcDream.App.csproj`
references Silk.NET Vulkan packages and no OpenGL package.
- **Impact:** a contributor following the release README is told to provision
the wrong graphics capabilities and expect a UI that cannot exist. This
blocks reliable onboarding and makes support reports ambiguous.
- **Recommended direction:** establish one generated or checked public
capability description sourced from the actual project/backend manifest;
remove deleted project/options claims and document the current Vulkan debug
behavior.
## F-003 — The designated “current” documentation map is chronologically stale
- **Severity:** P1 high
- **Confidence:** high
- **Category:** documentation authority
- **Evidence:** `docs/README.md:7` labels its snapshot 2026-07-27 and
`docs/README.md:8-15` says world-interaction Slices 4-6 remain. The linked
authority records all six slices complete and user-accepted at
`docs/plans/2026-07-23-world-interaction-completion.md:737`. That plan's own
opening status still says “Slices 1-4 are complete; resume at Slice 5”
(`:3-21`), and the roadmap updated 2026-08-14 still calls the program paused
with only Slices 1-3 complete (`docs/plans/2026-04-11-roadmap.md:169-233`).
The milestones document likewise says Slices 1-4 are accepted and vendor
work remains (`docs/plans/2026-05-12-milestones.md:80-121`). The audit
baseline is 2026-08-17. `docs/README.md:45-46` and `README.md:114-116` also
retain the older 8,826-test baseline.
- **Impact:** the page explicitly intended to prevent old status banners from
overriding current truth does exactly that. Humans cannot reliably infer
current work or release scope from the documented authority order.
- **Recommended direction:** separate timeless navigation from generated
current status; make current claims mechanically checked against the
milestone/roadmap or remove duplicated status numbers and slice summaries.
## F-004 — Tracked tools are not reproducible from a fresh checkout
- **Severity:** P1 high
- **Confidence:** high
- **Category:** build graph / developer tooling
- **Evidence:** `tools/dump-keymap/dump-keymap.csproj:11`,
`tools/SkyObjectInspect/SkyObjectInspect.csproj:12`, and
`tools/WeatherSetupProbe/WeatherSetupProbe.csproj:12` reference an absent
relative `references/DatReaderWriter` checkout.
`tools/StarsProbe/StarsProbe.csproj:12` and
`tools/WeatherEnumerator/WeatherEnumerator.csproj:12` reference
`C:\Users\erikn\source\repos\acdream\...`. Only `RetailTimeProbe` is in
`AcDream.slnx`, so the documented release build cannot expose these failures.
Building all 13 tracked tool projects at the baseline produces five failures:
the three absent relative references above, plus API-drift compile errors in
`PesChainAudit` and `RainMeshProbe`. The two absolute-reference projects pass
only because that unrelated source checkout exists on this audit machine.
The other eight build. Separately, the 84 tracked PowerShell/Python/CDB
scripts contain 107 absolute Windows-path tokens across 55 files and 82
developer-home path tokens across 43 files.
Three tracked root `launch-a6-issue98-*.ps1` scripts also hard-code an old
`.claude\worktrees\strange-albattani-3fc83c` checkout, and the three
`launch-flap-*.ps1` scripts are dated one-off probe launchers which rely on
now-historical environment switches and `--no-build` output.
- **Impact:** important forensic tools silently depend on one developer's
machine and can decay without CI visibility. Human maintainers cannot know
which tools are supported or buildable.
- **Recommended direction:** classify supported versus archival probes;
migrate supported tools to package/owned project references and include a
dedicated tool build gate. Move truly historical one-off probes out of the
maintained build surface with an explicit archival label.
## F-005 — Mandatory reference workflow depends on repositories absent from the repository
- **Severity:** P1 high
- **Confidence:** high
- **Category:** reproducibility / external context
- **Evidence:** `AGENTS.md:1481-1546` says `references/` holds six vendored
projects and requires domain work to cross-reference them. The tracked tree
contains only the `references/WorldBuilder` gitlink (`.gitmodules:1-4`), and
it is uninitialized at the audit baseline. Its URL is a personal SSH fork
(`git@github.com:eriknihlen/WorldBuilder.git`) rather than the public upstream
named by `NOTICE.md`, so even this one reference assumes account/key access.
`README.md:252` instead calls the references gitignored external repositories.
- **Impact:** the mandatory implementation and verification process cannot be
followed from a clone, while two top-level documents disagree about how the
prerequisites are obtained. Source comments and research citations that rely
on those names become inaccessible institutional memory.
- **Recommended direction:** publish a reproducible reference bootstrap
manifest with exact revisions, licences, and optional/required status. Make
docs distinguish vendored, submodule, package, machine-local, and historical
references.
## F-006 — Canonical documents link unavailable private/agent context
- **Severity:** P1 high
- **Confidence:** high
- **Category:** undocumented context / onboarding
- **Evidence:** `docs/README.md:79-84` sends maintainers to
`../claude-memory/MEMORY.md`; that path is absent. `AGENTS.md:725` mandates
`.claude/skills/investigate/SKILL.md` for audits; that path and skill are
absent. Numerous instructions also name Claude-only tools and machine-local
paths.
- **Impact:** the documented “start here” chain leaves a human or clean agent
without purportedly durable subsystem truth and mandatory procedure. This is
exactly the hidden AI-context dependency the audit is intended to detect.
- **Recommended direction:** move durable engineering knowledge into tracked,
tool-neutral repository documents. Treat optional personal memory and agent
skills as accelerators, never required authorities.
## F-007 — Architecture authority mixes current contract, stale state, and campaign journal
- **Severity:** P1 high
- **Confidence:** high
- **Category:** architecture documentation
- **Evidence:** `docs/architecture/acdream-architecture.md:79` retains the
heading “two coexisting presentation stacks” while lines 83-107 say there is
one. Line 461 calls L.2 the current organizing program while current milestone
documents name M4. Lines 1299-1301 repeat an obsolete 1,622-line/7,823-test
checkpoint and say a final visual matrix remains. The current
`GameWindow.cs` has 1,860 lines and repository authorities state that matrix
completed.
- **Impact:** a document declared the single source of truth cannot be read as
a stable contract without disentangling historical checkpoints. Maintainers
may preserve obsolete boundaries or restart completed work.
- **Recommended direction:** split normative architecture from dated decision
records and generated status. Keep ownership invariants concise; link to
historical campaign evidence rather than embedding its evolving ledger.
## F-008 — Extreme source and test unit size is widespread
- **Severity:** P2 medium (systemic; split points still require owner judgment)
- **Confidence:** high
- **Category:** human comprehension / change risk
- **Evidence:** 83 owned C# files are at least 1,000 lines, 24 at least 2,000,
and nine at least 3,000. Largest examples are
`src/AcDream.Core/Physics/TransitionTypes.cs` (6,345),
`src/AcDream.Runtime/Physics/RuntimeSetPositionState.cs` (6,283),
`src/AcDream.App/UI/RetailUiRuntime.cs` (4,611), and
`src/AcDream.App/Rendering/Wb/WbDrawDispatcher.cs` (3,773). Test code has 68
files at least 1,000 lines and nine at least 3,000. This is not only long
retail pseudocode: a structural count finds 32 types/170 methods in
`RuntimeSetPositionState.cs`, 27/115 in `RetailUiRuntime.cs`, 38/155 in
`LiveEntityRuntime.cs`, and 17/153 in `WorldSession.cs`, with no region or
generated-file boundary. `RetailUiRuntime` also contains 51 lock sites. The
largest test units mirror the same owners rather than providing small,
navigable contract groupings: `RuntimeInitialCreateContinuationExecutorTests`
is 4,765 lines, `HeadlessSessionHostTests` 3,601,
`RuntimeSetPositionStateTests` 3,522, `RuntimePhysicsStateTests` 3,198, and
`RuntimeRemotePlacementDriveControllerTests` 3,070.
- **Impact:** large units are difficult to navigate and review, but retail
algorithm cohesion and lifecycle invariants may justify some. The risk is
systemic enough to require file-by-file cohesion review rather than a blind
line-count rule.
- **Recommended direction:** after the cohesion review, split only along real
ownership or lifecycle seams; use partial files for navigability where a
single state machine must remain one owner.
## F-009 — Launcher child exit and supervisor disposal can deadlock
- **Severity:** P0 release blocker
- **Confidence:** high (captured live managed stacks)
- **Category:** concurrency / launcher lifecycle
- **Evidence:** a normal full Release test run hung twice in
`LauncherProcessSupervisorTests.ANullStderrLogPathBehavesExactlyAsBeforeForBothChildProcessKinds`
(`tests/AcDream.Launcher.Core.Tests/Launching/LauncherProcessSupervisorTests.cs:668`).
A `dotnet-stack` capture showed the test thread in
`WindowsSystemChildProcess.Dispose` -> `System.Diagnostics.Process.Dispose`
while holding `LauncherProcessSupervisor._gate` from
`LauncherProcessSupervisor.Dispose` (`src/AcDream.Launcher.Core/Launching/LauncherProcessSupervisor.cs:335-341`).
The process-exit callback thread was inside the runtime Process callback,
blocked entering the same supervisor gate through
`OnProcessExited` -> `SetState` -> `PublishPendingStateChanges`
(`LauncherProcessSupervisor.cs:215-225`, `241-279`). The child wrapper
unregisters and disposes the Process at
`WindowsSystemChildProcess.cs:179-205`.
- **Impact:** this is a classic lock-order inversion between the supervisor
lock and `System.Diagnostics.Process`'s internal synchronization. A launcher
session that exits concurrently with disposal can hang shutdown indefinitely;
the official full-suite command can also hang rather than report a result.
- **Recommended direction:** detach the owned child under `_gate`, then
unsubscribe/dispose it outside `_gate`; add a deterministic two-party race
test that proves both exit-callback and disposal orders converge. Audit the
ordinary `SystemChildProcess` wrapper for the same inversion.
**Post-baseline resolution checkpoint (2026-08-18, uncommitted):** the working
tree now performs the ownership transfer and all child operations outside
`_gate`; `OnProcessExited` obtains its optional exit code without holding that
gate. A barrier-controlled regression captures the exit delegate before
unsubscription and makes child disposal wait for that callback. It times out in
five seconds with the old lock shape, passes in milliseconds with the fix, and
passed 25/25 fresh-process repetitions. Launcher.Core passes 339/339. Two
fresh serialized complete-solution runs each finished under a 900-second hard
bound with 14,748 passes / 77 skips / 0 failures in 1:28.822 and 1:30.241.
F-009 remains a baseline finding until the coherent R1 checkpoint is reviewed
and committed; the current working-tree mechanism and required gates are
resolved.
## F-010 — Published build/test baseline materially overstates the portable gate
- **Severity:** P1 high
- **Confidence:** high
- **Category:** test reporting / release confidence
- **Evidence:** the baseline Release build succeeds but emits 26 warnings,
while `README.md:114-116`, `docs/README.md:45-46`, and issue #228 describe
17. A portable Release test run reports 5,510 App passes / 76 skips and
4,797 Core passes / one skip. The complete per-project total is 14,747
passes / 77 reported skips when the separately successful 338 Launcher.Core
cases are included; the all-solution process did not finish because of
F-009. xUnit additionally reports and drops one duplicate-ID theory row,
which is not included in its skipped total.
- **Impact:** “all tests pass / five intentional skips” is not a reproducible
statement for a clean contributor environment. Optional installed-DAT/GPU
tests, permanent scaffolds, known regressions, manual generators, and a
duplicate data row are collapsed into one obsolete number, hiding which
release properties were actually exercised.
- **Recommended direction:** publish separate portable, installed-DAT,
GPU/visual, connected, and manual-tool gates from machine-readable CI output.
Fail on unexpected skips and duplicate discovery IDs; do not hand-edit test
totals into multiple living documents.
## F-011 — Mandatory WorldBuilder inventory is an obsolete OpenGL design guide
- **Severity:** P1 high
- **Confidence:** high
- **Category:** architecture documentation / renderer ownership
- **Evidence:** `AGENTS.md:32-38` requires reading
`docs/architecture/worldbuilder-inventory.md` before rendering or DAT work.
That inventory calls `src/AcDream.App/Rendering/Wb` “GL infrastructure”
(`:21-27`), describes GL upload and GL-owned atlas records (`:43-101`),
inventories the deleted `Chorizite.OpenGLSDLBackend` and recommends taking
its OpenGL renderer/wrappers (`:343-469`), and says gameplay UI is
ImGui-based (`:538-564`). `NOTICE.md:16` repeats the GL description. The
binding architecture and project graph are Vulkan-only.
- **Impact:** this is not merely an old plan: it is a mandatory implementation
gate which tells maintainers to copy or preserve a deleted backend and UI
model. Its useful extraction/provenance inventory is inseparable from stale
technical direction.
- **Recommended direction:** retain a concise current inventory of owned ports,
provenance, and “do not re-port” rules. Move the pre-Vulkan component survey
into an explicitly historical decision record, and make current RHI/Vulkan
ownership the only normative rendering guidance.
## F-012 — A deleted presentation stack remains as unowned production code and tests
- **Severity:** P1 high
- **Confidence:** high
- **Category:** dead architecture / false test confidence
- **Evidence:** `AcDream.UI.ImGui` and the ImGui renderer were deleted, and
`src/AcDream.App/Composition/SettingsDevToolsComposition.cs:8-18` says no
`IPanelRenderer` implementation remains. Nevertheless
`AcDream.UI.Abstractions` retains 1,021 lines across `IPanel`, `IPanelHost`,
`IPanelRenderer`, `PanelContext`, `ChatPanel`, `DebugPanel`, and
`VitalsPanel`. Repository-wide source search finds no `IPanelHost` or
`IPanelRenderer` implementation and no production construction of any of
those three panels. At least 52 declared tests across the renderer/chat-panel
fixture set exercise this unreachable presentation path.
- **Impact:** humans see two UI designs in maintained production code despite
the one-stack architecture. Green panel tests can be mistaken for coverage
of the retained retail UI that actually ships, while changes to the dead
stack consume review and maintenance effort.
- **Recommended direction:** decide whether this is a real supported public
extension contract. If not, remove/archive the unreachable panel renderer
and its presentation-only tests while retaining shared VMs/settings that the
retail UI uses. If it is supported, name its owner, implement it in a shipping
host, and give it an explicit compatibility promise.
## F-013 — `UI.Abstractions` is neither backend-neutral nor a cohesive boundary
- **Severity:** P2 medium
- **Confidence:** high
- **Category:** dependency architecture / naming
- **Evidence:** `src/AcDream.UI.Abstractions/AcDream.UI.Abstractions.csproj`
references `AcDream.Core`, the higher-level `AcDream.Runtime`, and
`Silk.NET.Input`. Its public input contracts expose Silk `Key` and
`MouseButton` (`Input/KeyChord.cs`, `Input/InputDispatcher.cs`), its global
usings import `AcDream.Runtime.Chat`, and the same assembly also owns JSON
settings persistence, gameplay VMs, and the dead immediate-mode panel
protocol in F-012. The plugin UI contract instead lives in the BCL-only
`AcDream.Plugin.Abstractions` assembly.
- **Impact:** the assembly name suggests a small stable presentation seam, but
it couples platform input, runtime gameplay, persistence, and abandoned UI.
Changes propagate across unrelated concerns and make dependency rules harder
for a human to infer from project names.
- **Recommended direction:** define the supported contracts first, then split
backend-free input identities, application settings/storage, reusable
gameplay view models, and any real presentation protocol along their actual
consumers. Keep Silk adapters in App or a platform-input adapter assembly.
## F-014 — CI does not run the repository's complete release test gate
- **Severity:** P0 release blocker
- **Confidence:** high
- **Category:** continuous integration / release verification
- **Evidence:** `.github/workflows/headless-portability.yml` is the only
deterministic build/test workflow. Its presentation-free lane runs selected
project tests but explicitly omits `AcDream.Core.Tests` (`:114-133`). The
Linux graphical lane runs all `UI.Abstractions` tests but only five filtered
App test classes (`:251-262`); the Vulkan lane runs only App Vulkan tests
(`:368-375`). No workflow invokes `dotnet test AcDream.slnx`, the 4,797 Core
and most of the 5,510 App tests are absent from PR/push gates, and CLI tests
are absent as well. The daily hygiene workflow is an AI assessment, not a
deterministic required full-suite job.
- **Impact:** a change can merge while breaking the principal physics, UI,
rendering, or integration suites. The only local all-solution command can
currently deadlock under F-009, so there is no independent automated release
proof to catch it.
- **Recommended direction:** add a bounded, deterministic Release solution
build/test job on every supported OS row, publish TRX and skip manifests, and
split environment-dependent gates explicitly. Give every job a hang timeout
and collect dumps; make the portable complete gate required before release.
## F-015 — Headline pass totals include hundreds of unexecuted test contracts
- **Severity:** P1 high
- **Confidence:** high (syntax-tree inventory plus executable baseline)
- **Category:** test reporting / false confidence
- **Evidence:** 294 attributed methods across 107 test files contain 314
method-level empty returns. Of these, 248 methods in 85 files return when a
DAT directory, prepared package, or fixture is absent, and 23 methods in nine
files return when an opt-in environment variable is unset. xUnit records
these as passes. The three `LiveHandshakeTests` return at lines 38, 115, and
265 unless `ACDREAM_LIVE=1`; a source comment even calls this “skipped,” yet
Core.Net reports 1,004 passes and zero skips. The same pattern spans all
three Bake determinism facts and large App/Core DAT-backed suites. Separately,
73 `InstalledDatFact` methods produce real skips but all inherit one
campaign-specific “LA8 gate” reason from
`CharacterManagementLiveDatTests.cs:367-383`.
- **Impact:** a clean or CI machine can report a green contract it never
executed, so neither the 14,747-pass baseline nor a future CI total proves
the named rendering, physics, bake, layout, or live-network behavior. This
obscures asset provisioning failures and makes release confidence dependent
on undocumented machine state.
- **Recommended direction:** replace empty-return gates with explicit dynamic
skips or failures, assign traits and prerequisite-specific reasons, and emit
separate portable/installed-DAT/prepared-package/live-network manifests.
Treat an unexpected zero-execution group as a failed release gate.
## F-016 — Deleted OpenGL and temporary probe apparatus remain in shipping assemblies
- **Severity:** P1 high
- **Confidence:** high
- **Category:** dead code / diagnostics / misleading comments
- **Evidence:** `FrameScreenshotController.cs:123-247` retains an entire
`IDefaultFramebufferSurface`/`ReadDefaultFramebuffer` OpenGL resolve path;
repository-wide source search finds no production call, only four tests.
`WorldPassSurface.cs:7-18,198-242` keeps `IRenderFrameGlState` solely so the
only implementation can be an empty `NullRenderFrameGlState`, and keeps a GL
state reader whose production result is always `default` on Vulkan.
`WorldRenderDiagnostics.cs` still models and formats that nonexistent state.
`FrameProfiler.cs:11-30,199-213,289-296` retains an `ImGui` stage and CSV
column although the frontend is deleted. Most significantly,
`Core/Rendering/RenderingDiagnostics.cs` is 834 lines and declares 33
environment-backed render probe reads; its comments repeatedly say
“throwaway apparatus — strip once ...” and describe GL/FBO/SSBO failure
hypotheses from May/June. `PortalVisibilityBuilder.cs:268-272,1055-1062`
still carries a static TEMP A8 dump switch/dictionary and another explicitly
throwaway flap probe.
- **Impact:** a human maintainer must reason about backend contracts and
instrumentation that cannot observe the current renderer, while tests make
some deleted-backend helpers appear supported. Static process-wide probe
state also conflicts with the documented multi-session runtime model and
expands hot code paths for closed investigations.
- **Recommended direction:** inventory each probe by an active, reproducible
support need; retain only bounded diagnostics with a named owner and current
backend semantics. Remove deleted-GL-only interfaces/tests and rename generic
stages/contracts around current RHI concepts. Move investigation history to
research records rather than preserving it in executable branches.
## F-017 — Headless plugin callback failures are silently discarded
- **Severity:** P1 high
- **Confidence:** high
- **Category:** plugin reliability / observability
- **Evidence:** `HeadlessPluginHost.cs:231-237` wraps every
`Action<WorldEntitySnapshot>` invocation in `catch { }`. This applies both to
replay-on-subscribe and subsequent live delivery. No failure is sent to
`HeadlessPluginLogger`, the session diagnostics stream, or plugin state, and
the headless tests do not cover a throwing event handler. By contrast,
`GameRuntimeEventHub` records observer dispatch failures, and the plugin
architecture promises logged failure isolation
(`docs/plans/2026-04-10-plugin-architecture-design.md:72`).
- **Impact:** a plugin can stop receiving or processing world events while the
process, session, and plugin all remain apparently healthy. Operators and
plugin authors get no error or identity, making production diagnosis nearly
impossible and contradicting the stated failure-containment contract.
- **Recommended direction:** isolate each callback but report the exception
with plugin/subscription identity through the existing diagnostics channel;
define and test whether repeated handler failures fault/unsubscribe the
plugin or remain rate-limited warnings.
## F-018 — Diagnostic configuration bypasses the runtime's typed ownership model
- **Severity:** P2 medium
- **Confidence:** high
- **Category:** configuration / hot-path maintainability
- **Evidence:** owned source/tools contain 129 direct
`Environment.GetEnvironmentVariable` calls. Central diagnostic holders
account for 71, but production parsing, physics, and presentation paths read
environment variables directly while processing work:
`LiveEntityNetworkUpdateController.cs:436-437,503,521,607,907,1855,2563,2914`,
`RuntimeRemotePhysicsUpdater.cs:630`, `UpdateMotion.cs:163`,
`PlayerDescriptionParser.cs:458,473`, `PhysicsEngine.cs:2300`, and
`TransitionTypes.cs:1374,3905,5991`. Several are evaluated for every relevant
packet, entity update, or collision transition rather than captured once.
`RenderingDiagnostics` and `PhysicsDiagnostics` also expose large mutable
process-static state even though Headless explicitly supports multiple
isolated sessions.
- **Impact:** configuration cannot be discovered from one schema, captured in
a session snapshot, or controlled consistently in tests. Mid-process
environment mutation produces inconsistent behavior, direct reads add noise
to sensitive paths, and process-global mutable probes are unsafe to attribute
in multi-session diagnostics.
- **Recommended direction:** parse diagnostic configuration once into a typed,
documented snapshot, pass the minimal immutable flags/sinks to each owner,
and make any runtime toggles explicitly session-scoped. Eliminate direct
environment reads below composition roots.
## F-019 — The build toolchain and dependency graph are not reproducibly pinned
- **Severity:** P1 high
- **Confidence:** high
- **Category:** build reproducibility / contributor onboarding
- **Evidence:** the repository has 44 project files but no `global.json`,
`Directory.Build.props`, `Directory.Packages.props`, `NuGet.config`, or
`packages.lock.json`. Twenty-eight projects request `LangVersion=latest`,
only 23 declare `TreatWarningsAsErrors`, and common nullable/language/test
package settings are copied independently. CI installs floating SDK
`10.0.x` in all four jobs (`headless-portability.yml:82-84,163-165,235-237,
328-330`), while this baseline was evaluated with SDK 10.0.300. There are 84
direct PackageReference declarations distributed across the projects.
- **Impact:** a patch can compile or analyze differently as the .NET 10 SDK,
compiler, workload, or transitive dependency resolution advances. Project
templates already diverge in warning policy, so “warnings as errors” is not
a repository-wide contract. Humans cannot reproduce a released build from
the commit alone.
- **Recommended direction:** pin the accepted SDK feature band with
`global.json`, centralize common compiler/analyzer settings and package
versions, enable locked restore for release/CI, and document the one command
that proves a clean checkout. Keep tool-only exceptions explicit rather than
allowing per-project drift.
## F-020 — The issue ledger is an unbounded mixed tracker, research log, and archive
- **Severity:** P1 high
- **Confidence:** high
- **Category:** issue management / documentation architecture
- **Evidence:** `docs/ISSUES.md` is 19,073 lines (about 1.23 MB) with 380 issue
headings but only 378 unique IDs; #32 and #234 each have duplicate headings.
A block scan finds 207 bodies with closed-like status, another 50 whose title
says closed/done without a nearby status, 114 open-like bodies, five
nonstandard states, and four headings with no status signal. Thus roughly
two-thirds of the tactical file is closed history. Issue #419 alone begins
with about 100 lines of failed-attempt narrative, byte-decoded research, and
mandatory next-session protocol. The file contains 29 broken relative links,
including missing `references/holtburger` targets, moved source/tests, and
repo-root paths incorrectly resolved from `docs/`. Its own convention asks
every session to scan open issues while also warning against mechanically
separating the closed blocks (`:13-23`).
- **Impact:** humans cannot reliably answer “what is open?”, follow evidence,
or know whether a correction supersedes nearby text without parsing a large
chronological narrative. Duplicate IDs and link rot weaken commit/issue
traceability, while maintaining research inside the tactical queue guarantees
continued growth and merge contention.
- **Recommended direction:** define a small machine-checkable issue index with
one record per ID and normalized state/owner/severity/link fields. Move closed
bodies and long investigations to immutable archive/research documents while
retaining backlinks. Add link/status/duplicate-ID validation to CI and make
the active view generated or trivially filterable.
## F-021 — Output-only diagnostics are counted as release regression tests
- **Severity:** P1 high
- **Confidence:** high (method-body and same-file helper scan, followed by
manual inspection of the contract-like cases)
- **Category:** test suite signal / naming
- **Evidence:** 51 attributed methods across 27 App/Core test files contain no
assertion, expected exception, or other failure signal; their only result is
diagnostic output. The exact catalog is T-011 in `test-quality-audit.md`.
Many are openly named probes, dumps, inspections, or characterizations, but
others claim behavioral contracts such as
`ScenarioB_StairDescent_RampCellRetention`,
`Descent_RealCameraSweep_StairCellRetention`,
`SoundTables_WithAmbientSlots_ExistForWireBinding`, and
`ReplicateProductionEmission_OnPortalFills`. Their bodies calculate and
print observations without comparing them to an oracle. Most DAT-backed
cases also silently return as passes when the local asset prerequisite is
absent (F-015).
- **Impact:** investigation utilities inflate normal pass totals and names
imply coverage that does not exist. A production regression can alter every
printed value while CI remains green, and maintainers cannot tell which
tests are automatic gates versus manual forensic tools.
- **Recommended direction:** move output-only programs behind a separately
invoked diagnostic command/trait that publishes artifacts but is excluded
from release pass totals. Retain a method as a regression test only after it
has a stable oracle and a failure assertion; make names explicitly diagnostic
until then.
## F-022 — Campaign source-shape tests became a brittle parallel architecture
- **Severity:** P1 high
- **Confidence:** high
- **Category:** tests / architecture governance
- **Evidence:** 30 App test files read production source and contain 560
literal text assertions plus 86 calls to multi-fragment ordering helpers.
`GameWindowSlice8BoundaryTests` explicitly describes itself as a “Temporary
source-shape freeze” to be replaced by functional owner tests, but it remains
with nine facts, 73 literal assertions, and 31 ordering calls after the
documented Runtime ownership campaigns. Similar tests pin private field,
constructor, local-variable, and call spellings across composition, frame,
streaming, and Runtime-ownership files. See T-012 for scope and nuance.
- **Impact:** maintainers must satisfy literal implementation snapshots in
addition to the architecture document and behavioral contracts. Equivalent
refactors fail noisily, while semantics can regress without changing the
pinned text; this discourages cleanup of the exact giant files the tests
freeze.
- **Recommended direction:** retain true dependency and ownership invariants,
but encode them through project references, type/reflection checks, or
syntax/semantic architecture analysis. Give every campaign freeze a removal
condition and delete it when functional coverage lands; do not preserve
private method bodies as a permanent specification.
## F-023 — Release/App publish output always embeds the smoke plugin
- **Severity:** P2 medium
- **Confidence:** high
- **Category:** packaging / plugin defaults
- **Evidence:** `AcDream.App.csproj:85-129` unconditionally builds
`AcDream.Plugins.Smoke` and copies its DLL plus manifest to both ordinary
build and publish output. `GraphicalPluginSession.Create` always scans the
output `plugins` directory, and `PluginSession.Start` documents and
implements a null allow-list as “load every discovered id.” Launcher-created
normal/probe sessions protect themselves with an explicit empty list, but
omitted plugin configuration is intentionally the developer/direct-launch
flow. The embedded plugin subscribes to every entity-spawn event and writes
smoke diagnostics; it is not product functionality.
- **Impact:** a release artifact contains a test plugin and direct/config-based
launches can enable it merely by omitting a field. This makes “no configured
plugins” ambiguous, adds observable logging/event work, and gives human
maintainers two different defaults depending on launch path.
- **Recommended direction:** keep the example plugin buildable/testable but
exclude it from release publish output by default. Package samples
separately, make plugin enablement explicit in all production launch paths,
and test the published manifest contents.
## F-024 — Plugin registration cleanup failures are swallowed and can leak a plugin lifetime
- **Severity:** P1 high
- **Confidence:** high
- **Category:** plugin lifecycle / failure containment
- **Evidence:** `ScopedPluginHost.cs:53-205,260-269` catches and discards
failures while rolling back selection subscriptions, entity-event
subscriptions, and UI registrations. `PluginSession.Dispose` otherwise logs
failures from plugin `Disable` and ALC unload, but these inner cleanup errors
never reach it. Repository plugin tests cover ordinary unsubscribe and
collectible-context release, but none supplies a host event remove accessor
or UI registration whose cleanup throws.
- **Impact:** shutdown can report a plugin as released while a host registration
still references plugin-defined code or data, preventing collectible ALC
unload and allowing callbacks after disable. The failure is invisible to the
operator and cannot be distinguished from a CLR unload delay.
- **Recommended direction:** continue best-effort cleanup across all
registrations, but collect and report every failure with plugin and
registration identity. Define whether disposal faults the plugin status or
emits a bounded warning, and add adversarial removal/disposal tests.
## F-025 — The advertised plugin compatibility contract is not enforced or packaged
- **Severity:** P1 high
- **Confidence:** high
- **Category:** public API / documentation / compatibility
- **Evidence:** `PluginManifest.Parse` accepts every positive `apiVersion`, but
`PluginLoader` and `PluginSession` never compare it with a host-supported
version. The manifest's `dependencies` list is parsed and tested only for an
empty default; no production code consumes it. The approved plugin design
promises a locally published `AcDream.Plugin.Abstractions` NuGet package,
dependency handling, callback faulting/unhooking, hot reload, and a much
broader surface, while the project has no package metadata or pack target and
currently exposes eight small source files. No compatibility matrix or API
evolution policy exists.
- **Impact:** an incompatible plugin can pass discovery and fail later through
type loading or missing behavior, while a declared dependency has no effect.
Plugin authors cannot consume a versioned artifact from the release or know
which parts of the “design locked” document actually exist.
- **Recommended direction:** define one supported manifest/API version,
reject unsupported versions before loading code, either implement dependency
resolution or remove the field, and publish/version the abstractions package
as part of the release. Replace the aspirational design's status with an
explicit implemented-versus-planned matrix.
## F-026 — There is no owned release process for the updater-facing artifacts
- **Severity:** P1 high
- **Confidence:** high
- **Category:** release engineering / project governance
- **Evidence:** the repository has no tags, centralized product version, release
workflow, changelog, contribution guide, security policy, or CODEOWNERS. The
launcher production path is already hard-coded to fetch
`releases/latest/download/manifest.json` and per-RID hashed archives, but no
tracked workflow or production tool builds those archives and manifest;
`new-campaign-la-update-fixture.ps1` only creates a loopback test fixture.
`headless-portability.yml` publishes temporary CI outputs but does not create
a release, provenance/attestation, checksum manifest, or GitHub assets.
- **Impact:** the client/updater implementation cannot be reproduced as an
end-to-end release operation from repository instructions. Version choice,
artifact contents, signing/provenance, rollback, announcement, and security
response depend on undocumented maintainer knowledge.
- **Recommended direction:** define the version and release checklist in a
short maintained document; automate clean pinned builds, full bounded tests,
per-RID packages, SBOM/provenance/checksums, manifest generation and
validation, and release publication. Add contributor/security/changelog
ownership before calling the repository release-ready.
## F-027 — The retail-divergence authority is structurally unauditable
- **Severity:** P1 high
- **Confidence:** high
- **Category:** architecture documentation / technical-debt governance
- **Evidence:** `retail-divergence-register.md` is 809,163 bytes but only 525
physical lines because 243 lines exceed 1,000 characters and seven exceed
10,000; its longest section heading is 45,240 characters. It contains 320
active table rows (20 IA, 85 AD, 161 AP, 50 TS, four UN), with change history
repeatedly prepended to headings and individual cells. The title says
“current through 2026-07-31” while the same document records changes through
2026-08-17. Of 843 backtick path references, 18 occurrences across 11 unique
paths no longer resolve, including deleted App physics/UI classes and moved
Core.Net/UI command files.
- **Impact:** the file designated as the first diagnostic stop is hard to diff,
review, merge, filter, or validate mechanically. Current rationale is buried
inside corrections to prior rationale, and stale locations make it unsafe as
a source-navigation index.
- **Recommended direction:** store one normalized record per divergence (or one
small table per subsystem) with stable ID, kind, owner, current rationale,
oracle, live code symbols, and review date. Move amendment narratives to
linked decision history, generate a compact active index, and validate counts
and paths in CI.
## F-028 — Production comments depend heavily on campaign history and unavailable references
- **Severity:** P1 high
- **Confidence:** high (Roslyn comment-trivia inventory plus manual review)
- **Category:** comments / provenance / human maintainability
- **Evidence:** 37,734 owned source comment trivia nodes include 2,172 with a
campaign/slice/phase/checkpoint or commit reference, 1,016 with issue-number
references, and 617 with probe/temporary/workaround language. “holtburger”
appears 163 times in 52 source files and 50 times in 23 test files; 23 files
cite a literal `references/holtburger` path that is absent from the tracked
repository. Confirmed stale comments still claim GL-thread, GL-repeat,
OpenGL-framebuffer, and ImGui behavior after the Vulkan-only cutover; examples
include `Content/ObjectMeshData.cs`, `Content/MeshExtractor.cs`,
`UiDatElement.cs`, `FrameScreenshotController.cs`, and `FrameProfiler.cs`.
- **Impact:** the local reason for code is often inseparable from a private
worktree, a huge issue block, or campaign chronology. Humans cannot verify
provenance, and stale backend terminology actively misstates thread and
ownership contracts.
- **Recommended direction:** lead every maintained comment with the current
invariant and why it exists; keep retail symbol/address or stable public
revision as secondary provenance. Move attempt history, dates, campaign
labels, and closed probes into research/decision records. Add a comment-link
and forbidden-stale-backend vocabulary check for shipping code.
## F-029 — The two mandatory agent instruction files are materially out of sync
- **Severity:** P1 high
- **Confidence:** high
- **Category:** AI/human workflow authority / duplicated documentation
- **Evidence:** `AGENTS.md` opens by calling itself a synchronized agent-facing
port of `CLAUDE.md` and orders shared instructions to remain in both files.
A baseline diff reports 255 insertions and 27 deletions between them.
`AGENTS.md` is about 91,000 characters/1,435 physical lines; `CLAUDE.md` is
about 107,000/1,658. The agent copy still says to resume world-interaction
Slice 4, while the Claude copy records that program and several later
campaigns complete. They disagree on whether the required reference set has
six or five repositories, on AC2D's status, and on the semantics/default of
`ACDREAM_STREAM_RADIUS`; the newer copy also adds many private-memory entry
points absent from the agent copy.
- **Impact:** tool choice determines the project's “current truth.” An agent
following the repository's own instructions can restart completed work,
search for a retired reference, or run a gate with obsolete configuration.
Humans cannot safely review a 200-KB duplicated operational manual for
semantic parity on every change.
- **Recommended direction:** maintain one tool-neutral project instruction
authority and generate thin tool-specific wrappers. If two tracked copies
must remain, generate one from the other and fail CI on drift. Keep current
campaign state out of both; link one generated status page instead.
## F-030 — The repository already records seven unresolved load-sensitive tests
- **Severity:** P1 high
- **Confidence:** high for the documented recurrence; not all reproduced in
this single audit run
- **Category:** test determinism / release signal
- **Evidence:** open issues #302, #308, #321, #336, #340, #346, and #402 name
seven separate intermittent tests across App, Core, Core.Net, and Runtime.
The mechanisms include exact `GC.GetAllocatedBytesForCurrentThread` equality,
real-time deadline/sleep loops inside a virtual-clock transport soak, an
unresolved concurrent audio-cache dedup race, and shared reader/budget tests
that fail only under suite load. The exact catalog is T-014. All passed in
the one per-project audit baseline, which is consistent with the recorded
intermittent profiles rather than evidence of resolution.
- **Impact:** repeated release gates can alternate red/green without a source
change, encouraging maintainers to normalize failures as noise. Two cases may
represent production races rather than merely fragile assertions, so blanket
retries would hide defects.
- **Recommended direction:** assign an owner and deterministic reproduction to
each row; replace wall time and exact un-warmed allocation equality with
controlled mechanisms, and capture concurrency schedules/state. Quarantine
may separate reporting temporarily, but must not turn the cases into silent
passes or an approved generic retry list.
## F-031 — The launcher persists account passwords in plaintext without public disclosure
- **Severity:** P1 high
- **Confidence:** high
- **Category:** credential handling / release documentation
- **Evidence:** `AccountProfile.Password` is serialized into
`launcher-profiles.json`; `LauncherProfileStore.cs:119-181` explicitly calls
this the accepted plaintext-credential design. Linux creation is carefully
constrained to mode 0600, while Windows relies on the normal user-profile
ACL. The password is commendably kept out of child arguments/config and fed
over standard input, with redaction tests around status/crash output. The
public `README.md`, however, never tells launcher users that their password is
stored reversibly on disk, and there is no SECURITY document describing the
threat model or migration/erasure behavior.
- **Impact:** users may reasonably assume a launcher uses an OS credential
vault. Profile backups, sync tools, support bundles, or same-user processes
can expose the credential, and users cannot make an informed choice from the
release documentation.
- **Recommended direction:** prefer Windows Credential Manager and a Linux
Secret Service/keyring with an explicit fallback. If plaintext remains an
intentional alpha limitation, disclose it prominently before password entry
and in release docs, document exact file locations/permissions/deletion, and
keep the existing no-logs/no-arguments tests as hard gates.
## F-032 — Tracked “current truth” memory documents preserve superseded architectures
- **Severity:** P1 high
- **Confidence:** high
- **Category:** persistent project memory / documentation authority
- **Evidence:** project instructions describe `memory/` as persistent project
knowledge and route maintainers into it. Yet
`memory/project_gamewindow_decomposition.md:1,138-147` labels itself “current
truth” while prescribing an ImGui frame/submission path;
`memory/project_ui_architecture.md:18-43` says ImGui/OpenGL is the current
backend and “stays forever,” later partially supersedes itself, and ends by
saying to resume interaction Slice 4 (`:269`).
`memory/project_linux_graphical.md:39,75-77` still describes the packaged
cimgui/OpenGL/bindless-texture gate. Those projects/backends were deleted and
the six-slice interaction program is complete.
- **Impact:** the repository presents dated handoff notes as reusable current
authority. A human or agent following the prescribed “start at memory” path
can reintroduce deleted stacks or apply obsolete gate requirements even when
the architecture document is correct.
- **Recommended direction:** classify every memory file as current, superseded,
or historical with an owner/review date. Remove “current truth” from dated
handoffs, add a generated stale-link/forbidden-backend check, and route active
entry points only to maintained subsystem documents.
## F-033 — The prepared-content boundary still exposes vendor and deleted-backend vocabulary
- **Severity:** P2 medium
- **Confidence:** high
- **Category:** architecture boundary / dependency ownership
- **Evidence:** public `AcDream.Content.ObjectMeshData`, `MeshBatchData`, and
related package serializers expose `Chorizite.Core.Lib.BoundingBox`,
`Chorizite.Core.Render.Enums.TextureFormat`, and other DatReaderWriter/
Chorizite value types. `AcDream.Core.Rendering.Wb.TextureHelpers` also
consumes the vendor texture enum, forcing `Chorizite.Core` into Core and App.
`UploadFormats.cs` deliberately encodes OpenGL `GL_*` numeric constants and
instructs future maintainers to add new members using GL values, even though
the only production graphics backend is Vulkan. `AcDream.App.csproj` itself
records that the package audit remains unclean after deletion of the raw-GL
backend. The mandatory WorldBuilder inventory describes this as the intended
MP1 boundary rather than a remaining migration.
- **Impact:** a CPU/prepared-data contract that should describe acdream-owned
semantics is coupled to a pre-1.0 vendor package and an absent backend's ABI.
Package/API churn reaches Core, Content, App, package serialization, and bake
compatibility together. The names also mislead human maintainers about which
graphics API actually owns upload behavior.
- **Recommended direction:** introduce small acdream-owned bounding-box and
source-texture-format records at the extraction boundary, convert vendor
objects once, and version the serialized representation explicitly. Name
upload hints by channel layout/component type rather than GL constants; map
those semantics to Vulkan at the RHI boundary. Treat the current vendor types
as an acknowledged migration seam until removed.
## F-034 — Generated reverse-engineering state and capture logs dominate the repository
- **Severity:** P1 high
- **Confidence:** high
- **Category:** repository hygiene / onboarding / artifact provenance
- **Evidence:** the baseline's tracked worktree content is approximately
937 MiB. `tools/ghidra_project` contributes 575 MiB in 34 files, including
five opaque Ghidra database blobs around 90 MiB each; `docs/research`
contributes another 299 MiB. Thirty-seven tracked `.log` files total about
151 MiB, including a 29 MiB acdream capture. Thirteen individual tracked
files exceed 10 MiB. There are no Git LFS rules or documented artifact-
retention/support policy; `.gitattributes` only has a workflow merge rule.
The shared object store used by this worktree contains 2.65 GiB of pack data,
illustrating the history cost after generated artifacts change.
- **Impact:** every contributor pays a large clone/storage/indexing cost for
opaque tool databases and raw historical output that ordinary build, test,
and review work does not use. Binary state cannot be meaningfully diffed or
code-reviewed, logs can accidentally retain sensitive machine/session data,
and provenance/licensing review is mixed with the source tree rather than an
explicit artifact inventory.
- **Recommended direction:** keep reproducible extraction scripts, compact
curated fixtures, checksums, tool versions, and evidence summaries in Git.
Move reconstructable Ghidra projects and raw capture bundles to a versioned
artifact store or LFS only if their distribution is legally approved; publish
a bootstrap manifest and retention/redaction policy. Purging existing Git
history is a separate coordinated repository migration, not an ordinary
cleanup commit.

403
docs/reviews/test-quality-audit.md Normal file
View file

@ -0,0 +1,403 @@
# Test-quality audit
Status: **complete at the recorded baseline**.
## Baseline
- 1,260 tracked C# files under `tests/`, totaling 402,131 lines.
- 12 primary test projects, plus fixture/helper projects.
- The reproducible per-project Release total is currently 14,747 passes and 77
reported skips. One additional duplicate-ID theory row is silently dropped
by xUnit discovery. The official all-solution process does not currently
complete reliably because of the launcher deadlock described below.
- The Release build emits 26 warnings, all in test projects. These include
nullable-flow problems, dead fixture fields, xUnit assertion-style findings,
and a duplicate `InlineData` warning.
A clean `Rebuild` classifies those 26 as: seven CS8602; three each CS0649,
CS8600, CS8604, and CS8767; two each xUnit2013 and xUnit2017; and one each
CS8625, xUnit1025, and xUnit2000. An incremental build can misleadingly print
zero because no test compiler/analyzer target reruns; the warning baseline must
therefore come from a clean/rebuild gate.
## Executable baseline by test assembly
| Assembly | Passed | Skipped | Notes |
|---|---:|---:|---|
| App | 5,510 | 76 | installed-DAT/GPU/manual/regression groups collapse into dynamic skips; many other gated facts silently no-op |
| Bake | 21 | 0 | completed |
| CLI | 4 | 0 | completed |
| Content | 154 | 0 | completed |
| Core.Net | 1,004 | 0 | three disabled live-network facts are reported as passed |
| Core | 4,797 | 1 | plus one duplicate-ID theory row dropped during discovery; many absent-DAT facts silently no-op |
| Headless | 166 | 0 | completed |
| Launcher.Core | 338 | 0 | passes alone in 48 s; deadlocks in full run |
| Launcher | 67 | 0 | completed |
| Platform | 4 | 0 | completed |
| Runtime | 1,756 | 0 | completed |
| UI.Abstractions | 926 | 0 | completed |
## Review criteria
Every test source file and parameter/data source will be checked for:
- name/behavior/assertion agreement;
- a meaningful regression signal rather than successful execution only;
- independence from reimplemented production logic;
- redundancy with stronger tests;
- stable public behavior versus implementation detail;
- deterministic time, ordering, filesystem, culture, and environment handling;
- skip/manual/live-DAT intent and discoverability;
- correct layer/project placement;
- fixtures that can actually fail for the behavior named;
- data rows that exercise distinct boundaries rather than inflating counts.
The all-file pass combines syntax-tree method inventory with a per-file signal
matrix (attributed methods, assertions/exception oracles, early returns,
environment/asset gates, sleeps/delays, source-text reads, diagnostic naming,
and helper-local assertions). It is followed by manual body/helper review for
every exception class identified by that matrix. The project totals below make
the breadth visible; counts are lexical triage signals, not quality scores:
| Test area | Files | Attributed methods | `Assert.*` calls | empty-return sites | environment reads | sleep/delay sites | file-reading heuristic |
|---|---:|---:|---:|---:|---:|---:|---:|
| App | 515 | 4,812 | 20,997 | 121 | 73 | 10 | 43 |
| Bake | 5 | 19 | 91 | 1 | 1 | 0 | 0 |
| CLI | 1 | 4 | 19 | 0 | 0 | 0 | 1 |
| Content | 29 | 134 | 716 | 25 | 6 | 1 | 0 |
| Core.Net | 115 | 867 | 3,467 | 30 | 15 | 21 | 0 |
| Core | 398 | 3,398 | 8,576 | 70 | 1 | 9 | 6 |
| Headless | 18 | 140 | 806 | 5 | 0 | 5 | 4 |
| Launcher.Core | 27 | 232 | 989 | 19 | 0 | 13 | 10 |
| Launcher | 7 | 38 | 261 | 0 | 0 | 4 | 1 |
| Platform | 2 | 4 | 17 | 0 | 0 | 0 | 0 |
| Runtime | 91 | 1,344 | 8,601 | 64 | 0 | 1 | 3 |
| UI.Abstractions | 46 | 462 | 1,228 | 1 | 0 | 5 | 2 |
Fixture/helper projects were reviewed with their consumers and contain no
attributed tests. An assertion count includes helper assertions and repeated
parameterized-oracle calls; it is not an executable-case count. The final
column is deliberately broad (it also sees fixtures and result files); T-012's
manual follow-up is the exact 30-file production-source-reading subset.
## Skip and conditional-execution inventory
| Source | Declared reason | Initial concern |
|---|---|---|
| `AcDream.App.Tests/UI/Layout/CharacterManagementLiveDatTests.cs:373-383` | installed-DAT/probe gate | dynamic skip behavior and release discoverability |
| `AcDream.App.Tests/UI/Layout/ChatLayoutFixtureGenerator.cs:29` | manual fixture generator | generator represented as a skipped test |
| `AcDream.App.Tests/UI/Layout/RadarLayoutFixtureGenerator.cs:14` | manual fixture generator | generator represented as a skipped test |
| `AcDream.App.Tests/Rendering/TowerAscentReplayTests.cs:206` | unresolved issue #119 residual | permanent red test hidden as skip |
| `AcDream.Core.Tests/Conformance/PvsConformanceTests.cs:22` | “P0 scaffold” for future capture | incomplete scaffold counted in the suite |
The custom `InstalledDatFactAttribute` is used by 73 methods. It reports a real
xUnit skip unless `ACDREAM_PROBE_LIVE_MOUNT=1`, but its single LA8-specific
reason is inherited by unrelated character creation, map, tooltip, and other
installed-DAT suites. In contrast, 294 attributed methods across 107 files
contain method-level empty `return` statements. The syntax inventory divides
the 314 return sites as follows (a method can contain more than one category):
| Gate type | Return sites | Methods | Files | Result when gate is unavailable |
|---|---:|---:|---:|---|
| DAT/package/fixture | 251 | 248 | 85 | **pass**, without exercising the named behavior |
| opt-in environment variable | 23 | 23 | 9 | **pass**, without exercising the named behavior |
| operating system | 21 | 19 | 11 | pass on the unsupported OS |
| other condition | 19 | 18 | 12 | varies; several suppress missing expected data |
The first two groups are not included in xUnit's 77 reported skips. Therefore
the executable totals above describe discovered and completed cases, not the
number of named contracts that were actually exercised.
## Confirmed test-quality findings
### T-001 — Full-suite launcher test exposes a production lock inversion
`LauncherProcessSupervisorTests.ANullStderrLogPathBehavesExactlyAsBeforeForBothChildProcessKinds`
is not itself low-value, but its unbounded `using`-scope disposal exposed a
real production deadlock. The managed stack proves a supervisor-lock / Process-
lock inversion between `LauncherProcessSupervisor.Dispose` and the process exit
callback. The test needs a deterministic race harness after the production
fix; the current timing-dependent version alternates between passing in 48 s
and hanging indefinitely.
**Post-baseline resolution checkpoint (2026-08-18, uncommitted):**
`DisposeAllowsAnAlreadyCapturedExitCallbackToComplete` is now the deterministic
race harness. Explicit barriers make the fake child capture the exit delegate,
release it from inside disposal, and wait for the callback to return. The old
lock shape fails boundedly after five seconds; the fixed shape passed 25/25
fresh-process repetitions, all 22 supervisor tests, Launcher.Core 339/339, and
two serialized complete-solution runs of 14,748 passes / 77 skips. The original
real-process null-stderr test remains as end-to-end coverage rather than the
only accidental race trigger.
### T-002 — One physics assertion is a literal tautology
`Issue265SteepSlopeCaptureBisectTests.cs:920` asserts
`Assert.Equal(newModelVelocityBeforeToggle.Z > 0.01f,
newModelVelocityBeforeToggle.Z > 0.01f)`. It cannot fail and therefore proves
nothing about the comparison described by the surrounding comment. This is a
genuinely useless assertion inside an otherwise evidence-oriented test. It is
already recorded as open issue #342.
### T-003 — Duplicate theory data is silently discarded
`MotionInterpreterTests.cs:491-497` supplies both
`MotionCommand.Crouch` and literal `0x41000012u`; those values are identical.
xUnit emits duplicate case ID `6a4edd...` and skips the second row during
discovery. The test name is accurate, but the duplicate row is redundant and
the reported suite total does not reveal it. The build's xUnit1025 warning and
issue #228 already acknowledge this.
### T-004 — Test warnings include dead fixture state and contract mismatches
The current build reports 26 warnings. Examples requiring source-level review
include never-assigned `BeginTurnBlocked`/`BeginTurnUnblocked` fields in
`RemoteChaseEndToEndHarnessTests.cs:130-131`, nullable dereferences in installed-
DAT inspection tests, and nullability-mismatched fake implementations in App
composition tests. These are not all equivalent, but a release gate that
normalizes them as an old fixed warning count makes new warning regressions
invisible.
### T-005 — At least 52 tests exercise an unreachable UI presentation stack
The `IPanelRenderer` widget/menu tests and the ChatPanel layout/input/focus
tests are internally meaningful, but no production `IPanelRenderer` or
`IPanelHost` exists and no shipping code constructs `ChatPanel`, `DebugPanel`,
or `VitalsPanel`. The former ImGui implementation was deleted. These tests are
therefore obsolete release tests rather than tautologies: they can detect
changes in compiled dead code, but cannot protect user-visible retained UI.
They should be removed with that abandoned stack or moved behind a clearly
owned compatibility contract if the panel API is intentionally supported.
### T-006 — The installed-DAT attribute gives 73 unrelated tests one misleading skip identity
`InstalledDatFactAttribute` is declared inside
`CharacterManagementLiveDatTests.cs:367-383` and always says “installed-DAT
LA8 gate.” Repository-wide attribute inspection finds 73 methods using it,
including character creation and other UI/DAT areas unrelated to LA8. The
attribute does correctly produce a reported xUnit skip, but the reason does not
identify the prerequisite or contract of the test that was skipped. Split the
fixture capability from test-specific skip messages, and publish installed-DAT
gate results by suite rather than as one campaign-era bucket.
### T-007 — 271 gated facts report success without exercising their contract
A syntax-tree scan of every attributed method found 248 methods across 85
files that empty-return when DATs, a prepared package, or a replay fixture are
absent, plus 23 methods across nine files that empty-return unless an opt-in
environment variable is set. Examples include the three `LiveHandshakeTests`
(`:38`, `:115`, `:265`), whose own comment calls the return a skip although
xUnit reports all three as passed; 17 UI live-mount/powerbar probes; and broad
App/Core rendering and physics suites guarded by `datDir is null`. The pattern
also appears in all three Bake determinism facts. These names look like normal
regression contracts and their passes are included in headline totals. Use a
real skip/fail mechanism, group them under explicit installed-DAT/package/live
traits, and make release reporting state exactly which group ran.
### T-008 — Several diagnostic “tests” are deliberately incapable of failing
The whole `SmokeTest.TestProject_IsWired` body is `Assert.True(true)`.
`CellarLipWedgeTests.cs:99,305,331`,
`CellarUpTrajectoryReplayTests.cs:315,372`, and
`DoorCollisionApparatusTests.cs:238` end diagnostic output with an always-true
assertion; comments explicitly say some always pass. `GpuContractTests.cs:257`
compares the same enum constant to itself inside an otherwise useful test.
These are tools disguised as release tests. Move output-only apparatus to an
explicit diagnostic command/trait, delete the wiring smoke test, and replace
the useful diagnostics with assertions over the observed invariant.
### T-009 — Double-click tests use wall-clock sleeps instead of a controllable clock
`InputDispatcherDoubleClickTests.cs:53,73,89,106,108` sleeps for 10 or 600 ms
to cross real timing boundaries. This adds at least 640 ms to a small unit-test
class and leaves its outcome dependent on scheduler timing because production
`InputDispatcher` owns the clock. Inject `TimeProvider` (or a narrow monotonic
clock contract) and advance fake time deterministically.
### T-010 — Two entire tests have no runtime contract to protect
`SmokeTest.TestProject_IsWired` is the literal tautology described in T-008.
`ChaseCameraTests.ImplementsICamera` merely assigns a `ChaseCamera` to an
`ICamera` variable and calls `ToString`; compilation already proves the stated
interface relationship, and the call has no assertion or behavioral signal.
Both tests are useless as maintained release cases. The compile-only interface
check should be deleted or replaced by a named interface behavior test.
### T-011 — 51 release-suite methods are output-only diagnostic apparatus
A method-body and same-file helper scan found 51 attributed methods across 27
files that have no assertion, exception expectation, or other failure signal;
their observable contract is limited to diagnostic output. This count excludes
legitimate explicit no-throw cases such as no-op disposal, empty lifecycle
hooks, and secure/loopback transport construction. It also excludes tests whose
assertions live in a helper that the scanner can resolve.
Many names honestly say `Probe`, `Dump`, `Diagnostic`, `Inspection`, or
`Characterize`; these are useful investigation programs in the wrong execution
surface, not regression tests. The more serious naming problem is the subset
that reads like a behavioral contract—especially the facility-hub scenarios,
stair/camera retention sweeps, ambient-slot existence tests, production-
emission replication, and drawn-polygon comparisons—while only printing the
calculated value. Most DAT-backed examples also combine with T-007: they pass
without output when their local fixture is absent.
Exact inventory (method names in one cell belong to the named file):
| Test file | Output-only attributed methods |
|---|---|
| `App.Tests/Rendering/CornerFloodReplayTests.cs` | `Scratch_ReciprocalPrimitive_SyntheticPair` |
| `App.Tests/Rendering/Issue131SetupProbeTests.cs` | `Diagnostic_LookInFlood_AdmitsHallPorchFromCottage`; `Diagnostic_DumpOutstageCandidateSetups` |
| `App.Tests/Rendering/Issue176177FacilityHubFloodReplayTests.cs` | `ScenarioB_StairDescent_RampCellRetention`; `ScenarioC_CorridorSeamGazeSweep_Bistability`; `ScenarioE_RootLagWindow_ForwardChainRetention`; `ScenarioD_CorridorWalk_PerStepChurn` |
| `App.Tests/Rendering/Issue177StairDescentCameraFloodTests.cs` | `StairCellComposition_ShellVsStatics`; `RealStaircase_FineYawZoomSweep_FindKnifeEdge`; `FloodDepthFrom015E_VsRetail26`; `StaircaseSweep_EyeClearanceFromCeilingPortal`; `ParkedYawZoomSweep_StairAdmission`; `Descent_RealCameraSweep_StairCellRetention` |
| `App.Tests/Rendering/Issue181VisFlapReplayTests.cs` | `Diagnostic_FlappingCellViewRegion_SliverOrLarge` |
| `App.Tests/Rendering/Issue181WallPressEquilibriumTests.cs` | `Diagnostic_WallPressedCamera_EyeWanderAndViewerCellStability` |
| `App.Tests/Rendering/TowerAscentReplayTests.cs` | `Diagnostic_TopOfStairs_GateByGate` |
| `App.Tests/UI/Layout/FaPanelSlotProbeTests.cs` | `ProbePanelSlotTable` |
| `App.Tests/UI/Layout/MapHousePanelSlotProbeTests.cs` | `ProbeMapHousePanelSlot` |
| `App.Tests/UI/Layout/OptionsPanelLiveMountProbeTests.cs` | `ProbeLiveMountShapes`; `ProbeMenuPopupSizingAndTextStyle`; `ProbeFilterLabelHome` |
| `App.Tests/UI/Layout/PowerbarLayoutProbeTests.cs` | `ProbePowerbarAuthoredStrings`; `ProbeSecureTradeLayout`; `ProbeTotalItemsTemplate` |
| `App.Tests/UI/Layout/SpewBoxLayoutDumpDiagnosticTests.cs` | `SweepInstalledLayoutDescs_ForSpewBoxElementClass` |
| `Core.Tests/Audio/EnvCellSoundEmitterInventoryTests.cs` | `PortalDat_SetupsWithAmbientSlotSoundTables`; `SoundTables_WithAmbientSlots_ExistForWireBinding` |
| `Core.Tests/Conformance/CottageDoorwayCharacterizationTests.cs` | `Characterize_CottageNeighborhood_PrintStructure`; `Characterize_Doorway_FindInteriorPoints` |
| `Core.Tests/Conformance/DungeonLandblockDatProbeTests.cs` | `Probe_Dungeon0125_vs_Holtburg_A9B4` |
| `Core.Tests/Conformance/HoltburgTorchFalloffProbeTests.cs` | `Dump_Holtburg_StaticLight_Falloffs` |
| `Core.Tests/Conformance/Issue113DoorVanishDiagnosticTests.cs` | `DumpHoltburgBuildings_OrphanGeometry`; `ReplicateProductionEmission_OnPortalFills`; `DumpPortalFillSurfaceTypes`; `DumpControls_HallAndCottage` |
| `Core.Tests/Conformance/Issue113PhantomStairsDumpTests.cs` | `Dump_Cell104_ExteriorPortalPlane_Vs_GapPoint`; `DumpAAB3_Watchtower_TopDownMap`; `DumpHallModel_PolyFlagHistogram` |
| `Core.Tests/Conformance/PvsConformanceTests.cs` | `Pvs_CottageInterior_MatchesRetailCellDrawList` (also explicitly skipped and empty) |
| `Core.Tests/Conformance/ThresholdDivergenceDiagnosticTests.cs` | `Diagnose_ThresholdTransitions` |
| `Core.Tests/Physics/CameraCornerSealReplayTests.cs` | `Diagnostic_DispatchTrace_LeakPath_vs_Controls` |
| `Core.Tests/Physics/DoorSetupGfxObjInspectionTests.cs` | `HoltburgCottage_CellPortals_DatInspection`; `HoltburgLandblockStatics_DatInspection` |
| `Core.Tests/Physics/Issue137CorridorSeamInspectionTests.cs` | `CorridorSeam_FindPolygonMatchingLiveHit` |
| `Core.Tests/Physics/Issue186ConnectorCellGeometryInspectionTests.cs` | `Dump_ConnectorCells_ShellAndCollision` |
| `Core.Tests/Physics/Issue188FadingDoorMotionTableInspectionTests.cs` | `Reflect_DatReaderWriter_HookTypes`; `Dump_PedestalWeakSpot_MotionTable_HookContents` |
| `Core.Tests/Rendering/Issue176177DungeonSeamInspectionTests.cs` | `CorridorNeighborhood_CoplanarOverlappingDrawnPolyPairs`; `UnderHall_DrawnPolys_SurfaceColors`; `CellVertexNormals_SmoothOrFaceted_Dump` |
| `Core.Tests/Rendering/Issue93TownNetworkFountainRoomLightInspectionTests.cs` | `StaticObjects_SetupLightsCount_Dump` |
Move these to a separately invoked diagnostic tool/trait with artifact output,
or give each a stable oracle and assertion before retaining it in release test
counts. Contract-like names must not remain green when no comparison occurs.
### T-012 — Temporary source-text freezes became a second implementation specification
Thirty App test files locate the repository, read production `.cs` files, and
assert their literal contents or ordering. Across those files there are 560
`Assert.Contains`/`DoesNotContain`/`Matches` calls and 86 calls to custom
`AssertAppearsInOrder` helpers (each commonly checks many fragments), spread
through files containing 219 facts. Not all 219 facts use source text, and a
few whole-tree negative dependency scans are valuable architecture guards; the
problem is the large exact-fragment subset.
The clearest example is `GameWindowSlice8BoundaryTests`: its class summary
calls it a “Temporary source-shape freeze” whose assertions will be replaced by
functional tests as owners are extracted. It still contains nine facts, 73
literal fragment assertions, and 31 ordering-helper calls after the documented
Slice J/K ownership migrations. `UpdateFrameOrchestratorTests` contains another
85 literal fragment assertions and 13 ordering calls. Composition and Runtime-
ownership files repeat exact field names, constructor spellings, local-variable
names, and call text.
These cases are not useless: they did protect mechanical campaign cutovers.
They are now wrongly maintained as durable behavioral tests. Harmless renames,
formatting, extraction, or equivalent refactoring can break them, while a
semantic error that retains the expected text can pass. Keep structural rules
that encode real layer boundaries, preferably through project references,
reflection, Roslyn syntax/semantic analysis, or architecture-test tooling.
Retire campaign freezes once the named functional contract exists, and avoid
making literal method bodies a parallel architecture source of truth.
### T-013 — A retry-stability assertion compares the controller only with itself
`RuntimeLocalPlayerFirstEntryStateTests.RetryAtAwaitingActivationNeverRecommitsPublicationOrDuplicatesTheBody`
correctly captures the first `PhysicsBody` outside its three-iteration retry
loop and compares every later body with it. The adjacent controller assertion
at line 438 instead evaluates
`Assert.Same(fixture.Movement.Controller, fixture.Movement.Controller)` inside
each iteration. It can at most prove that two immediate property reads return
the same reference; it never compares the controller across retries as the
surrounding contract implies. Capture the first controller alongside `body`
and compare subsequent iterations to that reference.
### T-014 — Seven open load-sensitive tests make a green full run non-repeatable
The repository's own open issue ledger names seven distinct intermittent
tests; all passed in this audit's single per-project baseline, so none should be
silently ignored as “the known flake”:
| Issue | Test | Recorded mechanism/state |
|---|---|---|
| #302 | `PortalProjectionTests.ClipToRegion_FrameOwnedStore_ReusesExactResultArray` | exact per-thread allocation count; observed about one failure in six isolated App runs |
| #308 | `NakEmissionTests.LossSoak_TwoPercentBidirectional_ZeroMessageLoss_LedgersConverge` | virtual transport clock but real 60-second `DateTime.UtcNow` deadline and `Thread.Sleep` convergence loop |
| #321 | `DatSoundCacheTests.GetWave_ConcurrentSameId_PublishesOneCanonicalWaveAndDecodesOnce` | unresolved test-harness versus production cache race under load |
| #336 | `RuntimeCollisionReportingStateTests.WarmedSteadyContactRefreshDoesNotAllocate` | exact zero-allocation assertion; measured 2,944 bytes once under full load |
| #340 | `StreamingWorkBudgetTests.DestinationAndEmptyUnloadPriorityNeverBypassPublicationBudget` | load-sensitive, deterministic in isolation; root cause not recorded |
| #346 | `PortalProjectionTests.ProjectToClipLease_ReusesPooledWorkWithoutResultArrays` | second exact-allocation assertion in the same file; repeated full-load failures |
| #402 | `LandblockBuildFactoryTests.Build_UsesTheSuppliedSharedReaderGate` | roughly two failures in five repeated full App runs; shared-state/timing cause unresolved |
The #308 XML documentation calls the 10,000-message soak a virtual-clock test,
but its convergence actually has two real-time loops and a fixed sleep. The
names of the other six describe real contracts; they are not useless, but their
measurement mechanisms are unreliable or potentially expose real races. They
need deterministic clocks/schedulers, allocation warmup/bounds, and captured
race details before the suite can be a release gate.
### T-015 — Four explicit skips are tools, an empty scaffold, and a hidden known failure
Beyond the 73 custom installed-DAT skips, the remaining four reported skips
are not ordinary unavailable-platform tests:
- `ChatLayoutFixtureGenerator.GenerateChatFixture` and
`RadarLayoutFixtureGenerator.GenerateRadarFixture` are manual artifact
generators permanently represented as skipped facts. They have no regression
oracle and belong in a documented tool command.
- `PvsConformanceTests.Pvs_CottageInterior_MatchesRetailCellDrawList` is an
empty future scaffold. It is genuinely useless until a captured oracle and
implementation exist; a TODO/issue is clearer than a permanent green-suite
skip.
- `TowerAscentReplayTests.RetailShouldSeeTwoCells_AtFailureWindow` is a known
#119 residual deliberately disabled “until the fix.” A skipped failing
contract cannot guard against worsening or resolution; track the expected
current behavior separately, or make it an explicit non-blocking known-
failure lane with ownership and expiry.
Together with T-006, these explain all 77 reported skips. The release report
should distinguish unavailable external prerequisites from manual generators,
unfinished tests, and known product failures.
## Tests whose maintained names overclaim their behavior
This is the actionable naming subset, not a demand to rename every
campaign-prefixed test. A name is listed when its stated outcome is materially
stronger than its oracle:
| Test | Why the name is wrong or misleading | Classification |
|---|---|---|
| `PvsConformanceTests.Pvs_CottageInterior_MatchesRetailCellDrawList` (`:23`) | Empty body and permanently skipped; performs no comparison. | useless unfinished scaffold |
| `SmokeTest.TestProject_IsWired` (`:7`) | Only `Assert.True(true)`; compilation already establishes project wiring. | useless tautology |
| `ChaseCameraTests.ImplementsICamera` (`:59`) | Assignment proves only a compile-time relationship and `ToString()` is not asserted. | useless compile-only check |
| `GpuContractTests.TheDepthStencilAttachmentFormatCarriesAStencilAspect` (`:245-257`) | Checks clear/load fields, then “verifies” the format with `Assert.Equal(GpuTextureFormat.Depth24Stencil8, GpuTextureFormat.Depth24Stencil8)`; the pass description carries no format at all. | wrongly named and missing its central oracle |
| `RuntimeLocalPlayerFirstEntryStateTests.RetryAtAwaitingActivationNeverRecommitsPublicationOrDuplicatesTheBody` (`:424-438`) | The body half is checked across retries, but the adjacent controller stability assertion compares the property with itself. | partially overclaimed |
| `EnvCellSoundEmitterInventoryTests.PortalDat_SetupsWithAmbientSlotSoundTables` (`:130`) | Inventories/prints candidates without asserting that any qualifying setup exists. | output-only diagnostic named as existence contract |
| `EnvCellSoundEmitterInventoryTests.SoundTables_WithAmbientSlots_ExistForWireBinding` (`:187`) | Prints the inventory without an existence or binding assertion. | output-only diagnostic named as existence contract |
| `Issue176177FacilityHubFloodReplayTests.ScenarioB_StairDescent_RampCellRetention` (`:101`) and the `ScenarioC`/`ScenarioD`/`ScenarioE` peers (`:143,217,242`) | Calculate and print retention/churn; never assert the named retention or bistability result. | output-only replay named as contract |
| `Issue113DoorVanishDiagnosticTests.ReplicateProductionEmission_OnPortalFills` (`:194`) | Replicates and prints emission; no expected result is asserted. | diagnostic whose verb implies verified equivalence |
| `Issue176177DungeonSeamInspectionTests.CorridorNeighborhood_CoplanarOverlappingDrawnPolyPairs` (`:247`) and `UnderHall_DrawnPolys_SurfaceColors` (`:473`) | Dump calculated pairs/colors without asserting the noun phrase in the name. | inspections named as facts |
The remaining output-only cases in T-011 generally advertise `Probe`, `Dump`,
`Diagnostic`, `Inspection`, or `Characterize`; those names are honest, but the
methods are still misplaced in the default release suite. Contract-like names
in T-011 should either gain a stable oracle or be renamed and moved with the
diagnostic apparatus.
### T-016 — The test taxonomy preserves investigation history as its primary index
Forty-seven test files begin with an `Issue###` identifier, 53 filenames contain
`Probe`, `Dump`, `Diagnostic`, `Inspection`, or `Characterization`, and 17
contain `Replay`. The descriptive suffix often makes an individual file
understandable, so the issue-prefixed group is not automatically low-value.
The maintainability problem is that issue identity and one-off evidence shape
are the primary browsing taxonomy even after a behavior becomes permanent.
This couples discovery to the 19,073-line issue archive and mixes durable
regression contracts with T-011's output apparatus. When a test graduates into
a stable contract, organize it under the owning component/behavior and retain
the issue ID in a trait or comment; keep investigation programs in a separate,
non-default diagnostic project.

52
src/AcDream.Launcher.Core/Launching/LauncherProcessSupervisor.cs
View file

@ -199,6 +199,13 @@ public sealed class LauncherProcessSupervisor : ILauncherProcessSupervisor
return;
}
StopProcess(process, timeout);
}
private static void StopProcess(
ILauncherChildProcess process,
TimeSpan timeout)
{
process.TryRequestGracefulStop();
process.CloseMainWindow();
if (!process.WaitForExit(timeout) && !process.HasExited)
@ -214,12 +221,22 @@ public sealed class LauncherProcessSupervisor : ILauncherProcessSupervisor
private void OnProcessExited(object? sender, EventArgs e)
{
int? exitCode;
lock (_gate)
int? exitCode = null;
if (sender is ILauncherChildProcess process)
{
exitCode = _process is { HasExited: true } process
? process.ExitCode
: null;
try
{
exitCode = process.HasExited ? process.ExitCode : null;
}
catch (Exception error)
when (error is InvalidOperationException
or ObjectDisposedException)
{
// Disposal can detach the child after its native exit
// callback has already captured this handler. The terminal
// transition is still authoritative; only the optional exit
// code became unavailable during teardown.
}
}
SetState(LauncherSessionState.Exited, exitCode);
@ -325,21 +342,26 @@ public sealed class LauncherProcessSupervisor : ILauncherProcessSupervisor
_disposed = true;
process = _process;
_process = null;
}
if (process is null)
{
return;
}
if (process is { HasExited: false })
{
Stop(DisposeStopTimeout);
StopProcess(process, DisposeStopTimeout);
}
lock (_gate)
{
if (_process is not null)
{
_process.Exited -= OnProcessExited;
_process.Dispose();
_process = null;
}
}
// Event removal and child disposal can wait for an already-running
// native Process.Exited callback. They must stay outside _gate: that
// callback commits the terminal state through SetState, which needs
// the same gate. Holding it here creates the exact
// supervisor-gate/Process-internals lock inversion this ownership
// transfer is intended to prevent.
process.Exited -= OnProcessExited;
process.Dispose();
}
}

154
tests/AcDream.Launcher.Core.Tests/Launching/LauncherProcessSupervisorTests.cs
View file

@ -500,6 +500,58 @@ public sealed class LauncherProcessSupervisorTests
Assert.Equal(23, supervisor.ExitCode);
}
[Fact]
public async Task DisposeAllowsAnAlreadyCapturedExitCallbackToComplete()
{
var factory = new ExitDisposeRaceChildProcessFactory();
var supervisor = new LauncherProcessSupervisor(factory);
var states = new ConcurrentQueue<LauncherSessionState>();
supervisor.StateChanged += (_, state) => states.Enqueue(state);
supervisor.Start(Spec(), password: null);
ExitDisposeRaceChildProcess child = factory.LastCreated!;
child.BeginExit(47);
Assert.True(
child.ExitCallbackReady.Wait(TimeSpan.FromSeconds(5)),
"the captured exit callback did not reach its barrier");
Task disposeTask = Task.Run(supervisor.Dispose);
try
{
// Child disposal releases the captured callback and then waits
// for it to return. If the supervisor still holds _gate around
// child.Dispose(), the callback blocks in SetState and this
// bounded wait deterministically times out.
await disposeTask.WaitAsync(TimeSpan.FromSeconds(5));
}
finally
{
// Keeps the test process recoverable against the old deadlocking
// implementation: release the fake's disposal wait so the failed
// assertion cannot leave a ThreadPool worker permanently blocked.
child.ReleaseDisposeForCleanup();
await disposeTask.WaitAsync(TimeSpan.FromSeconds(5));
await child.ExitCallbackTask.WaitAsync(TimeSpan.FromSeconds(5));
}
Assert.True(child.Disposed);
Assert.Equal(LauncherSessionState.Exited, supervisor.State);
Assert.Equal(47, supervisor.ExitCode);
Assert.Equal(
[
LauncherSessionState.Starting,
LauncherSessionState.Running,
LauncherSessionState.Exited,
],
states);
// Idempotent convergence: a second explicit disposal neither touches
// the child again nor republishes terminal state.
supervisor.Dispose();
Assert.Equal(1, child.DisposeCallCount);
Assert.Single(states, state => state == LauncherSessionState.Exited);
}
[Fact]
public void LauncherProcessSpecCarriesNoCredentialLikeMember()
{
@ -891,6 +943,108 @@ public sealed class LauncherProcessSupervisorTests
}
}
private sealed class ExitDisposeRaceChildProcessFactory
: ILauncherChildProcessFactory
{
public ExitDisposeRaceChildProcess? LastCreated { get; private set; }
public ILauncherChildProcess Create(LauncherProcessSpec spec)
{
LastCreated = new ExitDisposeRaceChildProcess();
return LastCreated;
}
}
/// <summary>
/// Deterministically models the System.Diagnostics.Process exit/dispose
/// lock cycle. BeginExit captures the current delegate before the
/// supervisor can unsubscribe it. Dispose then releases that callback and
/// waits for it to return, just as Process.Dispose can wait on in-flight
/// exit machinery. The cleanup release exists only to let the test fail
/// boundedly against the old implementation instead of hanging its host.
/// </summary>
private sealed class ExitDisposeRaceChildProcess : ILauncherChildProcess
{
private readonly StringWriter _standardInput = new();
private readonly ManualResetEventSlim _exitCallbackReady = new(false);
private readonly ManualResetEventSlim _releaseExitCallback = new(false);
private readonly ManualResetEventSlim _exitCallbackReturned = new(false);
private readonly ManualResetEventSlim _releaseDisposeForCleanup = new(false);
private EventHandler? _exited;
public bool HasExited { get; private set; }
public int ExitCode { get; private set; }
public TextWriter StandardInput => _standardInput;
public bool Disposed { get; private set; }
public int DisposeCallCount { get; private set; }
public ManualResetEventSlim ExitCallbackReady => _exitCallbackReady;
public Task ExitCallbackTask { get; private set; } = Task.CompletedTask;
public event EventHandler? Exited
{
add => _exited += value;
remove => _exited -= value;
}
public void Start()
{
}
public void BeginExit(int exitCode)
{
HasExited = true;
ExitCode = exitCode;
EventHandler? captured = _exited;
ExitCallbackTask = Task.Run(() =>
{
_exitCallbackReady.Set();
_releaseExitCallback.Wait();
try
{
captured?.Invoke(this, EventArgs.Empty);
}
finally
{
_exitCallbackReturned.Set();
}
});
}
public bool TryRequestGracefulStop() => false;
public bool CloseMainWindow() => false;
public void Kill()
{
throw new InvalidOperationException(
"the already-exited race child must not be killed");
}
public bool WaitForExit(TimeSpan timeout) => HasExited;
public void ReleaseDisposeForCleanup() =>
_releaseDisposeForCleanup.Set();
public void Dispose()
{
DisposeCallCount++;
_releaseExitCallback.Set();
_ = WaitHandle.WaitAny(
[
_exitCallbackReturned.WaitHandle,
_releaseDisposeForCleanup.WaitHandle,
]);
Disposed = true;
_standardInput.Dispose();
}
}
private sealed class FakeChildProcess(
LauncherProcessSpec spec,
bool exitsWithinStopTimeout,