[bug] Read-only paths still write: stats seeds and migrates, viz and the Stop hook rewrite v1 votes files

[SaulMoro]

Current status

Updated 2026-10-04 at 14:29 UTC against main at a8957a89. This issue remains open for the negative-feedback team-file read and the decision about ordinary stats owner seeding.

Item	Current state	PR and contributor
Dashboard vote reads through viz.ts	Fixed on main. Uses readUserVotes, so a V1 file is converted only in memory.	#973 merged, @SaulMoro
hasPendingVoteDeltas	Fixed on main. Its read no longer persists an unlocked V1 migration.	#973 merged, @SaulMoro
Ordinary stats scope config migration	Fixed on main. Config loads use read-only options. Plain stats also suppresses the misleading dry-run migration notice.	#970 merged, @SaulMoro
recallFeedback --negative team-file read	Still uses the migrating loader on main. Two open PRs cover the same row.	#975 by @ydflow, or #976 by @88lin
Ordinary stats seeds session-owners.jsonl when absent	Still present by design in the current implementation. Requires a maintainer decision.	No PR changes that policy.
In-memory stats owner credits and public preview integration	Followup pending, shared with #900.	#977 open, @SaulMoro

Remaining read that writes

On current main, negative feedback reads the team votes file through loadUserVotes at src/votes.ts. That loader persists a V1-to-V2 upgrade. The surrounding withVotesLock protects the local vote file, not this team file in the reports checkout.

The command must still write the requested downvote to the local file. Only the team-file read should switch to readUserVotes.

Proposed fix	Owner	Snapshot at this update	Scope
#975	@ydflow	Head d33e4263, open and mergeable. Its eight feedback CLI regressions pass. The five fork-safe E2E failures are existing stats and recall previews refused by the current guard.	Replaces the team-file loader and adds real-CLI regression coverage and design documentation.
#976	@88lin	Head c7b8eff0, open and mergeable. All 14 feedback CLI cases pass. Fork-safe E2E fails two obsolete stats and recall refusal assertions.	Same loader replacement, unit and real-CLI regressions. Also restores the stats and recall dry-run guard entries.

These are alternatives for the same vote-reader fix. The row has owners and is no longer up for grabs. Landing either corrected implementation resolves that row. Both PRs are not required.

#976's guard change overlaps #977. #977 also fixes inferred owner credits and nested missing-baseline reads, which the guard-only restoration does not cover. Coordinate the merge result before treating stats previews as complete. #900 tracks their public preview status.

Comment and CI audit on 2026-10-04: @ydflow confirmed the rebase and kept #975 scoped to its vote reader. The #975 job confirms eight new feedback tests pass and all five failures are in stats-recall-dry-run.test.ts. The #976 job confirms its feedback suite passes, but two dry-run-refusal.test.ts cases still expect the restored previews to refuse. The #977 fork-safe E2E job passed.

The proposed integration order is to merge #977, then rebase the selected negative-feedback PR and rerun CI. That keeps the complete preview correction together. @ydflow reports focused #975-on-#977 checks passing, but also reports eight Windows failures in a broader local run. Those local results were not independently rerun for this status audit.

Ordinary stats owner seeding still needs a decision

readSessionOwners() still creates a missing session-owners.jsonl using an exclusive write at src/session-owners.ts. An ordinary stats run can take this path. The seed derives owners from the snapshots available when it is created, so moving the write can change attribution behavior.

#970 deliberately kept this ordinary-run behavior. #977 also retains it. Its change supplies one invocation-local owner and credit state to dry-run filtering and baseline reads, so previews use the same inferred values without persisting them. It does not resolve the policy question about ordinary stats.

An ordinary stats run also retains its existing reports-checkout refresh behavior. The merged config fix does not make the entire command filesystem-read-only.

Before closing this issue:

• Merge one of fix(recall): keep team votes read-only during negative feedback #975 or fix(votes): read the team votes file without migrating it #976 and verify that negative feedback leaves the team V1 file unchanged while recording the local downvote.
• Decide whether ordinary stats should infer missing owners without persisting them, or retain seeding as an explicit accepted behavior. Record that decision here.
• Track fix(dry-run): restore stats and recall previews with inferred credits #977's related preview correction to completion in [bug] --dry-run still writes: recall maintenance/promote run for real, writes past the loader, and flags that are ignored #900 and preserve its corrections if fix(votes): read the team votes file without migrating it #976's overlapping guard change lands first.

Resolved migration race

The original hasPendingVoteDeltas used loadUserVotes without holding the vote-file lock. A V1 upgrade could overwrite a concurrent locked vote update with its earlier snapshot. #973 removes the migration write from that reader. Dashboard scans use the same read-only loader after #973.

Atomic temp-file writes protected against torn files, but did not prevent the stale-snapshot overwrite. This is historical context for the merged fix, not an outstanding finding against current main.

Evidence

This update checked current main, GitHub merge and open states, and both alternative PR diffs. No additional CLI tests were run for the status update. #973, #970, #975, #976, and #977 contain their own validation records.

The original issue was found while working on #900 and the read-only loader fixes in #893 and #901. Ordinary-run incidental migrations belong here. Requested dry-run behavior and its command guard remain tracked in #900.

[ydflow]

Coordination update for #975 / #976 / #977, also relevant to #900:

• fix(recall): keep team votes read-only during negative feedback #975's failing fork-safe E2E job has five failures, all in the existing stats/recall preview tests at the stale guard. Its eight new negative-feedback CLI regressions pass, and the six Lint & Test jobs plus Build pass. CI evidence.
• fix(dry-run): restore stats and recall previews with inferred credits #977 now has passing CI, including fork-safe E2E. It covers both the guard integration and inferred owner-credit/missing-baseline behavior. A guard-only restoration should not replace those corrections.
• I checked fix(recall): keep team votes read-only during negative feedback #975 on top of fix(dry-run): restore stats and recall previews with inferred credits #977 in a separate local worktree: build/typecheck/lint, 71 unit tests and 21 feedback CLI tests pass. Broader local CLI tests are not fully green on Windows: 34 pass / 8 fail due to log-file snapshots and an existing Windows ESM preload path; representative failures reproduce without fix(recall): keep team votes read-only during negative feedback #975 too.

@88lin @SaulMoro, I propose landing #977 first, then choosing one of #975 or #976 for the remaining negative-feedback row and rebasing that PR before rerunning CI. #975 remains scoped to the vote reader; it does not copy #977's changes. If maintainers prefer #976, #975 can be superseded rather than merging the same fix twice.

Ordinary stats owner seeding still needs the separate maintainer decision described above. This update does not resolve that policy question or request closing #972.