feat(project): GitHub reader pins a commit, caches, fails over and enforces limits (G-A2) - #181
Conversation
…forces limits (G-A2) The reader resolved nothing and read `main`. Now (design demo-as-github-project.md 4.5, 3.6, 6.6): - one resolve call (`commits/<ref or HEAD>`) per repo and ref; the listing and every file are read at that one commit, so a push in the middle of a visit cannot mix two versions; the answer is remembered for 5 minutes across page loads; a full SHA in the id skips the call, never for a trusted project (it reads what GitHub says the default branch is now) - four-part ids (a branch, tag or commit) read instead of being refused; `main` is `HEAD` - the degrade table of 4.5, row by row: raw at the commit, jsDelivr at the commit when the file host is refused or down, a remembered commit when the resolve call fails, `HEAD` / unversioned jsDelivr when nothing is known (kept in memory only), a failed step naming the hosts - a file cache in a database of its own (`datatug-github-files`, version 1; never a new version of an existing database): keyed by owner/repo@sha/path, bounded at 20 MB, 20 repos and two commits per repo, least recently used out first; blocked or full storage is no cache, not a failure; opened from a lazy chunk - every request omits credentials, sends no referrer, follows no redirect; a renamed or moved repo (the API answers a redirect) is "No DataTug project here"; no host but the three - a 256 KB cap per project file counted on the bytes received (the transfer is cancelled past it), 40 distinct files per run (`GithubReadBudget`), a time limit per request - a project at the repo root lists from the root (`listDirectory` built the prefix `/`) - the project summary is read through the reader (one commit, one cache) Closes the G-A2 items of datatug-apps#180. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…d CI runner It took 3.5 s of its 5 s default timeout on main CI and 5.1 to 5.6 s on the G-A2 PR (the package ran more specs at once), failing the test job. About 1 s locally. No change to what it checks. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…le through the reader The project summary is now read through the GitHub reader (one commit, one cache) instead of a separate HttpClient read, so the spec serves it from a fake of GitHub. Missed locally by running targeted specs; found by the full CI run. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
[review r1 #181] Adversarial review (Opus): code read, 385 targeted tests, 47 probe cases and four mutations run on an export of this head. Reviewed-Head: fa6fe54 Held up under attack: commit pinning and trust (a fork commit never reaches the trusted read), absent-caching (403/429/5xx/network/redirect store nothing), byte caps on the stream, the host allow-list and path encoding, cache accounting across two tabs, rollback of the cache database. Paths below are under Majors
Minors
Not verified: a real browser (gzip bodies, real VERDICT: blockers=0 majors=3 minors=14 land=no 🤖 Generated with Claude Code |
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
S1: a project created in a repository this browser has already read opens. GithubProjectReaderService.forget(org, repo) drops the repo's sessions and the remembered resolve answers at every ref (IGithubFileStore.forgetResolved); DatatugStoreGithubService.forget drops its summaries; GithubProjectCreateService calls both once the files are committed, before registering and navigating. The absence of datatug-project.json is no longer kept in the cache. The files are committed to the repository's default branch (read from GitHub), not main. S2: the project summary is read through defer, so a failed read is read again. S3: every call to the cache is guarded (guardGithubFileStore, injectable timer GITHUB_TIMER, 1.5 s): a call that fails or times out is an empty cache, and a timeout turns the cache off for the visit. An IndexedDB open that is blocked and later succeeds is closed; every connection yields to a version change. M1/M5: raw refused or down and the mirror 404 is "the hosts did not answer", never absent, never kept. M2/M4: degrade rows 4 and 5 run a second visit with the API still refusing; the trusted-project test is named for what it asserts. M3: the unreachable trust clause is removed; the guarantee is in isTrustedGithubProject (no ref but HEAD is trusted). M7: a remembered commit GitHub does not know (listing or any raw 404 at it) drops the memory and is resolved again once. M8: a cached listing is validated. M9: an empty .json file is null. M10: the reader's observables are lazy (defer); the budget still counts distinct files. M11: the resolve call gives up after 5 s and the read degrades to HEAD. M14: fetch gets parsed.href. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
|
[fix r1 #181] Head: 6a8376c (two commits on top of fa6fe54: a lint commit and the fix commit). CI on this head: build, test, e2e and journey e2e all green. Paths are under Majors
Minors
Left, as asked
Checks run locally: targeted specs for the reader, store, file store, http, create service, new-project form, nav, folders and services (740 passed); 🤖 Generated with Claude Code |
|
[review r2 #181] Second adversarial review (Opus): 376 targeted tests, 35 mutations and 15 probe cases on an export of this head. Reviewed-Head: 6a8376c S1, S2, S3 and every minor claimed fixed are confirmed, each by a mutation that fails tests. The trust property holds (after a re-resolve the trusted project's commit comes only from Major, accepted as a tracked follow-up (must close before the demo is switched on; added to #180)
Minors (follow-up, same issue)
Not verified: a real browser (IndexedDB open time, VERDICT: blockers=0 majors=1 minors=6 land=yes 🤖 Generated with Claude Code |
…(G-A2 follow-up) (#182) 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/`. Source commits: - a733763 fix(project): reader keeps one commit per visit after an absent file (G-A2 follow-up) Refs #180 (A1 and minors 1-4 of the second review of #181). - A 404 at a remembered commit is no longer proof the commit is gone. The commit is doubted only when its listing answers 404/422, or when the project file is 404 and nothing has been read at it in this visit (from the cache or the network). Any other 404 is an absent file. - Asking again is asked once per commit. The persisted memory is dropped only after GitHub has answered (that the ref is gone or moved), for that ref only (new dropResolved), and replaced by the answer otherwise. A refusal or timeout keeps the remembered commit for the visit. - When the commit does change the whole visit moves: listing, files and decoded JSON read at the old commit are forgotten, reads still on their way are read again at the new one, and readInfo names the commit in use. - forget(org, repo) bumps a per-repository generation, so a resolve that started before it does not write its commit back. - Cache guard: the first call's wait (1500 ms) no longer turns the cache off for good. The calls after it answer empty at once, the opening has its own limit (5 s), and an answer within it switches the cache back on. The deletion of remembered answers is attempted even when the cache is off. - Raw refused and the mirror 404 at a pinned commit: an optional file is absent for this visit, not kept; the project file and unversioned reads keep the error. - 4ea3f69 fix(project): no assumed default branch; drop a redundant defer (G-A2 follow-up) Refs #180 (minor 6 of the second review of #181). - toGithubRepo no longer falls back to 'main' when GitHub's payload has no default_branch: the repository is incomplete. requireGithubRepo throws a GithubRepoError saying what GitHub did not return; createRepo and the project creation use it, and the new project form shows its message. (listRepos leaves out a repository it cannot commit to.) - DatatugStoreGithubService.getProjectSummary: the defer around getRawJson was redundant (the reader's observable is lazy and a failed read is read again by the next subscriber); the existing S2 test, which fails the project file once and then recovers, passes without it. - e37de9a fix(project): GitHub reader serves one commit per visit (review r1 of #182) Fix round on the review of #182 (M1, minors 1-5, 7, 8). Paths under libs/datatug/main/src/lib/services/repo/. - M1: a read that completes while "which commit?" is being asked is not delivered at the old commit. loadText and loadTree share readAtCommit, which waits for a pending doubt of the commit it read at (bounded by the 5 s resolve timeout) before the epoch check, and reads again if the visit moved. readInfo waits for the doubt too, so it never names a commit that is about to be replaced. - Minor 1: the project file being 404 at a remembered commit is always doubted (one API call; the same commit named again means the project is absent and the commit stands, asked once per commit). The "confirmed" commits bookkeeping is gone: a project pushed from elsewhere into a repo already viewed opens on the first try. - Minor 2: a gone commit (listing 404/422) whose re-resolve is refused or times out fails the listing read with the refused-listing error (rate limit) or the read error, never []; an unanswered doubt is not memoised, so a later read in the visit asks again and recovers. No fallback to HEAD. - Minor 4: a read at the old commit that fails after the visit moved is read again at the new one (bounded). - Minor 5: the wait of a cache call that began before the cache was proven no longer turns a proven cache off. - Minor 3 (second half): DatatugStoreGithubService keeps each cached summary with the reader's new read-only visitEpoch(projectId) and reads again when it changed (the visit moved, or the repository was forgotten). - Minor 7: tests pin that a re-resolved commit is not remembered. - Minor 8: an old-commit read after the move no longer sets mirrorUsed. Left as they are: 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. Pull request: #182 Review: #182 (comment)
What
G-A2 of
docs/design/demo-as-github-project.md(datatug/backstage): the GitHub reader pins a commit, caches what it read, fails over to jsDelivr and enforces the limits of 3.6. Public method signatures are unchanged (new optionaloptionsargument ongetRawJson/getRawText), so no project page changes. Not part of this PR: routes,index.html,main.ts, the demo hand-off, the chat page and its IndexedDB (datatug-chat-sessionsstays at version 2, untouched).How a project is read now
api.github.com/repos/<o>/<r>/commits/<ref or HEAD>,Accept: application/vnd.github.sha), remembered 5 minutes across page loads. A full SHA in the id skips it, never for a trusted project (3.6 point 3: a trusted run reads what GitHub says the default branch is now; trust isisTrustedProjectAddress, so a trusted id never has a ref).git/trees/<sha>?recursive=1) and every file (raw.githubusercontent.com/<o>/<r>/<sha>/<path>) come from that one commit. The project summary now goes through the reader too (it used its own HTTP read, which could come from another commit).datatug-github-filesv1 keeps files, absent files and the listing byowner/repo@sha/path; bounded 20 MB, 20 repos, two commits per repo, least recently used out first. It is opened from a lazy chunk. Blocked, full or missing storage means no cache, never a failed read.cdn.jsdelivr.netat the same commit.HEAD(memory only), else unversioned jsDelivr (memory only), else a failed step naming the hosts.readInfo(projectId)reportsstate(given | resolved | remembered | unresolved | missing | moved),commit,fromMirror,mayBeStalefor the later "sources" line.mainisHEAD; four-part ids (branch, tag, commit) read instead of being refused.Limits and rules (3.6)
credentials: 'omit',referrerPolicy: 'no-referrer',redirect: 'error',httpson exactly three hosts (api.github.com, raw.githubusercontent.com, cdn.jsdelivr.net), 20 s timeout including the bodyGithubReadBudget(cache hits count; a shell read passes no budget, so existing pages are not limited by it)GithubProjectNotFoundError, reasonmoved)What an existing user can observe (
datatug-demo-projects@datatug@demo-project-1)Measured against real GitHub, Chromium, full side-menu walk (
github-store.spec.ts, first test):api.github.comraw.githubusercontent.comSo: one more rate-limited call on a first visit (30 first visits an hour per address instead of 60), and none on return visits; files and listing are one commit; the project summary is no longer a separate read. Files are served from the local cache on return visits. Error text for an exhausted file host now names the hosts (it used to say "GitHub API rate limit"). A project at the repo root now lists (it listed nothing). Nothing else visible: the e2e suite passes unchanged.
datatug/chinook-demo(project at the root), measured: cold shell walk (project, entities, environments, queries) 2 API + 13 raw; warm 0.Acceptance, item by item
github-project-reader.service.tsresolveCommit,loadTree,loadTextcommit.spec"a push in the middle of a visit…", e2e budget test (one sha in every raw URL and the listing)resolveCommitcommit.spec"the commit of an address (3.6, point 3)"loadText,resolveCommitcommit.spec"the degrade table, row by row" (rows 1 to 6 plus the cached listing)github-file-store.tsgithub-file-store.spec(eviction plan table; over IndexedDB)github-file-store.tsheader,openDatabasegithub-file-store.spec"rollback-safe": the chat database made by the chat service is untouched at v2 and still lists its chat after the cache ran; a later-version cache database is still used; a later build's upgrade is not blockedgithub-http.tsreadBodyLimited,GithubReadBudgetgithub-http.spec(cancelled after the cap whatever the headers say; exactly cap passes; bytes not characters),commit.spec"limits"githubGet(redirect),resolveCommitgithub-http.spec,commit.spec"requests"assertReadableGithubProjectIdreader.spec"a project id the reader reads at a ref"listDirectoryprefix (issue 180)listDirectory,getQueryreader.spec"lists a project at the repo root"; realchinook-demolists its queriescommit.spec"the cache and the request budget" (demo project and the 4.3 file set), e2e "request budget (G-A2)"Bundle (production build,
nx run datatug-app:build)Initial total 470.51 kB to 470.10 kB estimated transfer (2.09 MB raw, unchanged). The reader's lazy chunk: 13,865 to 19,509 bytes raw (+5.6 kB, +2.1 kB gzip). New lazy chunk with the IndexedDB code: 3,220 bytes raw (1.5 kB gzip), loaded on the first GitHub read only. All JS: 6,085,197 to 6,094,788 bytes raw.
Verification
nx test datatug-main datatug-app(full, with coverage): 139 files / 1910 tests and 13 files / 352 tests pass. Two small test-only follow-up commits: the GitHub-store root-folder spec now serves the project file through the reader's fake fetch, and the exhaustiveproject-urlround-trip spec (3.5 s of its 5 s timeout on main CI) gets a 60 s timeout.nx run-many -t lint -p datatug-main datatug-app: clean;pnpm run check:zoneless: ok; production build: okgithub-store.spec.tsincl. the new budget test,label-spacing.spec.ts), Chromium: 10 of 10 passNot in this PR (issue 180)
The
CheckedDataUrl/tableUrlstrust items and thehttp://localhostallowance of the federated executor belong to G-A3b: there is no fetch layer for outside data yet to wire them into. Closed here: resolved commit,listDirectoryroot prefix, 256 KB cap on the stream, redirect refused, four-part ids.Design versus reality
raw.githubusercontent.comand jsDelivr answer200for a renamed repo (measured); only the API redirects. So the refusal is enforced on the API calls, whereredirect: 'manual'is used to tell a redirect from an outage (it is still never followed). Withredirect: 'error'a rename would look like an outage and degrade into reading the renamed repo. The file hosts keepredirect: 'error'.🤖 Generated with Claude Code