Skip to content

fix(core): scope github tree fetches and surface remote source health - #68

Merged
siracusa5 merged 2 commits into
mainfrom
c/confident-carson-9fe43f
Jul 29, 2026
Merged

fix(core): scope github tree fetches and surface remote source health#68
siracusa5 merged 2 commits into
mainfrom
c/confident-carson-9fe43f

Conversation

@siracusa5

Copy link
Copy Markdown
Collaborator

Summary

  • Path selectors on a github layer now scope the GitHub tree-listing request itself (GET .../git/trees/{ref}:{dir} per selector prefix) instead of filtering the whole-tree response after the fact. A repo whose whole-tree listing permanently truncates (GitHub's ~100k entry / 7MB limit) now indexes normally as long as its selectors have a static directory prefix — the default selectors (CLAUDE.md, docs/**, .context/**, etc.) always do.
  • A selector with no static prefix (**/*.md) still falls back to a whole-tree request, and a truncated response is still refused (never published as a partial index) — now naming which scope truncated.
  • The github adapter swallows API failures by design (warn-and-continue: an unreachable repo must not fail a resolve), which made "empty repo" and "unreachable repo" indistinguishable at every read API. It now exposes health() — the last swallowed failure, credential-scrubbed — forwarded through the cache wrapper.
  • POST /api/sources/sync returns 502 with lastError/lastErrorAt/lastSuccessAt when the remote is unreachable, instead of 200 {ok:true}.
  • GET /api/graph reports a new "degraded" status (serving, but its last request failed) alongside "ok"/"error", so the playground and console Sources tables stop showing a green row over an outage. Both UIs updated: new .status--warn treatment in the playground, degraded mapped through adaptSources in the console with the failure + staleness surfaced in the focus line.

Both were called out as deliberately deferred follow-ups from #65.

Test plan

  • npm test from repo root (12 engine suites) exits 0
  • cd apps/console && npm test (69 tests) exits 0
  • cd apps/console && npm run typecheck exits 0
  • packages/core/tests/github-source-test.sh extended: scope derivation (treeScopes), a bigrepo fixture mode where only the whole-tree listing truncates (the real monorepo shape), subtree-truncation refusal naming the scope, missing/all-missing selector scopes, path-traversal guard on selector prefixes, adapter health() end to end (including that it never changes the degraded read path or echoes the token), and a full service round-trip: healthy Sync → forbidden → degraded Sync (502) + degraded /api/graph row → recovered Sync + /api/graph row, with credential-leak checks on the graph row
  • packages/core/tests/service-test.sh extended: a healthless source (local bundle) reports plain ok with null health fields, so degraded never appears for an adapter that doesn't report it
  • apps/console/src/api.test.ts extended: adaptSources maps a degraded row to its own status (not healthy) with the failure + staleness in focus, and gives a zero-concept source an empty coverage bar regardless of how it got there

🤖 Generated with Claude Code

siracusa5 and others added 2 commits July 28, 2026 22:12
Large repos indexed as permanently empty because a truncated whole-tree
listing was refused outright. Path selectors now scope the request itself
(GET .../git/trees/{ref}:{dir} per selector prefix) instead of filtering
the response after the fact, so a repo past GitHub's tree-listing limit
still indexes as long as its selectors have a static directory prefix.

The github adapter also swallows API failures by design (warn-and-continue),
which made an unreachable repo indistinguishable from an empty one at every
read API. It now exposes health() with the last swallowed failure, forwarded
through the cache wrapper. Sync returns 502 with the failure detail instead
of 200 {ok:true}, and /api/graph reports a new "degraded" status (serving,
but its last request failed) so the playground and console Sources tables
stop showing a green row over an outage.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…cope work

- degraded status now requires an "index"-scope failure, not any failure —
  a single denied file (content scope) no longer paints an otherwise-healthy
  repo as down; that file already falls through the cascade on its own.
- a content-scope lastError now clears on the next successful read of that
  file, matching what the header comment already promised, instead of
  lingering until the index's own TTL happens to expire.
- a layer whose only selector names a directory the repo doesn't (yet) have
  is a quiet empty index again, not a hard failure — that was punishing the
  exact narrow-selector config this feature exists to enable on an oversized
  repo. A genuinely bad ref is still announced, disambiguated by one extra
  root probe only in the otherwise-ambiguous case.
- scrub() now also redacts fine-grained PATs (github_pat_...), not just the
  classic prefix, since lastError now reaches an HTTP response body and not
  only stderr.
- CLAUDE.md's github.mjs row updated; it still described one recursive tree
  call.

Found by two Opus review agents given the diff cold (one per finding,
independently corroborated by a second read where relevant) and confirmed by
tracing the code by hand and writing tests that exercise each fix end to end
before landing them.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@siracusa5
siracusa5 merged commit 9c4596e into main Jul 29, 2026
7 checks passed
siracusa5 pushed a commit that referenced this pull request Jul 29, 2026
Integrates the github source adapter (#65), git-history section dates (#67)
and remote source health (#68) with background indexing and the utilityProcess
engine. Four conflicts, all union rather than either-or:

- okf-local: keeps main's commit-date history AND the async/bounded walk.
  documentDate's mtime fallback is now an async stat, since it runs in the
  read path the desktop engine must not block on.
- files: keeps the shared parseDocument seam AND the async parseFile. Dates go
  through localDate, so a folder source and a repo source agree on the calendar
  day instead of one of them slicing UTC.
- service: index state and adapter health are orthogonal, so both are reported.
  A source is 'indexing' until read, then 'degraded' if its adapter says its
  last request failed, then 'ok'. Resolution runs over anything with a
  snapshot, so a degraded source keeps contributing — main's warn-and-continue
  intent, expressed through the snapshot model.
- types.ts: GraphSource carries indexing progress and the health timestamps.

Two integration defects the merge exposed, both fixed:

- httpError lost its `detail` argument when the HTTP internals moved to
  http-util.mjs, so a failed remote Sync answered with a bare message instead
  of ok:false and the health timestamps the API promises.
- github-source-test asserted a settled source status straight after a Sync.
  Sync re-indexes, and reads answer from the background index, so the
  assertion raced the re-read. It now passes ?wait=, the same contract the
  service and playground suites use.

Verified on the merged tree: engine 13/13 suites, console typecheck + 125
tests + build, site build, desktop unit (34) + navigation + cli-status +
smoke + isolation (2ms main-loop lag while indexing 2,500 documents).
siracusa5 added a commit that referenced this pull request Jul 29, 2026
Setup froze the Mac app on "Resolving…" until it stopped responding: an
unbounded synchronous directory walk, every file read and tokenized twice per
request, no timeout anywhere, and the engine sharing an event loop with the
window that draws.

- Sources are read by background jobs. /api/graph and /api/resolve-all answer
  from what is indexed right now and report `indexing`/`indexingSources`;
  clients render what they have and poll. Reads are partial by design, so
  completeness assertions pass ?wait=. First graph request against a
  901-document folder: ~6ms.
- The engine runs in its own Electron utilityProcess. Worst-case main-loop
  stall while indexing 2,500 documents: ~694ms sharing the loop -> ~30ms
  isolated, enforced by a new isolation test in CI.
- Indexing limits are manifest-backed settings (Settings -> Indexing), not
  environment variables. Precedence is manifest > env > default.
- New Files view: browse and edit the context files behind each source, as
  rendered Markdown or raw text. The layer file APIs moved into the engine so
  the desktop app can reach them, and now cover markdown-folder layers.
- Markdown renders as typed data through React elements — no HTML string, no
  dangerouslySetInnerHTML, no hand-rolled sanitizer.
- Adding a source only rejects what the user can fix on the form. A too-large
  folder is accepted, then surfaces as a visible source error pointing at
  Settings.
- An unexpected engine exit is fatal, taking the same clean dialog-and-exit
  path as a failed boot.
- The Mac app icon is regenerated from the canonical brand mark via
  `npm run icon`; the stale hand-maintained master is gone.

Integrates the github source adapter (#65), git-history section dates (#67)
and remote source health (#68): index state and adapter health are orthogonal,
so a source reads `indexing` until read, `degraded` if its adapter reports its
last request failed, then `ok`.

Reviewed adversarially in a second session, which found real defects here —
a rejected oversized body that kept buffering, a containment guard whose
realpath result was discarded, and an inline SVG preview path. All fixed,
along with revision-aware saves, cooperative index cancellation, and moving
the API bearer out of renderer argv onto trusted IPC.

Validation: engine 13/13 suites, console typecheck + 125 tests + build, site
build, desktop unit/navigation/cli-status/smoke/boot-failure/isolation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant