fix(graphql): set no-store on GraphQL error envelopes

[cleanjunc]

Summary

• A GraphQL POST that populated result.errors (a resolver hitting a failed KV/D1/R2 read) still returned the success cache directive public, max-age=60, stale-while-revalidate=300, so a transient backend error advertised itself as cacheable for 60s and served while stale for a further 300s.
• A GraphQL execution failure always comes back as a standards-compliant 200 envelope with a populated errors array, so it slips past the existing norm that only clean 200s are cached. Emitting no-store on that envelope keeps a fronting cache, or a directive-respecting client, from pinning a backend error for the success window.

What Changed

• handleGraphQLRequest now sets cache-control: no-store when result.errors is populated and keeps the success directive only for a clean result. The status and body stay byte-identical, and the SDL/GET response plus the pre-execution 400/405/413 paths are untouched.
• Added two focused tests in tests/graphql.test.mjs: a clean POST keeps the success directive, and an errored POST (a resolver read that throws, the realistic corrupt-artifact trigger) switches to no-store. The errored case fails on the prior code and passes with the fix, and the pair covers both branches of the new conditional.
• No schema, OpenAPI, or contract surface changed, so nothing was regenerated and validate:contract-drift stays clean.

Closes #2084

Registry Safety

• No secrets, PATs, wallet data, private dashboards, private URLs, or
  validator-local state.
• Generated artifacts were produced by repo scripts, not hand-edited.
• R2-only/high-churn detail artifacts are not committed.
• Public API/OpenAPI/schema changes are intentional and documented.

Validation

• npm run check
• npm run validate
• npm run validate:schemas
• npm run validate:api
• npm run validate:openapi
• npm run validate:types
• npm run validate:artifact-budgets
• npm run validate:docs
• npm run validate:intake
• npm run validate:workflows
• npm run worker:test
• npm run test:coverage
• npm run scan:public-safety
• git diff --check

[superagent-security]

Superagent didn't find any vulnerabilities or security issues in this PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.21%. Comparing base (3ad8ce2) to head (6022b09).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2199   +/-   ##
=======================================
  Coverage   93.21%   93.21%           
=======================================
  Files          52       52           
  Lines        8225     8226    +1     
  Branches     3018     3019    +1     
=======================================
+ Hits         7667     7668    +1     
  Misses         96       96           
  Partials      462      462           

Files with missing lines	Coverage Δ
src/graphql.mjs	97.30% <100.00%> (+0.01%)	⬆️

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[gittensory-orb]

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review — safe to merge

2 files · 1 AI reviewers · no blockers · readiness 55/100 · CI green · clean

✅ Approved — safe to merge

Review summary
Minimal, correct fix for a real bug: GraphQL error envelopes were being sent with `public, max-age=60, stale-while-revalidate=300` because the cache-control path didn't distinguish a GraphQL-level error (a 200 with a populated `errors` array) from a clean success. The conditional `result.errors?.length ? 'no-store' : ...` is exactly right — undefined and empty-array both coerce falsy, matching the spec guarantee that a non-empty `errors` array is the only error signal. The two new tests cover both branches, the errored case fails on the prior code and passes on the fix, and the pattern is consistent with how `no-store` is applied in `rpc-proxy.mjs`.

Signal	Result	Evidence
Code review	✅ No blockers	1 reviewers, synthesized
Linked issue	✅ Linked	#2084
Related work	⚠️ 2 scoped overlaps	Top overlaps are listed below; lower-confidence bulk is hidden.
Review load	❌ 8/20	Readiness component derived from cached public PR metadata and labels.
Validation evidence	❌ 5/25	Cached preflight status is hold.
Open PR queue	❌ 3/10	17 open PR(s), 15 likely reviewable, 2 unlinked.
Contributor context	✅ Confirmed Gittensor contributor	cleanjunc; Gittensor profile; 28 PR(s), 16 issue(s).
Gate result	✅ Passing	No configured blocker found.

Nits — 3 non-blocking

• tests/graphql.test.mjs: The explanatory comment above the error test ('readR2 parses the body outside its try/catch, so the rejection propagates') is a structural claim about a code path not shown in the diff — worth a quick spot-check against `readR2` in `src/graphql.mjs` to confirm the comment is accurate and won't silently break if `readR2` ever gains a wrapping try/catch.
• src/graphql.mjs:920: The inline comment is on the wordy side for this repo's style; the essential point ('GraphQL error is a 200 with a populated errors array; no-store prevents caching a transient failure') could be trimmed to a single clause — the conditional itself is self-documenting once the reader knows the GraphQL envelope contract.
• Consider also covering the `result.errors = []` (empty-array) edge case in tests to document that an empty errors array is intentionally treated as success — even though graphql-js never produces it, the explicit assertion would guard against a future spec drift.

Review context

• Author: cleanjunc
• Role context: outside_contributor
• Public audience mode: oss maintainer
• Lane context: Repository registration is not available in the local Gittensory cache.
• Public profile languages: Python, TypeScript, JavaScript, Scala
• Official Gittensor activity: 28 PR(s), 16 issue(s).
• Related work: Items reference the same linked issue GraphQL POST returns cacheable success directives (max-age=60, SWR=300) even when result.errors is populated #2084. (issue #2084)
• Related work: Open PR work references issue GraphQL POST returns cacheable success directives (max-age=60, SWR=300) even when result.errors is populated #2084. (issue #2084)

Contributor next steps

• Review top overlaps.
• Add scope summary.
• Fix blocker.
• Expect slower review.
• Refresh registry data or choose a registered active repo.
• Check active issues and PRs before submitting.

Signal definitions

• Related work = same linked issue, overlapping active PRs, or title/path similarity.
• Review load = cached public PR metadata such as size labels, changed paths, and preflight status.
• Open PR queue = repo-wide review pressure; it is not a PR quality failure.
• Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

Review details

Generated from public PR metadata and the diff. Advisory only; deterministic signals remain authoritative.

Minimal, correct fix for a real bug: GraphQL error envelopes were being sent with `public, max-age=60, stale-while-revalidate=300` because the cache-control path didn't distinguish a GraphQL-level error (a 200 with a populated `errors` array) from a clean success. The conditional `result.errors?.length ? 'no-store' : ...` is exactly right — undefined and empty-array both coerce falsy, matching the spec guarantee that a non-empty `errors` array is the only error signal. The two new tests cover both branches, the errored case fails on the prior code and passes on the fix, and the pattern is consistent with how `no-store` is applied in `rpc-proxy.mjs`.

Nits (3)

• tests/graphql.test.mjs: The explanatory comment above the error test ('readR2 parses the body outside its try/catch, so the rejection propagates') is a structural claim about a code path not shown in the diff — worth a quick spot-check against `readR2` in `src/graphql.mjs` to confirm the comment is accurate and won't silently break if `readR2` ever gains a wrapping try/catch.
• src/graphql.mjs:920: The inline comment is on the wordy side for this repo's style; the essential point ('GraphQL error is a 200 with a populated errors array; no-store prevents caching a transient failure') could be trimmed to a single clause — the conditional itself is self-documenting once the reader knows the GraphQL envelope contract.
• Consider also covering the `result.errors = []` (empty-array) edge case in tests to document that an empty errors array is intentionally treated as success — even though graphql-js never produces it, the explicit assertion would guard against a future spec drift.

_{🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed}

---

💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

• Re-run Gittensory review