Initial audit record: reviewed 2026-09-05 before implementation. Findings and counts below describe that baseline; subsequent fixes are listed above.
The current tree contains substantial security and reliability improvements, but it is not ready to treat the earlier remediation pass as complete. This audit retains 22 findings: 4 High, 12 Medium, and 6 Low. The most consequential are destructive recovery replay (R01), journal-controlled backup execution under a weaker-trust parent directory (R02), Web shutdown abandoning workers (R03), and cross-target CI tools that cannot execute on the build host (R04). Review those four before release work continues.
The existing validation suite is green: normal/race Go tests, vet/Staticcheck, 180 frontend assertions, dependency scans, embedded-asset freshness, and six application cross-builds. Targeted probes still reproduced integration defects. Passing unit suites and compilable target binaries do not establish crash recovery, process ownership, real browser initialization, or complete CI-script correctness.
“AI slop” is assessed here as observable maintenance waste: unreachable paths, unused extension points, mock-only tests, redundant wrappers, and duplicated rules. No conclusion is drawn about who or what originally wrote a file. Existing failure-injection, authentication, archive, transaction, and lifecycle tests are valuable; wholesale test deletion or architectural replacement is not recommended.
A SHA-256 inventory captured 798 tracked/untracked nonignored files before analysis. Aggregate fingerprint of the sorted compact JSON path-to-hash map: 270747fe7c23e6252098960547f1575f948e06c8f9df714c3ae0ff9f1a8b90cf. Appendix C records hashes for the files cited by findings. Line references refer to this reviewed snapshot and may move after later edits.
The audit combined broad source inventory with risk-directed tracing of installer/update transactions, recovery and authentication boundaries, configuration persistence, Web/TUI/CLI lifecycle, package observations, frontend components, build/release scripts, tests, and agent instructions. Production Go and authored JavaScript accounted for approximately 58,000 lines; corresponding Go/JavaScript tests accounted for approximately 37,000 lines. This was a deep path-based review, not a claim that every line or platform behavior was independently proven correct.
Confidence labels distinguish executed behavior from source-confirmed paths and conditional exposure. Synthetic persisted journal states demonstrate replay behavior; they do not replace power-loss testing. Windows/macOS install/uninstall, UAC, signatures, ACLs, quarantine, and native process-tree handling were source-reviewed or cross-compiled, not exercised natively during this audit. No live signing service, protected GitLab settings, publication endpoint, or historical released-client bootstrap was newly qualified.
Each item identifies the live code, observed behavior, impact, a proposed direction, and useful validation. R06/R20 cover consolidation at different boundaries; R12/R13/R17 should share one configuration-parser effort. Counts are this audit’s retained items, not additions to the historical 101-entry register.
No findings match these filters.
HighReproducedBugs, Security
R01 Installer recovery can delete the only original copy
Source: pkg/installer/transaction_journal.go:468; pkg/installer/file_transaction.go:366; pkg/installer/file_transaction.go:461
Observation: rollbackInstallerJournal removes the live target before checking whether the required backup exists. The forward operation durably records intent before moving the original into that backup. Recovery also consumes backups with Rename without a durable per-entry replay state.
Evidence and confidence: A disposable, schema-valid file-swap journal with had_original=true, a live original, and an absent backup caused RecoverInstallerTransactions to delete the original and then report a missing-backup error. This persisted state represents both interruption after intent but before backup creation, and interruption after a previous recovery restored the original but before journal retirement. Both fixtures returned original_deleted=true. These were constructed boundary states, not physical power-loss tests; the complete probe is in Appendix A.
Impact: An interrupted installation, FFmpeg replacement, or overlay can turn a recoverable situation into permanent data loss on the next recovery attempt. Ordinary rollback tests do not cover this replay ordering.
Proposed direction: Record enough durable per-entry state and original evidence to distinguish untouched, backed-up, applied, and restored targets. Validate restoration prerequisites before deleting anything. Make replay safe to repeat at every filesystem/journal boundary, and retain recovery evidence until restored bytes and directory entries are durable.
Validation after approval: Inject termination at every journal write, original rename, replacement, restore, and journal retirement boundary. Restart recovery repeatedly and assert that an original or authenticated replacement always survives, including when the backup is absent.
HighReproduced; exposure conditionalSecurity
R02 Self-update recovery executes backups authorized only by a local journal
Source: pkg/version/transaction.go:921; pkg/version/transaction.go:935; pkg/version/transaction.go:738; pkg/version/transaction.go:1165; pkg/version/version.go:742
Observation: Startup recovery scans adjacent transaction directories and accepts structurally valid JSON as authority. ManifestKeyID need only be nonempty; the recovery loader does not authenticate that field or establish the journal directory and backup as belonging to a trusted transaction. rollbackUpdateTargets invokes the backup for identity checking. Its legacy fallback first captures evidence from the very backup it is about to trust; attacker-supplied evidence in a forged journal is not an independent trust anchor either.
Evidence and confidence: A fabricated applying journal with manifest_key_id=not-a-trusted-key and a dummy expected digest caused a harmless shell backup to create a marker before identity validation rejected it: backup_executed=true, original_preserved=true. The probe used only disposable paths and the current user. No cross-user privilege escalation was attempted. See Appendix A.
Impact: Execution is demonstrated; exploitation requires an attacker able to create appropriately named sibling files/directories beside the portable executable, or to modify trusted transaction state. A shared writable parent can have weaker trust than the executable itself, including a sticky directory where the attacker cannot replace that executable. Under that condition recovery can run attacker code with the manager’s privileges. This is not a demonstrated remote manifest-signature bypass.
Proposed direction: Establish and enforce transaction ownership, parent-directory trust, no-follow path handling, and restrictive permissions before inspecting executable artifacts. Bind recovery to durable evidence in a protected transaction store. Reject or explicitly migrate unauthenticated legacy state without executing it. Keep authenticating downloaded payloads separately.
Validation after approval: Exercise forged, symlinked, foreign-owned, and modified journals and backups under different user identities on native platforms. No backup should execute before independent trust is established; valid interrupted transactions must still recover.
HighReproduced lifecycle gapBugs
R03 Web shutdown neither cancels nor joins background installation jobs
Source: pkg/web/server.go:284; pkg/web/jobs.go:256; pkg/web/api_settings.go:365; cmd/mpv-manager/main.go:442
Observation: Jobs derive their contexts from context.Background. Server.Shutdown closes server/SSE signals and shuts down HTTP, but it has no worker cancellation-and-join phase. Install workers outlive the HTTP requests that created them. runWebMode can return when the server stops accepting requests, without waiting for an independently running shutdown routine to finish.
Evidence and confidence: An overlay probe created an active job and called Server.Shutdown: shutdown_error=<nil>, job_context_cancelled=false, active_jobs=1, with an immediate return. The probe isolates job ownership; source tracing connects that gap to process exit through the Web lifecycle. Go HTTP shutdown drains HTTP connections, not arbitrary background workers. Go Server.Shutdown documentation.
Impact: Closing the manager or triggering its shutdown endpoint can terminate the process during installer/configuration mutation and before a terminal job result is persisted. Subprocesses can continue independently (R05). The replay defect in R01 makes this combination especially concerning.
Proposed direction: Give the server explicit ownership of worker admission, contexts, and completion. Stop new operations, cancel work that can safely stop, let commit/rollback complete, drain output and persist outcomes, then allow main to exit. Make the main goroutine wait for that whole lifecycle.
Validation after approval: Use a controlled slow worker and a commit barrier. Verify shutdown rejects new jobs, cancels pre-commit work, waits for in-flight commit/rollback and output drain, records terminal history, and exits only afterward.
HighReproducedBugs, Agent DX
R04 Cross-platform release jobs run target executables on the Linux build host
Source: .gitlab-ci.yml:225; .gitlab-ci.yml:237; .gitlab-ci.yml:242; .gitlab-ci.yml:313
Observation: The shared build script runs go run ./cmd/verify-manifest and go run go-winres while job-level GOOS and GOARCH describe the target artifact. go run compiles for those values and then attempts to execute the result inside the Linux image.
Evidence and confidence: GOOS=windows GOARCH=amd64 go run ./cmd/verify-manifest -keys-only failed before the program ran: fork/exec .../verify-manifest.exe: exec format error. Windows and Darwin jobs inherit the offending command. Linux arm64 also needs host/tool separation on an amd64 runner without configured emulation. Six direct application cross-builds passed; they do not execute this CI script.
Impact: Non-native tagged release jobs cannot reach artifact production. The go-winres generation and extraction invocations have the same environment problem, so correcting only manifest verification is insufficient.
Proposed direction: Run build-time utilities with explicit host GOOS/GOARCH, or prebuild host tools once. Apply target variables only to compilation of the release artifact. Keep the Windows resource architecture as an explicit tool argument.
Validation after approval: Execute the complete shared script in the pinned Linux CI image for every target, including verification and Windows resource generation/extraction. Assert both tool execution and artifact identity; do not substitute go build-only checks.
MediumReproduced on LinuxBugs, Performance
R05 Command cancellation leaves descendants running and can keep output draining indefinitely
Source: pkg/installer/command_runner.go:71; pkg/installer/command_runner.go:84; pkg/installer/command_runner.go:176
Observation: The command runner uses exec.CommandContext without process-tree ownership or a bounded post-cancellation pipe-drain interval. Killing the immediate shell does not kill its descendants; descendants can retain inherited output descriptors. This matches Go’s documented default cancellation behavior. Go CommandContext documentation.
Evidence and confidence: A shell with a child that slept and wrote a harmless marker was given a 100 ms deadline. RunCommand returned after 602 ms, and the marker existed: child_mutated_after_cancel=true. An indefinitely living child can extend this mechanism indefinitely. See Appendix A.
Impact: Cancellation may appear acknowledged while child installers continue changing files. Inherited pipes can also keep a worker blocked after its direct child has died, preventing a correct shutdown join.
Proposed direction: Own subprocess trees using appropriate Unix process groups and Windows Job Objects or equivalent native handling. Define safe termination behavior for package managers, wait for descendants, and bound pipe draining with WaitDelay or equivalent handling.
Validation after approval: Run descendants that ignore termination, hold pipes, spawn grandchildren, and mutate a temporary marker. Verify no mutation occurs after cancellation completion and that output-drain waits remain bounded on each platform.
MediumSource-confirmedBugs, Performance
R06 Legacy version probes bypass the newer timeout and locale safeguards
Source: pkg/version/updates.go:155; pkg/version/updates.go:288; pkg/version/updates.go:341; pkg/web/server_version_cache.go:203; pkg/web/server_version_cache.go:354; pkg/web/package_version.go:64; pkg/web/api_install.go:552
Observation: CheckForAppUpdates falls back to queryPackageManagerVersion for records with no AppVersion. Its Flatpak, Brew, apt, pacman, and rpm commands use exec.Command and unbounded Output/CombinedOutput without LC_ALL=C. The Web cache calls this shared updater before reaching its newer concurrent bounded probes. The post-install Web GetPackageVersion wrapper also supplies context.Background instead of a job deadline.
Evidence and confidence: The live call chains were traced from synchronous Web cache initialization/refresh and the TUI update check into these helpers. The context-aware Web probe implementation therefore does not bound all version queries. Package and distro detection remain duplicated between pkg/version, pkg/web, pkg/installer, and pkg/platform.
Impact: A hung package tool can delay startup or leave update/install completion waiting indefinitely. Localized output can be misparsed, while divergent detection code makes fixes easy to apply to only one caller.
Proposed direction: Use one shared package observation service with caller context, an enforced probe timeout, bounded output, locale-stable commands, and distinct absent/unknown/error results. Reuse canonical distro classification and pass the job/server context through every entry point.
Validation after approval: Use a fake PATH with hanging, noisy, localized, and unavailable package tools. Cover missing-version records, startup, TUI refresh, Web refresh, and post-install version discovery; each must complete within its budget and preserve uncertain installed state.
MediumSource-confirmedBugs
R07 Install jobs disable cancellation before downloading or staging
Source: pkg/web/api_install.go:463; pkg/web/api_adopt.go:195; pkg/web/jobs.go:703; pkg/web/jobs.go:722
Observation: runInstallationJob calls BeginCommit at the start of execution, before constructing and running the installer. That installer still needs to download, authenticate, and stage artifacts. UI-change jobs similarly enter commit before the remote work. CancelJob refuses cancellation once commitStarted is set.
Evidence and confidence: The call order places almost the entire operation inside the non-cancellable phase. The UI continues to offer cancellation for running jobs, while the job model does not expose a useful per-phase cancellation capability.
Impact: Users cannot cancel a slow download or staging operation after the initial scheduling gap. This defeats a central background-job feature and can leave long-running operations needlessly holding resource leases.
Proposed direction: Move the commit boundary next to the first live mutation that must finish or roll back. Keep download, verification, and staging cancellable. Expose cancellation capability and the current phase to Web/TUI components.
Validation after approval: Pause a download and a staging step with barriers and verify cancellation works without changing live files. Pause inside commit and verify cancellation is explicitly deferred/rejected while commit or rollback reaches a truthful terminal outcome.
MediumSource-confirmed; native validation pendingBugs
R08 Windows MPC-QT uninstall reports success without waiting for or checking the uninstaller
Source: pkg/installer/windows.go:509; pkg/web/api_install.go:402
Observation: UninstallMPCQTWithOutput starts PowerShell Start-Process with -Verb RunAs but without -Wait or propagated child exit status. A failure to start PowerShell returns nil. A cmd.Wait error only prints a warning and also returns nil. Uninstaller lookup checks two hard-coded Program Files locations.
Evidence and confidence: The success/error branches are explicit in the Windows source. Web job completion removes the installed-app record after a nil uninstall error. Unlike the recently hardened installation path, this uninstall path does not wait for the actual elevated child or verify its result. It was not executed on Windows during this audit.
Impact: A cancelled UAC prompt, failed uninstaller, or still-running uninstall can be recorded as success and remove tracking for an application that remains installed. Custom locations are not reliably resolved.
Proposed direction: Resolve the selected installation’s uninstaller, wait for the actual elevated process, propagate its exit status, and verify the installation outcome before removing tracking. Report failure or a manual-action outcome truthfully.
Validation after approval: Native Windows cases should include UAC rejection, child nonzero exit, delayed completion, missing/custom uninstaller locations, and successful removal. Failed or uncertain outcomes must retain the selected app record.
MediumPolicy rejection reproducedBugs
R09 The Windows ownership safeguard has no migration path for existing installations
Source: pkg/installer/windows_ownership.go:33; pkg/installer/windows_ownership.go:103; pkg/web/api_adopt.go:407
Observation: A populated install directory without .mpv-manager-owned.json is rejected before update; uninstall also requires ownership evidence. The new installer creates that manifest for new payloads, but historical installations and the adoption flow do not establish an equivalent reviewed inventory.
Evidence and confidence: The platform-neutral validator rejected a temporary directory containing an existing mpv.exe with: refusing to modify populated directory without .mpv-manager-owned.json ownership proof. Source tracing shows adoption updates tracking/configuration without creating this ownership proof. Native upgrade from an archived release was not performed.
Impact: Previously installed or adopted MPV copies can appear managed while routine update/uninstall is refused. The refusal protects unrelated files, but the compatibility workflow is incomplete.
Proposed direction: Retain the safety check. Add an explicit ownership migration that proves a narrowly scoped file inventory, or present a supported manual migration/reinstall workflow before treating the install as fully managed. Never infer ownership of an entire populated directory.
Validation after approval: Use fixtures from historical manager releases and external portable MPV installs, including unrelated user files. Verify migration establishes only proven ownership and enables the advertised operations without deleting unrelated content.
MediumReproducedBugs
R10 A UI update overwrites configuration saved while its download is running
Source: pkg/installer/ui_config_update.go:144; pkg/installer/ui_config_update.go:164; pkg/installer/ui_config_update.go:175; internal/scriptopts/scriptopts.go:1
Observation: updateUIWithPreservedConfig reads the live config before invoking the staging/download callback. It later writes that old snapshot into staging and copies it over the live file. The installation transaction lock is acquired afterward, and the update does not revalidate the config revision or share the editor’s per-file update boundary.
Evidence and confidence: An overlay probe began with old-user-setting, saved saved-while-download-runs to the live config inside the staging callback, and completed successfully. The final file was old-user-setting: concurrent_edit_lost=true. The durable pre-update backup also comes from the early snapshot. See Appendix A.
Impact: A successful save from another process or editor can silently disappear during a ModernZ/uOSC update. Same-instance Web leases reduce one entry point, but do not protect other processes or external editors.
Proposed direction: Re-read and compare the live revision immediately before commit under the same coordination used by editors. Preserve the latest accepted bytes, or stop with a conflict instead of overwriting them. Ensure the backup represents the revision actually replaced.
Validation after approval: Coordinate two writers with deterministic barriers during download and just before commit. Assert the newer user edit survives or produces an explicit conflict, including cross-process use and edits through both UI-specific editors.
MediumSource-confirmedBugs
R11 UI transactions omit version metadata and clean-baseline state
Source: pkg/installer/ui_config_update.go:160; pkg/installer/ui_config_update.go:106; pkg/installer/common.go:541; pkg/installer/installer.go:794; pkg/installer/installer.go:940
Observation: The staging install writes the global ModernZ/uOSC version before staged bytes are committed to the live installation. Ordinary error paths attempt to restore that version, but the durable UI journal does not contain it. stagedUIPaths also copies .mpv-manager/ui-baselines although that path is absent from managedUIPaths, the UI rollback snapshot, and the commit sync set. Baseline/version persistence errors are reduced to warnings followed by nil.
Evidence and confidence: Comparing the staging callbacks, the commit path list, and the journal snapshot list exposes the mismatch. A process interruption after staging can leave the version advanced while the live UI is old; a rollback after baseline copy can leave new baseline metadata paired with restored scripts. These interruption cases were source-reviewed, not killed in a native harness.
Impact: Update detection and configuration migration/audit decisions can use metadata that does not describe the installed scripts. A successful-looking operation can also lack the baseline needed for later preservation and migration.
Proposed direction: Have staging return immutable artifact/version/baseline data without changing global state. Include authoritative metadata and baseline updates in the same durable transaction, with recovery compensation where stores differ. Surface persistence failures as failed or partial outcomes.
Validation after approval: Inject errors and restart at stage completion, baseline copy, version persistence, live commit, and rollback. Verify scripts, user config, clean baseline, and reported version always describe one committed generation.
MediumEmitted-file defect reproducedBugs
R12 The mpv.conf editor ignores profile scope and effective duplicate settings
Source: pkg/config/editor.go:29; pkg/config/editor.go:185
Observation: GetConfigValue and applyConfigValue scan for the first key= prefix without tracking profile headers. Missing keys are appended at EOF, which may still be inside a named profile. Spaced assignments are missed, and changing the first duplicate leaves later assignments in place.
Evidence and confidence: Setting hwdec on a file containing [special] and scale=bilinear appended hwdec=nvdec inside that profile. Changing hwdec in hwdec=no followed by hwdec=auto produced hwdec=nvdec followed by hwdec=auto. Both calls returned nil. The mpv manual confirms that settings remain in a named profile until another header or [default]. mpv configuration profiles. Player-level behavior was not exercised with a native mpv process here.
Impact: A successful settings save may affect only an inactive profile, alter an unintended profile, or leave a later effective assignment unchanged. The UI can display a value different from the setting mpv actually uses.
Proposed direction: Use a shared lossless parser that understands profile boundaries, whitespace, comments, quoting, and repeated assignments. Define the editor’s scope explicitly, update the effective assignment in that scope, and insert global settings into a valid default section.
Validation after approval: Round-trip global/default/named profiles, spaced keys, duplicate definitions, inline comments, and quoted values. Where practical, compare resulting effective options with a real mpv process rather than only checking generated text.
MediumRead truncation reproducedBugs
R13 Scalar configuration values containing commas are truncated in the Web editor
Source: pkg/config/editor.go:73; pkg/web/api_config.go:182; pkg/web/server.go:431; internal/webassets/static/config-page.js:1
Observation: GetConfigValue splits every setting except hwdec into comma-separated items. getMPVConfigValue returns only the first item, including for scalar values such as screenshot-avif-opts. The settings form uses that shortened value and submits its settings on Apply.
Evidence and confidence: Reading screenshot-avif-opts=crf=20,speed=6 returned ["crf=20", "speed=6"]. The Web scalar helper selects "crf=20". The read behavior was reproduced; the subsequent form rendering/submission path was source-traced.
Impact: Opening settings and applying a change can silently remove AVIF encoder options after the first comma. Other scalar path/template values with commas share the parsing problem.
Proposed direction: Separate scalar and list accessors using an explicit option schema. Preserve scalar text intact, and split only options whose data type is actually a list. Share the representation across Web and TUI.
Validation after approval: Round-trip AVIF suboptions and comma-bearing scalar values through GET, rendered form, and POST without edits, then with an unrelated field changed. Preserve all original scalar content.
MediumReproduced in ChromiumBugs, Performance
R14 Alpine components call init twice, duplicating Tasks requests and timers
Source: internal/webassets/templates/tasks.html:20; internal/webassets/templates/hotkeys.html:65; internal/webassets/templates/base.html:35; internal/webassets/static/tasks-page.js:17
Observation: Tasks, Hotkeys, and the jobs modal combine x-data components that expose init() with x-init="init()". Alpine automatically invokes an init method on a component before evaluating x-init. Alpine initialization documentation.
Evidence and confidence: A headless Chromium fixture loaded the repository’s vendored Alpine and actual Tasks component while counting calls: {"fetches":2,"intervals":2}. This uses the real framework lifecycle, which direct factory unit tests do not exercise. Hotkeys also installs media listeners in init; the jobs-modal initialization is likewise entered twice, although its downstream dialog effects were not separately counted.
Impact: Tasks performs duplicate initial requests and maintains two polling timers per mounted component. Other components repeat initialization side effects; missing lifecycle cleanup can compound leaks during remounts.
Proposed direction: Use Alpine’s automatic init convention consistently and remove redundant explicit invocation. Store and clean up timers/listeners in destroy where components can be removed. Make framework-mounted behavior part of focused frontend validation.
Validation after approval: Mount each affected component with the pinned Alpine runtime and assert one initialization, one fetch/polling loop, and complete timer/listener cleanup after removal and remount.
MediumBackend retention reproduced; frontend source-confirmedPerformance, Bugs
R15 Installation output limits do not bound memory across the complete pipeline
Source: pkg/installer/command_runner.go:131; internal/webassets/static/jobs.js:269; internal/webassets/static/jobs-modal.js:177; pkg/web/jobs.go:1
Observation: outputCapture accumulates pending text until a newline with repeated string concatenation and no byte cap. Carriage-return-only progress and a long unterminated line can grow indefinitely. The browser appends live output to job.output and the modal’s output array without retaining only the server’s bounded tail; per-line rendering/scroll updates add work.
Evidence and confidence: A probe fed 64 chunks of 64 KiB without a newline: pending retained 4,194,304 bytes. Repeated concatenation copies the accumulated prefix. Source tracing shows unconditional array pushes in both browser consumers, so server line-count limits do not bound an open client.
Impact: Verbose or malformed tool output can consume growing memory and increasing CPU, and a long-lived output modal can accumulate an expensive DOM. This is a local tool-output resilience issue, not a demonstrated unauthenticated network attack.
Proposed direction: Apply explicit byte and line budgets at capture, retained job history, SSE payload, and browser layers. Handle carriage-return progress, bound partial lines, use bounded buffers, and batch rendering/scrolling. Make truncation visible to the user.
Validation after approval: Stream very long single lines, CR-only progress, and many short lines through the full pipeline. Measure bounded retained bytes/nodes and responsiveness, including when the output modal stays open for the entire job.
MediumSource-confirmed concurrency gapBugs
R16 Checking updates bypasses the lease used for the same installed-app reconciliation
Source: pkg/web/server.go:183; pkg/web/api.go:91; pkg/web/server_version_cache.go:338; pkg/web/server_version_cache.go:379
Observation: /api/apps/refresh-installed acquires resourceManagerConfig, but /api/apps/check-updates does not. Both handlers call refreshVersionCache, which runs DetectAndSyncApps and removes stale app records. The latter is therefore a mutating reconciliation operation despite its update-check label.
Evidence and confidence: The route registrations and shared call chain expose the missing lease. Cache publication has a mutex, but that protects the cache assignment, not filesystem observation and config reconciliation against concurrent install/uninstall. The secondary stale-removal path also removes by method rather than the observed app identity.
Impact: An update check can observe a temporary installation state and alter tracking while a job owns the same resource. A stale observation can remove or recreate records and interfere with final job persistence. The exact timing-dependent end-user failure was not reproduced in this audit.
Proposed direction: Route every mutating reconciliation through the same operation coordinator, or make update checks read-only and move reconciliation behind a separately leased operation. Revalidate snapshot/identity before applying removals; preserve unknown observations.
Validation after approval: Hold an install/uninstall lease and call each refresh route with barrier-controlled detection. Verify that reconciliation is blocked or safely deferred, and that stale observations cannot remove a changed app record.
LowSource-confirmed optimizationPerformance
R17 Rendering Config repeatedly reads and parses the same file
Source: pkg/web/server.go:389; pkg/config/editor.go:29; pkg/web/api_config.go:182
Observation: getConfigSettings performs 22 GetConfigValue/getMPVConfigValue field reads. Each resolves the path, reads the entire mpv.conf, converts it to a string, and splits its lines. API/settings consumers repeat the pattern.
Evidence and confidence: The 22 call sites were counted in getConfigSettings and traced to os.ReadFile in GetConfigValue. No end-user latency improvement is claimed without a benchmark.
Impact: A small settings request does repeated I/O and allocations, and fields can come from different revisions if another writer saves between reads. Larger hand-maintained configs make the cost more noticeable.
Proposed direction: Parse once into an immutable request snapshot with typed scalar/list accessors, sharing the corrected semantics from R12/R13. Prefer request-scoped reuse before introducing a global invalidation cache.
Validation after approval: Benchmark representative small and large configs and count reads per request. Verify all fields are derived from one revision and that a subsequent request observes a newly saved file.
LowMulti-platform reachability analysisAI slop, Agent DX
R18 At least 24 production functions are unreachable from repository programs and tests
Source: pkg/web/package_detection.go:1 (removed by R18; baseline 346f272); pkg/web/locale_utils.go:34; pkg/config/config.go:625; pkg/version/version.go:449; pkg/web/server.go:919
Observation: The intersection of deadcode -test results for Linux amd64, Windows amd64, and Darwin arm64 contains 25 functions: 24 in production files and one unused test helper. The entire pkg/web/package_detection.go helper family is included, along with locale adapters, old config setters, a self-update wrapper, and SetPendingNotice.
Evidence and confidence: Analysis used golang.org/x/tools/cmd/deadcode@v0.45.0. The complete intersection is in Appendix B. Platform-specific results outside the intersection were deliberately excluded. Exported symbols may have external consumers, and custom build tags were not exhaustively analyzed; this is in-repository reachability, not proof that every public API can be deleted. Go deadcode analysis and limitations.
Impact: Unused alternate APIs and wrappers increase the search surface and invite agents to fix or reuse paths that the application never calls. Correctness-only Staticcheck does not establish that this exported surface is live.
Proposed direction: Confirm external API commitments, remove unneeded internal/public helpers, and explicitly retain any supported compatibility APIs. Add a reproducible platform-aware deadcode inventory or allowlist instead of claiming that the repository is clean from a single analyzer run.
Validation after approval: Re-run reachability with supported production roots/platforms and relevant build tags; then run normal tests and cross-builds after approved removals. Do not remove a platform-specific helper merely because Linux cannot reach it.
LowSource-confirmedAI slop, Agent DX
R19 Unused interfaces and tests exercise mock bookkeeping instead of shipped behavior
Source: pkg/installer/interfaces.go:50; pkg/installer/mocks.go:437; pkg/installer/interfaces_test.go:451; pkg/installer/interfaces_test.go:558
Observation: ArchiveExtractor and OutputWriter have no production consumers or real implementations. Their mock implementations are used by tests that check whether the mocks recorded strings, retained a format list, reset slices, or called a configured callback. TestInterfaceCompliance wraps compile-time assignments in a runtime test.
Evidence and confidence: Repository reference searches found these two interfaces only in their declarations, mock implementations, and mock-specific assertions. Six tests beginning at TestMockOutputWriter_RecordsOutput cover this unused scaffolding, not installation or extraction behavior. Other injected downloader/filesystem/command tests do exercise real behavior and should be retained.
Impact: The scaffolding inflates the apparent test surface and creates extension points that are not actually part of the running design. It adds maintenance cost without protecting the failures demonstrated in this audit.
Proposed direction: Remove unused seams and their mock-only tests after confirming intended consumers. Keep necessary compile-time assertions at package scope. Invest focused integration coverage in durable replay, subprocess ownership, framework initialization, and real orchestration boundaries.
Validation after approval: For every retained test, identify the production invariant it can fail. Verify removal of this scaffolding leaves production behavior tests intact; avoid replacing it with tests that simply restate a new implementation.
LowSource-confirmed cleanupAI slop
R20 Shortcut wrappers repeat platform dispatch already represented by the installer interface
Source: pkg/installer/common_handler.go:280; pkg/installer/common_handler.go:304
Observation: CreateInstallerShortcutWithOutput and CreateWebUIShortcutWithOutput repeat Windows/Darwin/Linux branches that perform the same nil check and call the same PlatformInstaller method. The interface implementation already chooses platform behavior. Some comments still describe these multi-platform wrappers as Windows-only.
Evidence and confidence: The three CreateWebUIShortcutWithOutput branches differ only in error wording; CreateInstallerShortcutWithOutput repeats the same shape. This is a concrete redundant wrapper pattern, not a recommendation to remove all delegation methods or rewrite the architecture.
Impact: Small policy changes require repeated edits, and comments make the supported behavior harder to infer. Similar low-value branching adds noise to an already large installer surface.
Proposed direction: Validate the platform installer/capability once and delegate once where behavior is truly identical. Keep explicit platform policy where it actually differs, such as Windows file associations. Correct stale comments during the approved cleanup.
Validation after approval: Use existing shortcut and platform tests plus cross-builds. A new test that only checks which identical branch called the same method is unnecessary.
LowMissing references verifiedAgent DX
R21 The finalized historical audit depends on four missing evidence reports
Source: docs/CODEBASE_REVIEW_FINALIZED_2026-08-31.md:10; docs/CODEBASE_REVIEW_FINALIZED_2026-08-31.md:99
Observation: The finalized August review links four source/reconciliation reports that do not exist in this working tree. Its Medium/Low section explicitly delegates detailed evidence and remediation to one of those missing files.
Evidence and confidence: Missing: CODEBASE_REVIEW_2026-08-31.md; CODEBASE_REVIEW_2026-08-31_glm-5.3.md; CODEBASE_REVIEW_COMBINED_2026-08-31-gpt-5.6-sol.md; CODEBASE_REVIEW_COMBINED_2026-08-31-glm-5.3.md. The current frontend/documentation contract tests still pass.
Impact: A new reviewer or agent cannot follow the advertised evidence for the earlier completion claims. The commit named in that report also does not describe the many uncommitted remediation files by itself.
Proposed direction: Restore the intended historical artifacts or make the canonical review self-contained with stable evidence and explicit supersession links. Associate remediation evidence with the actual reviewed tree/commit. Validate real local documentation links rather than only selected source strings.
Validation after approval: Check local Markdown/HTML references from maintained reports, verify every retained finding has resolvable evidence, and distinguish historical counts from the current findings. Preserve Atlas as the status authority.
LowSource-confirmed documentation driftAgent DX
R22 Root agent instructions mix durable rules with extensive history and stale status
Source: AGENTS.md:9; AGENTS.md:222; AGENTS.md:423; AGENTS.md:479
Observation: AGENTS.md is 499 lines, with a long session chronology, duplicated documentation indexes, old validation counts/tool versions in historical sections, and a current supported-platform list that omits the Windows arm64 artifact present in CI. Its frontend example pattern points to components affected by R14.
Evidence and confidence: The root document combines mandatory working conventions, current claims, historical narrative, and generated Atlas guidance. Reading it does not cleanly identify which claims are current or which test commands are sufficient for a particular subsystem.
Impact: Agents spend context on repeated history and can copy stale implementation patterns or mistake an old successful validation run for evidence about their current change. This is a navigation/maintenance problem, not an assertion that chronology is inherently useless.
Proposed direction: Keep root instructions focused on current invariants, Atlas workflow, repository entry points, and concise validation commands. Move history into maintained change/review documents and use narrowly scoped instructions for frontend, release, and installer work where needed. Link one canonical platform/build matrix and avoid duplicating mutable status.
Validation after approval: Walk through onboarding from atlas brief and the root instructions with a fresh checkout. Each rule, platform claim, and command should resolve to current code or authoritative documentation without relying on session history.
The targeted Go overlay tests intentionally assert the defective behavior to document the finding; they are temporary audit probes, not proposed regression tests. A regression test added during remediation should assert the desired corrected invariant instead. Final baseline comparison found only Atlas-generated TRACKING.md changed among pre-existing files. Application source, existing tests, dependency files, CI, and pre-existing documentation stayed byte-for-byte unchanged. TRACKING.md was not hand-edited.
The existing finalized August report retains a historical 101-entry baseline and says 98 are repository-remediated. This audit does not rewrite that register or automatically reopen every earlier item. R01 revisits the crash-durable installer boundary previously mapped to CM-01/CM-29. R02 revisits recovery trust and pre-execution authentication. R03/R05/R07 revisit worker lifetime and cancellation. R06 shows that fixing bounded Web probes did not cover all shared update callers. R11/R16 show incomplete integration of metadata transactions and resource coordination. R18 qualifies the earlier “dead-code clean” claim. Review and map these residual findings before updating earlier completion status.
Approval can be scoped by finding or phase. The next step is owner review of these reports; no fixes, dependency upgrades, refactors, native uninstall operations, publication, or implementation task execution were performed.
The following fixtures preserve the strongest evidence inside the reports so it does not depend on the lifetime of temporary files. Run standalone Go fixtures from the repository root with go run /path/to/fixture.go. They create and remove their own disposable data; Linux shell probes require /bin/sh. They intentionally demonstrate current defects. Ignore exact random temporary path names and microsecond timing differences.
SHA-256 values below identify the pre-existing files cited by findings. Report files themselves are intentionally excluded. The full 798-file starting inventory and raw validation logs remain locally in /tmp/mpv-manager-audit-20260905; the findings and reproduction code above are self-contained.