fix(project): reader keeps one commit per visit after an absent file (G-A2 follow-up)

[trakhimenok]

Refs #180. Closes the follow-ups of the second review of the GitHub project reader (G-A2, #181, review comment 5961426685): A1 and minors 1 to 4 and 6. Paths are under libs/datatug/main/src/lib/services/repo/.

A1: a 404 at a remembered commit is an absent file, not proof the commit is gone

github/github-project-reader.service.ts

• :563 doubtedByProjectFile, :895-922 (file) and :734 (listing): a remembered commit is doubted only when its listing answers 404/422, or when the project file is 404 and nothing has been read at the commit in this visit (confirmed, set at :453 by a cache hit with text, a file or a listing from the network). A 404 for any other file at a remembered commit is an absent file (and is remembered as absent, so a warm visit costs no API call, new absent files included).
• :576 doubt / :588 askAgain: asking again happens once per commit (memoised, whoever doubts it). If GitHub names another commit the visit moves; if it names the same one the commit stands (and is no longer "remembered"); if the call is refused or times out the remembered commit is kept for the visit and the 404 is absent, nothing persisted.
• :504-552 resolveCommit: the persisted memory is no longer deleted before asking. It is replaced by the answer, or dropped (:549, for that ref only, new dropResolved) once GitHub has answered that the ref is gone or the repository moved.
• :609 moveTo: on a change the whole visit moves: the listing, files and decoded JSON read at the old commit are forgotten, readInfo reports the commit in use, and reads still on their way (epoch, :675 loadTree, :825 loadText) are read again at the new commit when they come back. One visit never serves two commits.
• Trust is unchanged: the re-resolve is commits/HEAD (or the ref) on the API; a trusted project's commit comes from nowhere else.

Fix too

1. Cache guard, github/github-file-store-api.ts:96,119: the first call (chunk load and database open) no longer turns the cache off for good after 1500 ms. Reads still go on from the network after 1500 ms; the calls after it answer empty at once (not one wait each); the opening has its own named limit GITHUB_STORE_OPEN_TIMEOUT_MS (5 s), and an answer within it switches the cache back on for later calls. A late answer is never applied to a read that already completed. A store that had answered and then hangs is still off for the visit.
2. forget race, github-project-reader.service.ts:464,475,539: a per-repository generation; a resolve that began before forget does not write its commit back. With the cache off or paused forget (and dropResolved) still attempt the persisted delete, with the same wait (github-file-store-api.ts, the through calls).
3. Re-resolve drops only the memo of the ref concerned: new dropResolved(key) on the store (github-file-store-api.ts:41, github-file-store.ts:311). Tested with two refs.
4. Raw refused and the mirror 404 at a pinned commit (github-project-reader.service.ts:902): an optional file is absent for this visit, not persisted; the error naming both hosts stays for unversioned reads (HEAD). Decision to check: the project file keeps the error at a pinned commit too, because a lagging mirror is most likely to be missing exactly the project just created, and "No DataTug project here" would be the wrong answer for a retry-able failure.
5. Mutations: tests now fail when the memory drop is removed from the re-resolve path (and when it is repo-wide), and when resolve(true) becomes resolve(false) (12 tests). The redundant defer in datatug-store.service.github.ts:73 is removed: the reader's observable is lazy and the existing S2 test (project file fails once, then recovers) passes without it.
6. github/github-api.ts:54,77 and new-project/new-project-form.component.ts:255,291: no fallback to 'main' when the payload has no default_branch; toGithubRepo treats it as incomplete, requireGithubRepo throws a GithubRepoError saying what GitHub did not return, and the create form shows its message (repository just created, or chosen repository). listRepos leaves out a repository it cannot commit to.

Left as it is: a repository with no project file costs one API call per page load (the project file is the one 404 that asks again; accepted in the review).

Tests

New github/github-project-reader.followup.spec.ts (34 tests), written first and failing on main: the reviewer's two sequences; the push mid-visit with an absent file probed (also added to the existing commit spec); a gone commit (listing 422) recovering with one re-resolve; five absent optional files at no API call on a warm visit; refused and timed-out re-resolves; the whole-visit move including reads on their way; ref-only drop; forget race; slow cache. Guard, store, create-service, form, github-api and repos-service specs extended or added. Two existing M7 tests were changed on purpose (an absent non-project file no longer asks again; a refused re-resolve keeps the remembered commit instead of degrading to HEAD). 35 mutations of the reader and the guard were run; 8 survived the first round, 6 got a test and 2 were removed as redundant code (a second epoch check on decoded JSON, a repeated doubt test); the 33 that remain are all killed.

API calls (fake GitHub): cold visit 2 (commit, listing); warm within 5 minutes 0, absent files included; warm after 5 minutes 1 (the question); changed project 2; five new absent files on a warm visit within 5 minutes 0 API calls (5 file-host reads, then 0).

No change to the chat IndexedDB (still version 2) or to the cache database's version or schema.

🤖 Generated with Claude Code

[trakhimenok]

[review r1 #182] Adversarial review (Opus): 450 targeted tests, 54 mutations (47 killed), probes Q1-Q10 on an export of this head and of main.

Reviewed-Head: 4ea3f69

A1 as reported is fixed, trust is intact, the forget race and per-ref drop hold, the cache database's version and schema are unchanged. Paths under libs/datatug/main/src/lib/services/repo/. X is the remembered commit, Y the new one.

Major

• M1. Reads that complete while the "which commit?" question is in flight are delivered at X, then the visit moves to Y (github/github-project-reader.service.ts:680-686, :831-837, :576-602). The PR's own test github-project-reader.followup.spec.ts:616-634 asserts the stale listing. Fix tried by the reviewer: in loadText and loadTree take const commit = await session.commit and after loadTextAt/loadTreeAt await this.doubts.get(commit) before the epoch check; all other targeted tests pass, the test at :616 needs rewriting.

Minors

1. Regression against main: a project pushed from elsewhere into a repo already viewed is "No DataTug project here" for up to 5 minutes, reload does not help (:563-565, :708, :725): a cached listing "confirms" X so the project-file 404 is never doubted. Once M1 is fixed, doubt the project file whenever the commit is remembered (one call, already accepted). The rule is untested (mutations R25, R26 survive).
2. Gone commit + refused re-resolve: listDirectory returns [] with state: resolved, mayBeStale: false, and stays empty for the visit after the API recovers (:734-739). Throw the read error for the listing when the question was not answered and do not memoise an unanswered doubt; do not fall back to HEAD.
3. A gone commit is undetected when both its project file and listing are cached; DatatugStoreGithubService.summaryCache (datatug-store.service.github.ts:72-106) keeps the X summary after the reader moved to Y.
4. A read at the old commit that fails after the move surfaces its error instead of being read again (:688-691, :839-842); loop in the catch when the epoch changed.
5. Cache guard: a call started before the cache was proven can turn a working cache off (github/github-file-store-api.ts:148-155).
6. Slow device: the memo and listing are still never kept across visits when the database opens after the first wait (putResolved/putFile return empty while paused).
7. Test gaps: nothing pins that asking again cannot repeat (R32).
8. mirrorUsed can be set by an old-commit read after moveTo reset it (:883).

Judgement call (a), project file error when raw is refused and the mirror says 404: correct.

VERDICT: blockers=0 majors=1 minors=8 land=no

🤖 Generated with Claude Code

[trakhimenok]

[fix r1 #182]

Fix round for review r1, head e37de9aef05df2cc3bb87d1de3b098c860b8c9e7 (was 4ea3f69a9bac1df6b542810d98ac8580c6a34e3b). Paths under libs/datatug/main/src/lib/services/repo/. Each item had its test written first and seen red against the old code. About 18 hand mutations of the new code were run: all killed in the end (two survived the first run: an equivalent branch in the guard, removed, and the epoch's generation, for which an assertion was added).

M1. github/github-project-reader.service.ts readAtCommit (shared by loadText and loadTree): after the read at the session's commit it waits for a pending doubt of that commit (bounded by the 5 s resolve timeout), then does the epoch check and reads again if the visit moved. readInfo waits for the doubt too and names the commit after it. Tests (github/github-project-reader.followup.spec.ts, "a move of the visit while other reads are on their way"): Q9 and Q10 as assertions, with a delivered list that must stay empty while the question is on its way and everything at the new commit after; the old test that asserted the stale listing is rewritten. A hung question: with fake timers, nothing is delivered at 4999 ms, everything at 5000 ms at the remembered commit, readInfo included.

Minor 1. The project file 404 at a remembered commit is doubted whenever the commit is remembered. The confirmed set and doubtedByProjectFile are gone. Tests: Q2 (project a viewed, a push adds b, b opens on the first try, exactly one API call), a file or the listing read from the network or from the cache before the project file 404 is still doubted (these kill the R25/R26 family), asked once per commit per visit (a second project of the repo is not asked again), a commit answered in this visit is not doubted.

Minor 2. loadTreeAt case 'missing': the doubt now returns moved | stands | refused | unanswered. Refused fails the listing with the rate-limit error (what a refused listing gives), a failure or timeout with GithubReadError('the project listing', [api.github.com]), never []. An unanswered doubt is deleted from the memo, so a later read in the visit asks again. No fallback to HEAD (asserted: no /trees/HEAD request). Test Q4 with refused, 500 and timeout, including recovery in the same visit at the new commit. The project file read at that commit stays "absent" when the question is unanswered (unchanged).

Minor 4. readAtCommit catches a failure and, when the epoch changed (after waiting for the doubt), reads again at the new commit, at most 3 times. Test Q5, and a test that a failure at the commit the visit stays at is thrown once.

Minor 5. github/github-file-store-api.ts guarded: the wait of a call that began before the cache was proven no longer turns a proven cache off (beganProven). Test Q6.

Minor 3, second half. New read-only GithubProjectReaderService.visitEpoch(projectId) (generation.epoch, starts nothing). DatatugStoreGithubService.summaryCache (datatug-store.service.github.ts) keeps each summary with the epoch it was created at and reads again when it changed. Tests in the service spec (mocked reader) and in the reader spec (real reader: the X summary is not served after the visit moved, and is kept again from there on; the epoch changes on a move and on forget, not on an ordinary read). The existing mocks of the reader in datatug-store.service.github.spec.ts gained visitEpoch.

Minor 7. Test "a re-resolved commit is not marked as remembered" (a 404 of the project file at the commit GitHub named is not asked about again): fails with R32.

Minor 8. loadTextAt sets mirrorUsed only when the epoch is still the one the read started at. Test: a mirror answer arriving after the move leaves fromMirror: false.

API calls, from tests (API calls of a visit in the follow-up spec): cold 2 (commit, listing); warm under 5 minutes 0 (nothing at all); warm over 5 minutes, unchanged 1 (the commit); project changed 2 (the commit, the new listing); five new absent optional files on a warm visit 0 (5 file-host reads); repository with no project file 1 per page load, however warm (4 loads: 1, 1, 1, 1); gone commit 3 (the old listing, the commit asked again, the new listing).

Left as they are, as agreed: minor 3 first half (a gone commit undetected while both the project file and the listing are cached, for at most the memo's 5 minutes) and minor 6 (slow device: memo and listing not kept across visits when the database opens late). The cache database's version and schema are unchanged. Trust is unchanged: after any re-resolve the commit comes only from the API's answer for the ref.

One limit of M1 worth knowing: a read that completes at X before any question about X has been asked (for example a cache hit that finishes before the project file's 404 arrives) is delivered at X, as before; the fix covers reads that complete while the question is pending, as the review asked.

Checks run locally: targeted specs for services/repo and project/new-project (393 pass), lint for datatug-main and datatug-app (no findings), pnpm run check:zoneless. Full suite and e2e in CI.

🤖 Generated with Claude Code

[trakhimenok]

[review r2 #182] Second review (Opus): all earlier probes (P1-P12, Q1-Q10), seven new ones, a comparison on main, 26 mutations on the delta (25 killed), 393 targeted tests.

Reviewed-Head: e37de9a

M1 and the minors that were to be fixed are fixed; trust, the forget generation, the per-ref drop and the cache database's version and schema are intact. In no sequence built is an existing user worse off than on main. Paths under libs/datatug/main/src/lib/.

Minors, tracked on #180:

1. A listing delivered at the remembered commit before the project file's 404 raises the question stays on screen after the visit moves (services/repo/github/github-project-reader.service.ts:710-744; reachable because DatatugNavContextService.setCurrentProject emits the context before asking for the summary). Precondition: a project pushed from elsewhere under 5 minutes ago into a repo this browser resolved under 5 minutes ago. Main gives the identical result. Fix: hold listing delivery at a remembered commit until the visit's project file has answered, or re-read list pages when visitEpoch changes.
2. readInfo asked before the question exists names the old commit (:655-678).
3. visitEpoch does not reach the page header: services/project/project.service.ts:83-107 keeps the first summary for the page load.
4. A warm valid project waits up to 5 s when a missing project path of the same repo raised a question and commits/ hangs (:727); main waits the same.
5. When the question is refused, readInfo says resolved, mayBeStale: false for a commit GitHub did not confirm; suggest mayBeStale: true.
6. The "at most 3 times" re-read cap is unreachable (:334, :736); dead guard.
7. The summary cache stores its entry under the epoch read before its own read (services/repo/datatug-store.service.github.ts:69, :109); one extra in-memory read, no network.

Not verified: a real browser (ordering of IndexedDB against the network is inferred from the call order with gated fakes).

VERDICT: blockers=0 majors=0 minors=7 land=yes

🤖 Generated with Claude Code