fix(fetcher): paginate GraphQL connections + MAX_FETCHED_* caps (closes #66) (#95)

Co-authored-by: chrisleekr-bot[bot] <2890262+chrisleekr-bot[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: chrisleekr-bot[bot] <chrisleekr-bot[bot]@users.noreply.github.com>

Author: 3 people

IMPLEMENT.md

@@ -1,150 +1,56 @@
-# IMPLEMENT — Issue #58 (supply-chain attestations)
+# Implement: issue #66 — paginate GraphQL fetcher + MAX_FETCHED\_\* caps
 
 ## Summary
 
-Wires SLSA v1 build provenance + CycloneDX/SPDX SBOMs into every Docker image
-the release pipeline publishes, and adds a hard regression gate that fails the
-workflow if either attestation is silently dropped by a future refactor. All
-changes are additive YAML (`.github/workflows/docker-build.yml`) plus matching
-docs — no `Dockerfile.*`, `package.json`, or `src/` change. Closes #58.
-
-The fix has two layers:
-
-1. **BuildKit-native** (`provenance: mode=max` + `sbom: true` on the build
-   step): each per-arch leaf push gets an in-toto SLSA v1 provenance manifest
-   and an SPDX 2.3 SBOM stored as OCI subject descriptors, and the merge
-   step's `imagetools create` walks each index digest so the descriptors
-   survive the manifest-list assembly without any extra CLI plumbing.
-2. **GitHub-Sigstore** (`actions/attest-build-provenance@v4` +
-   `actions/attest-sbom@v4` after the merge): bind Sigstore-signed
-   attestations to the merged manifest digest, surfaced via the GitHub
-   Attestations API and Docker Hub's "Build attestations" badge. The
-   CycloneDX SBOM that flows into `attest-sbom` is generated from the
-   merged image by `anchore/sbom-action@v0`.
-
-The `scan` job runs `gh attestation verify` for both predicate types
-(`https://slsa.dev/provenance/v1` + `https://cyclonedx.org/bom`) before Trivy
-on every release tag — two separate calls so each predicate must exist.
+Closes #66 (`fix(pipeline): GraphQL fetcher silently truncates PR/issue context past 100 items`).
+
+`src/core/fetcher.ts` previously issued single-page GraphQL requests with `first: 100` on every connection (issue/PR comments, reviews, the inline comments nested under each review, and changed files). Anything past the first 100 items was silently dropped before the data ever reached the agent — the prompt looked complete but was missing context, which is the failure mode the issue calls out.
+
+The fix:
+
+1. Every paginated query exposes a single cursor variable named exactly `$cursor` and selects `pageInfo { hasNextPage endCursor }` on its one paginated connection. Both names are part of `@octokit/plugin-paginate-graphql`'s contract: the plugin hard-codes the cursor parameter name (`iterator.js`) and runs a depth-first search for the first `pageInfo` it finds (`object-helpers.js`), so multi-connection / aliased-cursor queries silently truncate or throw. The PR fetch is therefore split into three parallel `paginate(...)` calls (`PR_FIRST_QUERY` for top-level scalars + files, `PR_COMMENTS_QUERY`, `PR_REVIEWS_QUERY`); the issue fetch uses one. The plugin is bundled with `octokit ^5.0.5` and exposed on every `octokit.graphql` instance.
+2. A separate `REVIEW_COMMENTS_QUERY` walks each review's overflow inline comments keyed on the review node ID — the nested per-review pagination cannot ride along on `PR_REVIEWS_QUERY` because of contract-rule #1.
+3. Four new env vars (`MAX_FETCHED_COMMENTS` / `_REVIEWS` / `_REVIEW_COMMENTS` / `_FILES`, default `500` each) cap the merged result. When a cap fires the fetcher emits `log.warn({ connection, fetched, cap })` and sets `FetchedData.truncated.<connection> = true`.
+4. `buildPrompt` in `src/core/prompt-builder.ts` reads `data.truncated` and prepends a `WARNING: pre-fetched context is incomplete…` banner naming the affected connections, so the agent knows to reach for the GitHub CLI when it needs the missing items.
+5. The TOCTOU filter (`filterByTriggerTime`) runs after the paginate merge, so its semantics are unchanged.
+
+## Plan deviations
+
+- **T1 / T2 (install + thread plugin) skipped as no-ops.** `octokit ^5.0.5` already bundles `@octokit/plugin-paginate-graphql` and exposes `octokit.graphql.paginate` on every existing instance. Verified via lockfile inspection and a runtime probe. Threading a shared factory through 11 instantiation sites would have been pure churn for zero behaviour change.
 
 ## Files changed
 
-- `.github/workflows/docker-build.yml` · primary subject of the issue —
-  enables BuildKit attestations on the build step (T1), scopes
-  `id-token: write` + `attestations: write` to the merge job only (T3),
-  captures the merged manifest digest, generates a CycloneDX SBOM, and
-  publishes Sigstore-signed provenance + SBOM attestations after the
-  manifest-list push (T4); adds a `gh attestation verify` regression gate
-  to the scan job before Trivy (T5). Top-level perms gain
-  `attestations: read` so build/scan stay read-only; merge overrides locally.
-- `docs/operate/deployment.md` · new "Verifying image attestations"
-  subsection under "Build" with consumer-side `gh attestation verify` and
-  `docker buildx imagetools inspect` recipes covering both attestation
-  flavours and both image variants (T7).
-- `docs/operate/observability.md` · new "Supply-chain attestations" section
-  documenting the registry / Sigstore / GitHub-API storage matrix and
-  pointing operators at the consumer commands in `deployment.md` (T8).
-- `CLAUDE.md` · "Owns" cell for `docker-build.yml` updated to mention SLSA + SBOM attestations and the `gh attestation verify` regression gate; one-line `20260502-supply-chain-attestations` entry in "Recent Changes" (T9).
+- `src/core/fetcher.ts` · rewrote both queries to select `pageInfo`, switched to `graphql.paginate`, added `applyCap()` + nested review-comment follow-up; emits structured warn logs and sets `FetchedData.truncated` flags.
+- `src/types.ts` · extended `FetchedData` with `truncated?: { comments?, reviews?, reviewComments?, changedFiles? }`.
+- `src/config.ts` · added 4 zod fields + env wiring (`MAX_FETCHED_COMMENTS` / `_REVIEWS` / `_REVIEW_COMMENTS` / `_FILES`, default `500`).
+- `src/core/prompt-builder.ts` · added `buildTruncationBanner` and injected it into the prompt when any flag is set.
+- `test/core/fetcher.test.ts` · new tests covering pagination merge (length > 100), TOCTOU after merge, cap fire (log + flag), nested review-comment pagination, and banner injection.
+- `test/factories.ts` · extended `makeOctokit` with `graphqlPaginateResponses` (substring-keyed routing on the paginate fn).
+- `docs/operate/configuration.md` · 4 new rows documenting the env vars.
+- `docs/operate/observability.md` · new "Data fetching safety caps" section documenting the warn log shape and prompt banner.
 
 ## Commits
 
-See the PR commit list — short SHAs and conventional-commit subjects are
-visible there.
+- `fix(fetcher): paginate GraphQL connections + MAX_FETCHED_* caps (closes #66)` — single commit, branch `fix/issue-66-paginate-graphql`.
 
 ## Tests run
 
-- `bun run typecheck` · pass (no TypeScript output, exit 0)
-- `bun run lint` · pass (0 errors, 289 pre-existing warnings — none new from this change)
-- `bun run format` · pass after `bun run format:fix` re-flowed two doc tables
-- `actionlint .github/workflows/*.yml` · pass (no output, all workflows lint-clean)
-- `bun run scripts/check-docs-citations.ts` · pass (every `src/<file>:<line>` citation in-range)
-- `bun run scripts/check-docs-versions.ts` · pass (Bun version pins consistent with `.tool-versions`)
-- `mkdocs build --strict` · pass (`Documentation built in 0.58 seconds`, no warnings)
-- `bun test` · 519 pass / 153 skip / **194 pre-existing fail** — verified
-  baseline by `git stash && bun test` before reapplying my diff: same
-  pass/fail counts. The failing suites need Postgres + Valkey
-  (`bun run dev:deps`); they are unrelated to YAML / Markdown changes here.
+- `bun run typecheck` · clean.
+- `NODE_OPTIONS='--max-old-space-size=4096' bunx eslint .` · 0 errors / 291 warnings (all pre-existing return-type warnings in unrelated files).
+- `bun run format` · clean.
+- `bun test test/core/fetcher.test.ts` · 30 pass / 0 fail / 64 expect calls.
+- `bun run scripts/check-docs-citations.ts` · clean.
+- `bun run scripts/check-docs-versions.ts` · clean.
+- `bun run docs:build` · skipped locally (`mkdocs` not installed in workspace); runs in CI via `.github/workflows/docs.yml`.
+- `bun run test` (isolated runner): 78 files passed, 25 files skipped because Postgres / Valkey are not running in this workspace (pre-existing infrastructure dependency, none of the skipped files were touched by this PR).
 
 ## Verification
 
-Each task in the plan is satisfied as follows:
-
-- **T1** — `.github/workflows/docker-build.yml:160-169` adds
-  `provenance: mode=max` and `sbom: true`. Inline comment cites the
-  `push-by-digest` default-off behaviour the issue called out and links the
-  Docker multi-platform guide. BuildKit will emit per-arch attestation
-  manifests alongside each leaf image push.
-
-- **T2** — Merge step preserved as-is (`imagetools create` already walks
-  the per-arch index digests, which now reference both image AND
-  attestation manifests via the BuildKit emission from T1). Inline comment
-  at `.github/workflows/docker-build.yml:248-256` explains why no CLI
-  plumbing is needed; this matches the documented Docker multi-platform
-  pattern.
-
-- **T3** — Top-level adds `attestations: read` (line 61) so the scan job
-  inherits read-only verification scope. Merge job overrides locally
-  (lines 199-202) with `id-token: write` + `attestations: write` —
-  least-privilege; build/scan retain only the top-level grants.
-
-- **T4** — After `Create manifest list and push`, the workflow now:
-  1. `Inspect merged image and capture digest` (lines 261-274) — captures
-     the index digest with regex validation so a malformed parse fails
-     fast rather than silently passing a bad subject to attest-\*.
-  2. `Generate CycloneDX SBOM for merged image` (lines 276-283) using
-     `anchore/sbom-action@v0` — syft-backed, produces CycloneDX JSON.
-  3. `Attest build provenance` (lines 285-290) — Sigstore-signed in-toto
-     SLSA v1 attestation pushed to the registry as a sibling descriptor
-     on the merged manifest digest.
-  4. `Attest SBOM` (lines 292-298) — Sigstore-signed CycloneDX SBOM
-     attestation, same subject digest.
-
-- **T5** — Scan job's new `Verify image attestations` step (lines 329-343)
-  calls `gh attestation verify` twice with explicit `--predicate-type`
-  filters for SLSA provenance and CycloneDX SBOM. Each call fails the job
-  if its predicate type is absent — so dropping either attestation in a
-  future refactor will break the release before Trivy runs.
-
-- **T6 (deviation noted)** — Plan said to SHA-pin in addition to the major
-  tag. Repo-wide `Grep` for `uses: .+@[0-9a-f]{40}` returned zero matches:
-  every workflow uses tag-only pinning at the major version, with Renovate
-  handling bumps via the `github-actions` group rule in `renovate.json`. I
-  followed the **existing repo posture** (tag-only major-version pins for
-  `actions/attest-build-provenance@v4`, `actions/attest-sbom@v4`,
-  `anchore/sbom-action@v0`) over the plan's SHA-pin recommendation, since
-  adding SHAs only here would be immediately undone by the next Renovate
-  run and breaks consistency with the other 11 actions in the file.
-  Renovate's `github-actions` group will pick up bumps weekly. Also bumped
-  the action major versions from the plan's `@v3` to `@v4` because v4.x
-  has been GA since 2026-02-26 (today: 2026-05-02) and the repo otherwise
-  tracks current major versions for actions (`checkout@v6`,
-  `build-push-action@v7`, `download-artifact@v8`).
-
-- **T7-T8** — `docs/operate/deployment.md` and `docs/operate/observability.md`
-  updated with consumer verification commands and storage-surface matrix;
-  cross-linked. `mkdocs build --strict` passes; the project's bespoke
-  citation / version checks pass.
-
-- **T9** — `CLAUDE.md` CI/CD row updated and a `20260502-…` "Recent Changes"
-  entry added. Format auto-fix re-flowed the table column widths, expected.
-
-- **T10 (deferred — out-of-band verification)** — End-to-end smoke test via
-  `gh workflow run docker-build.yml` against a dev tag is the maintainer's
-  call to schedule (it pushes a real image to Docker Hub and consumes an
-  attestations-API quota). All YAML / docs gates that _can_ run locally
-  pass; the actual attestation-emit / verify behaviour is the maintainer's
-  smoke test on first dev release after merge.
-
-### Security posture preserved
-
-- Top-level `permissions:` only grew by `attestations: read` (least
-  required to verify); the existing `contents: read` and
-  `security-events: write` are unchanged.
-- The merge job's elevated scopes (`id-token: write`,
-  `attestations: write`) are confined to that one job — build and scan
-  cannot mint Sigstore tokens or write attestations.
-- `gh attestation verify` runs with `secrets.GITHUB_TOKEN` (the default
-  job token), no PAT.
-- All dynamic inputs flowing into `run:` blocks remain passed via `env:`
-  first (defense-in-depth posture from CLAUDE.md preserved — the new
-  steps follow the same pattern, e.g. `IMAGE_REF` / `REPO` / `TAG` /
-  `IMAGE` / `DIGEST` env mappings).
+- **Acceptance criterion: PRs/issues with > 100 comments / reviews / changed files no longer truncate silently.** New test "merges paginated issue comments into FetchedData (length > 100)" proves the merge for 250 comments; "merges paginated review comments across nested pageInfo" proves the nested review-comment merge (100 + 50 = 150).
+- **Acceptance criterion: a hard cap protects the prompt window.** `applyCap` clamps to `config.maxFetchedComments` (etc.), emits a structured warn, and sets `truncated.<connection> = true`. Test "logs warn and sets truncated flag when MAX_FETCHED cap fires" asserts all three (length, log fields, flag).
+- **Acceptance criterion: the agent must know when context is incomplete.** `buildPrompt` injects a `WARNING: pre-fetched context is incomplete…` banner naming the affected connections. Test "buildPrompt includes truncation banner when truncated flag is set" asserts the banner.
+- **TOCTOU semantics preserved.** Test "applies filterByTriggerTime AFTER pagination merge" sets `triggerTimestamp` to comment-240 of 600 and asserts comment-239 (newest pre-trigger) survives while comments 240+ are dropped.
+
+### Intentionally NOT done
+
+- T1 / T2 (paginate-graphql install + threaded factory). Plugin is already bundled with `octokit ^5.0.5` and `octokit.graphql.paginate` is present on every existing instance. Documented above under "Plan deviations".