fix(qa): three regressions from yesterday's nine PRs - #611
Merged
Conversation
Found by an adversarial pass over work shipped 2026-09-11. All three are mine, and all three are in the half nobody could look at: mobile has no component test runner at all. 1. Mark-as-finished was a silent no-op on a device clock behind the server. The helper added to make web and mobile write the same locator also sent updatedAt from the device; the server reads that as a last-write-wins guard and answers 200 with the row untouched. The shelf had already flipped optimistically. Web's markAsRead never sent one. Removed, with a payload test — and the ordinary queued progress write keeps its timestamp, which is what the guard is actually for. 2. Apple sign-in never warned about a dropped guest merge — #609 wired three of the four entry points. An iOS reader was landed in a new account with no indication their work stayed behind. A literal scan now covers all four. 3. The Tutor kept ordering get_example_sentence from BuildGoal on every re-plan turn; #605 removed it from AllowedTools and SystemPrompt and missed that copy. The guard from #605 could not have caught (3): it reflects over a string array while the instruction is prose. It now reads the words. Two false starts are in its docblock — matching against REGISTERED tools left the mutation green, because a deleted tool is registered nowhere; and a naive scan failed on a correct file, because these prompts wrap and a name lands across a concatenation. Both mutations are red now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011rgEMvYYi4Egj99dVtvm6E
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
An adversarial QA pass over the nine PRs of 2026-09-11 — the ones whose screens no person had opened. Three findings are regressions introduced by those PRs. All three are mine, and all three landed in the half that has no test that could run:
apps/mobilehas no component test runner, and not one.test.tsxfile exists under it.1 — "Mark as finished" was a silent no-op on a slow clock
markProgressFinished— added in #602 precisely to make web and mobile write the same locator — also sentupdatedAt: new Date().toISOString(), a device clock.UserDataEndpoints.UpsertProgressreads that as a last-write-wins guard and, when it is not newer than the stored value, answers 200 with the row untouched.useBookActionshas already flipped the shelf optimistically, so the reader sees "finished" and the server has nothing.Web's
markAsReadhas never sent one. The guard is right for a queued background sync — an old queued write must not overwrite a fresh one — and wrong for a button the reader just pressed.Removed, with a payload test that also pins the inverse: the ordinary
updateProgresskeeps its timestamp, because that caller genuinely is a queue. Mutation-verified.2 — Apple sign-in never warned about a dropped guest merge
#609 wired
warnIfNothingCarriedinto three of the four merge entry points. An iOS reader whose guest row was not merged was landed in a new account with no indication that their highlights, vocabulary and progress had stayed behind — the exact outcome #609 existed to prevent, on the one path it missed.A literal-scanning test now covers all four, in the shape
capabilityLiterals.test.tsandrouteLiterals.test.tsalready use. ItsKNOWN_GAPSlist is empty and the docblock says shrinking it is always the fix.3 — The Tutor kept ordering a tool that does not exist
#605 removed
get_example_sentencefromAllowedToolsand fromSystemPrompt, and missed the copy inBuildGoal— sent on every re-plan turn.The guard shipped with #605 could not catch it: it reflects over a string array, and the instruction is prose. Its own failure message said "the system prompt may still be telling it to call them"; nothing checked.
So the guard learned to read. It scans what the agent says for anything shaped like a tool name and requires it to be allowed. Two false starts are recorded in its docblock because both are instructive:
"…get_weak_" + "vocabulary…") and the scanner reportedget_weak_. It now glues adjacent literals.Both mutations — plain and split across a concatenation — turn it red.
Also in this PR
The QA pass left seven test files behind, several of them characterization tests that assert today's behaviour with names saying what to invert when fixed. The Apple one is inverted here (its assertion was "does not warn"). The rest stand.
Verification
dotnet format --verify-no-changescleanWhat this pass did NOT fix, and is worth a decision
set_book_progressderives its percentage as chapters-done/total while the app's canonical formula is word-weighted (computeBookProgress) — measured live, 0.2308 vs 0.0797 on the same book. It declarespercentUnit: "book", so the server's guard trusts it.get_my_reading— the tool whose description says "call this FIRST" — does not list it afterwards.🤖 Generated with Claude Code
https://claude.ai/code/session_011rgEMvYYi4Egj99dVtvm6E