acdream/docs/reviews/findings-ledger.md

892 lines
54 KiB
Markdown

# 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.
**Post-baseline maintained-.NET-tool checkpoint (2026-08-18, `c38f6b88`):**
all 13 tracked `.NET` tools now use repository-owned project references or
central package references, build warning-free, and are members of
`AcDream.slnx`. The release gate fails if any future project under `src/`,
`tests/`, or `tools/` is omitted from that graph. `tools/README.md` records each
tool's purpose and runtime prerequisites. This resolves the supported `.NET`
tool portion of F-004. The older PowerShell/Python/CDB scripts and historical
root launchers described above remain open for the later R6 archival decision;
F-004 is therefore not closed wholesale.
## 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, `0a934cf5`):** the campaign
branch 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 in this historical baseline, but its mechanism and required gates
are resolved in a durable campaign-branch commit that is not yet merged.
## 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.
**Post-baseline gate checkpoint (2026-08-18, `2ac05486`):** the new
repository gate emits per-assembly TRX plus a JSON summary that distinguishes
14,748 executed/passed cases from 77 reported skips and retains the known
duplicate-discovery warning in the process log. This makes the complete
portable count reproducible, but F-010 remains open until R3 classifies false
passes/skips/diagnostics and the stale public headline claims are replaced.
## 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.
**Post-baseline resolution checkpoint (2026-08-18, `2ac05486`):**
`.github/workflows/release-gate.yml` now runs the repository-owned Release gate
on Windows for pull requests, `main` pushes, and manual dispatch. The script
auto-discovers and verifies all 12 solution test projects, runs each once in a
fresh bounded process, arms VSTest mini-dump collection before the outer
process-tree watchdog, and uploads commands, logs, TRX, environment metadata,
aggregate counts, and hashes even on failure. The exact local workflow command
passed 14,748 executed / 77 skipped / 0 failed in 107.337 seconds; an isolated
watchdog probe returned 124 at 2.107 seconds and left no child. F-014 is
technically resolved in the campaign-branch commit, pending normal review and
actual GitHub execution after publication.
## 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.
**Post-baseline resolution (2026-08-18, `2ac05486` and `c38f6b88`):**
`global.json` pins SDK feature band `10.0.300` with `latestPatch` roll-forward.
`Directory.Build.props` now owns the repository-wide compiler, analyzer,
warnings-as-errors, deterministic-build, and lock-file policy;
`Directory.Packages.props` owns all 30 direct package versions; and
`NuGet.Config` clears machine fallback folders and sources in favor of the one
declared `nuget.org` source. Every one of the 44 supported projects commits a
neutral dependency graph, and all 14 source projects also commit `win-x64` and
`linux-x64` graphs. The custom `packages.<graph>.lock.json` names avoid NuGet's
special precedence for conventional `packages.lock.json` while allowing the
neutral and two RID graphs to coexist. The release gate uses forced locked
restore, hashes all 72 graphs, and builds every supported project. A clean,
exact-commit `c38f6b88` gate on SDK `10.0.300` passed with 0 build warnings,
14,742 passed / 77 skipped / 0 failed across 12 test assemblies. A separate
empty-package-cache locked restore succeeded using only the declared source,
and locked neutral/RID re-evaluation changed zero lock hashes. F-019 is
resolved on the campaign branch. Hosted GitHub Actions remain deliberately
parked; the documented local command is the release authority until runner
policy is restored.
## 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.