Skip to content

fix(selfhost): correlate Codex provider timeouts with job/PR context - #3315

Merged
JSONbored merged 1 commit into
mainfrom
fix-codex-timeout-fields-and-fallback-test
Jul 5, 2026
Merged

fix(selfhost): correlate Codex provider timeouts with job/PR context#3315
JSONbored merged 1 commit into
mainfrom
fix-codex-timeout-fields-and-fallback-test

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • logSelfHostAiProviderFailed (and the in-chain selfhost_ai_provider_failed_in_chain warn log in createChainAi) only ever carried provider/model/effort/timeoutMs/error. There was no way to correlate a Codex/Claude Code failure log line back to the review that triggered it — no job id, no repo, no PR number, no attempt number. This PR threads those four fields, when actually available, from the AI review pipeline down into the self-host provider layer.
  • Concretely: AiRunOptions (self-host provider options) gains optional jobId / repoFullName / pullNumber / attempt; runWorkersOpinion (the caller that invokes env.AI.run) now forwards repoFullName/prNumber from GittensoryAiReviewInput plus its own per-attempt retry counter; runAiReviewForAdvisory and runGittensoryAiReview gained an optional deliveryId/jobId passthrough, sourced from the real inbound webhook delivery id at the one production call site (src/queue/processors.ts) — the closest thing this queue has to a job id. All four fields are optional and only appear in the log when the caller actually supplied them, so the log line is byte-identical to before whenever they're absent (e.g. a sweep/repair fan-out with no single originating delivery, or any existing caller/test that predates this field).
  • The dual-AI fallback chain (createChainAi) already correctly falls through from a failing Codex attempt to the next configured provider (e.g. Claude Code) — that behavior needed no code change. It previously had no regression test of its own pinning it, so this PR adds one: a Codex timeout followed by a successful Claude Code fallback, asserting the review still completes, both the provider-level and chain-level failure logs carry the new correlation fields, and no secret ever leaks into the logged error text.
  • No job id was invented — deliveryId is real, existing GitHub webhook-delivery-id data already threaded through src/queue/processors.ts for audit-event metadata at the exact call site; I forwarded the same value rather than adding new plumbing.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue, or this is small enough that the summary explains why an issue is not needed. (Small, self-contained observability improvement — no issue filed.)

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage locally; codecov/patch requires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate. (Every new line in src/selfhost/ai.ts, src/services/ai-review.ts, and src/queue/processors.ts is covered, including both arms of each new conditional, verified directly against the lcov report.)
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

Also ran (this change touches self-host env docs and observability):

  • npm run selfhost:env-reference (regenerated apps/gittensory-ui/src/lib/selfhost-env-reference.ts — the new lines in src/selfhost/ai.ts shifted line numbers for existing env var references; no env vars were added or removed)
  • npm run selfhost:validate-observability
  • npm run cf-typegen:check (clean — no wrangler binding/var changed)
  • npm run db:migrations:check / npm run db:schema-drift:check (clean — no schema change)
  • Full npm run test:ci once after the initial implementation, then typecheck + the two affected unit test files again after each subsequent rebase onto a fast-moving main.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (N/A — no auth/session/CORS surface touched.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (No public API/OpenAPI/MCP surface changed; ui:openapi:check confirms.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A — no UI change; the only apps/gittensory-ui file touched is the regenerated env-reference data file.)
  • Visible UI changes include a UI Evidence section below with screenshots. (N/A — this PR has no visible UI change.)
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. (Not touched.)

Notes

  • The new correlation fields (jobId, repoFullName, pullNumber, attempt) are forwarded purely for operator log correlation; the error/detail text itself still goes through the existing errorMessage() secret-redaction path unchanged, and the new regression test asserts no secret value appears in the logged output.
  • attempt in runWorkersOpinion is the reviewer's own per-model retry counter (0, 1, 2) already computed in that loop — forwarded as-is, not invented.

Thread the webhook delivery id, repo, PR number, and retry attempt from
the AI review pipeline down into the self-host provider chain so a
selfhost_ai_provider_failed line (Codex timeout, Claude Code failure,
or the in-chain fallback warn) can be matched back to the review that
produced it without cross-referencing timestamps. All four fields are
optional and forwarded only when the caller actually has them, so an
unconfigured or test caller keeps the log line byte-identical.

Also adds a regression test pinning the existing (and correct) Codex-
timeout-falls-through-to-Claude-Code chain behavior, which previously
had no test coverage of its own.
@superagent-security

Copy link
Copy Markdown
Contributor

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

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui c6b195f Commit Preview URL

Branch Preview URL
Jul 05 2026, 01:44 AM

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 5, 2026
@loopover-orb

loopover-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-05 03:05:12 UTC

5 files · 1 AI reviewer · no blockers · readiness 93/100 · CI green · unknown

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
This change threads optional AI-provider correlation context from the webhook processor through the advisory review path and into self-host provider failure logs, while preserving omission semantics when callers do not supply the fields. The visible production path uses the real webhook delivery id as `jobId`, forwards repo/PR context from `GittensoryAiReviewInput`, and the provider/chain logging changes are observational only. The regression coverage exercises the Codex-to-Claude fallback and the absent-context log shape, and the generated self-host env reference update is consistent with the line shifts.

Nits — 4 non-blocking
  • nit: `test/unit/selfhost-ai.test.ts` should restore `console.error` in a `finally` or use the test framework's restore hook so a failed assertion in this new regression test cannot leak the spy into later tests.
  • In `test/unit/selfhost-ai.test.ts`, wrap the body after `vi.spyOn(console, "error")` in `try/finally` with `errorSpy.mockRestore()` to keep the test isolated even when an expectation fails.
  • In `src/services/ai-review.ts`, consider adding a narrow unit assertion at the `runWorkersOpinion` boundary that `attempt` increments with retries and preserves the same `jobId`/repo/PR context across attempts, since that is the only new per-attempt field not covered by the provider-chain test.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 56 registered-repo PR(s), 46 merged, 423 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 56 PR(s), 423 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: Python, TypeScript, JavaScript, Ruby, Go, Kotlin, MDX, Shell
  • Official Gittensor activity: 56 PR(s), 423 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 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

@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.74%. Comparing base (1e9284b) to head (c6b195f).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3315   +/-   ##
=======================================
  Coverage   93.73%   93.74%           
=======================================
  Files         276      276           
  Lines       30381    30386    +5     
  Branches    11073    11076    +3     
=======================================
+ Hits        28479    28484    +5     
  Misses       1257     1257           
  Partials      645      645           
Files with missing lines Coverage Δ
src/queue/processors.ts 92.98% <ø> (ø)
src/selfhost/ai.ts 98.05% <100.00%> (+<0.01%) ⬆️
src/services/ai-review.ts 96.10% <100.00%> (+0.03%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

1 participant