Skip to content

refactor(api): the main app's raw fetch tail goes through fetchWithAuth, and ApiClient gains an opt-in circuit breaker for polled dashboards (#12363) - #18398

Merged
mrveiss merged 7 commits into
mainfrom
issue-12363-raw-fetch-tail
Oct 12, 2026
Merged

mrveiss merged 7 commits into
mainfrom
issue-12363-raw-fetch-tail

Conversation

@mrveiss

@mrveiss mrveiss commented Oct 12, 2026

Copy link
Copy Markdown
Owner

Refs #12363
Refs #11682
Refs #18241

Single-issue rationale

This is slice S3 of the one-ApiClient work under step 3 (#18241). It is the main app's raw-fetch tail, plus the zero-importer HTTP modules. The other slices are separate on purpose:

#12363 and #11682 stay open. #12363 still needs its generated-types phase and the SLM axios slice (S4). #11682 still has its own fold steps for the Vision and FeatureFlags clients.

Thinking Path

The owner rule is one canonical client per app (ADR-008), reused and never duplicated. In the main app that client is utils/ApiClient.ts, and fetchWithAuth is its helper.

The owner's decision for the zero-importer modules was: "assess each, wire useful parts in; otherwise remove with zero-caller evidence". The results:

  • ChatManager.js: 0 importers (static imports, dynamic import(), require, import.meta.glob, docs, stories, tests). It was a localStorage chat cache with 3 raw fetches, superseded by the chat store and ChatRepository. Removed.
  • ApiCircuitBreaker.js: 0 importers, but ApiClient had timeout and retry and no circuit breaking. The breaker is ported to utils/apiCircuitBreaker.ts and is opt-in per request via RequestOptions.circuitBreaker. Its thresholds come from the SSOT config. It is wired into the polled monitoring callers. Its fake-503 EnhancedFetch cache was a duplicate and is dropped.
  • models/repositories/ApiRepository.ts: kept. The plan's "0 importers" was wrong: KnowledgeRepository and SystemRepository extend it.

What Changed

Circuit breaker

  • utils/apiCircuitBreaker.ts opens after N failures (thrown errors or 502/503/504), stays open for the recovery window, then allows one half-open probe.
  • ApiClient.rawRequest honours circuitBreaker: '<key>', and ApiClient.get stops retrying on CircuitBreakerError. Requests that don't set the option behave as before.
  • Thresholds are read from ssot-config.ts (timeout.circuitFailureThreshold from VITE_CIRCUIT_FAILURE_THRESHOLD, default 3; timeout.circuitRecoveryMs from VITE_CIRCUIT_RECOVERY_MS, default 30000). The defaults are those of the old module.
  • Wired into the polled dashboards only, so a user-initiated action still sees its own error:
    • usePrometheusMetrics: dashboard, services and alerts pollers, plus useSystemMetrics, useServicesHealth and useAlerts; key monitoring.
    • useServiceMessages.fetchLatest: key service-messages.
    • useErrorMonitoring's seven GETs: key error-monitoring.
  • Not wired: useSystemStatus (it goes through apiEndpointMapper.fetchWithFallback, which has its own fallback), and interactive or POST callers.
  • _extractErrorInfo moved to utils/apiErrorInfo.ts. ApiClient.ts goes from 842 to 834 lines, and its ceiling is lowered in both baseline files.

Raw fetch sites moved onto fetchWithAuth, with behaviour deltas:

Site Delta
router/index.ts onboarding status Now also sends Bearer and X-Organization-Id. Credentials, Accept and fail-open handling unchanged.
GlobalWebSocketService.quickHealthCheck Adds auth headers. try/catch becomes try/finally with the same semantics.
LiveEventService.sendSteeringMessage Now sends Bearer (it relied on the cookie only). Method, body and credentials unchanged.
HealthMonitor.checkBackendHealth Adds auth headers. Cache-buster headers and the 8 s abort unchanged.
ApiEndpointMapper Adds auth headers. Cache, timeout and fallback unchanged.

fetchWithAuth was used rather than apiClient.rawRequest, which would have added the 401 logout redirect to health pings. slmClient.ts stays a raw fetch: it targets the SLM origin with its own token, and must not send the main-app JWT.

Ratchet: repo_tests/frontend_api_contract_ratchet_test.py gains _RAW_FETCH_EXCEPTIONS, a per-file allowlist where each entry carries a one-line reason.

  • It has 15 entries:
    • bootstrap: AppConfig.js, ServiceDiscovery.js, ssot-config.ts;
    • external or data URLs: DesktopInterface, AutobotWidget, RumAgent, thumbnailWorker, VideoProcessor, useConnectionTester;
    • the client's own transport: ApiClient, fetchWithAuth, slmClient;
    • documentation samples: two CodeCell stories and the useTimeout JSDoc.
  • Tests:
    • a stale or reasonless entry fails;
    • contrast pair on a tmp tree: an unlisted raw fetch is counted and a listed one is not.
  • The raw-fetch scan now also reads .js. The usePrometheusMetrics local fetch is renamed refresh, and the returned key stays fetch, so callers are unchanged.
  • Main app raw_fetch goes from 17 to 0 unlisted. clients stays at 8. SLM constants are unchanged; that's slice S4.

Docs (#11682): docs/developer/FRONTEND_API_CLIENTS.md explains when to use which client: ApiClient, fetchWithAuth, the circuit breaker, the domain clients, ApiService, BatchApiService, WorkflowBuilderApiClient and SlmClient. ApiClient.ts links to it.

Verification

  • npm run type-check (autobot-frontend) at head a972d24241: rc=0, 0 error TS, 193 s. Pushed with AUTOBOT_PREPUSH_ALLOW_TIMEOUT=1 (owner-approved). The hook's vue-tsc timed out at its 159 s budget under machine load (load average about 10), so I ran it manually above. In the hook, vitest and pytest passed.
  • Full npx vitest run: 3716 passed, 1 failed. The failure, WorkflowCanvas.orgMode, passed 21/21 when run alone; it's in the test(frontend): four vitest files fail in the full run but pass in isolation #18254 family.
  • After the breaker wiring: the config, utils and composables suites passed 99/100 files. The one failure, useActionQueue (a 10 s timeout under load), passed 4/4 alone.
  • pollerCircuitBreaker.test.ts: 3/3. For each wired caller, the call after the first failed one makes no network request.
  • New tests: apiCircuitBreaker.spec.ts (7), ApiClient.circuitBreaker.spec.ts (2, with a contrast case without the option), rawFetchMigration.test.ts (3).
  • pytest repo_tests/frontend_api_contract_ratchet_test.py frontend_file_size_ratchet_test.py frontend_fragmentation_ratchet_test.py symbol_fork_ratchet_17312_test.py at head a972d24241: 106 passed.

Model Used

Claude Opus 5.5 (coordinating). Sonnet subagents did the planning and the implementation.

🤖 Generated with Claude Code

…te ChatManager (#11682)

ApiCircuitBreaker.js had zero importers but circuit breaking is absent from
ApiClient, so it is ported to utils/apiCircuitBreaker.ts and exposed as the
opt-in RequestOptions.circuitBreaker. ChatManager.js (zero importers) is
superseded by the chat store/ChatRepository. ApiClient error extraction moves
to utils/apiErrorInfo.ts, keeping ApiClient.ts under its recorded size.
…uth (#12363)

Onboarding status check (router), GlobalWebSocketService health ping,
LiveEventService steering POST, HealthMonitor and ApiEndpointMapper keep their
URL, method, body, credentials and timeout handling and now carry the bearer
token and org header. Lowers the router and GlobalWebSocketService size ceilings.
…t ratchet (#12363)

Adds a reasoned per-file exception list to the existing guard, scans .js for
raw fetch, fails on a stale entry, and lowers the main-app raw_fetch baseline
from 17 to 0 unlisted files.
…callers, thresholds from SSOT (#11682)

usePrometheusMetrics (dashboard/services/alerts/metrics pollers) and
useServiceMessages.fetchLatest opt in; ApiClient.get no longer retries a
CircuitBreakerError. Thresholds move to ssot-config timeout config
(VITE_CIRCUIT_FAILURE_THRESHOLD / VITE_CIRCUIT_RECOVERY_MS, defaults 3 / 30000).
usePrometheusMetrics' local fetch() is renamed refresh() (public key stays
fetch), so its raw-fetch allowlist entry is dropped (#12363).
@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 39 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Repository: mrveiss/AutoBot-AI/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 69df1459-207b-40c0-b63d-91891b0bbf26

📥 Commits

Reviewing files that changed from the base of the PR and between 13be1b4 and b44255e.


⛔ Files ignored due to path filters (1)
  • pipeline-scripts/hardcoded_values_baseline.txt is excluded by !**/*_baseline.txt, !pipeline-scripts/hardcoded_values_baseline.txt

📒 Files selected for processing (23)
  • autobot-frontend/src/composables/__tests__/pollerCircuitBreaker.test.ts
  • autobot-frontend/src/composables/useErrorMonitoring.ts
  • autobot-frontend/src/composables/usePrometheusMetrics.ts
  • autobot-frontend/src/composables/useServiceMessages.ts
  • autobot-frontend/src/config/ssot-config.ts
  • autobot-frontend/src/router/index.ts
  • autobot-frontend/src/services/GlobalWebSocketService.ts
  • autobot-frontend/src/services/LiveEventService.ts
  • autobot-frontend/src/services/__tests__/rawFetchMigration.test.ts
  • autobot-frontend/src/utils/ApiCircuitBreaker.js
  • autobot-frontend/src/utils/ApiClient.ts
  • autobot-frontend/src/utils/ApiEndpointMapper.js
  • autobot-frontend/src/utils/ChatManager.js
  • autobot-frontend/src/utils/HealthMonitor.js
  • autobot-frontend/src/utils/__tests__/ApiClient.circuitBreaker.spec.ts
  • autobot-frontend/src/utils/__tests__/apiCircuitBreaker.spec.ts
  • autobot-frontend/src/utils/apiCircuitBreaker.ts
  • autobot-frontend/src/utils/apiErrorInfo.ts
  • docs/developer/FRONTEND_API_CLIENTS.md
  • repo_tests/frontend_api_contract_ratchet_test.py
  • repo_tests/frontend_file_size_ratchet_baseline.py
  • repo_tests/frontend_file_size_ratchet_test.py
  • scripts/frontend_file_size_known_large.py

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@mrveiss

mrveiss commented Oct 12, 2026

Copy link
Copy Markdown
Owner Author

Review (Sonnet) at a972d24: approve once one line is dropped.

Required: pipeline-scripts/hardcoded_values_baseline.txt:653 still lists autobot-frontend/src/utils/ChatManager.js, which this PR deletes. repo_tests/stranded_recorder_entries_test.py:172 reads that baseline, so the stranded census fails on it. Same shape as #18343's three rows. It is the only stale reference to any deleted file.

Security: all five sites moved to fetchWithAuth resolve to the AutoBot backend (SSOT getBackendUrl/getApiBase, or the build-time VITE_API_BASE_URL), so no token goes to a third-party origin. The user-supplied probe and the SLM client correctly stay on the raw-fetch exception list.

Checked and fine:

  • the ChatManager.js deletion (no importers, dynamic imports or lazy loads anywhere);
  • the circuit breaker: opt-in, SSOT thresholds, a distinguishable CircuitBreakerError, half-open implemented;
  • the allowlist guard fails on new unlisted entries and on stale ones;
  • no duplicate ApiClient.

LOWs, non-blocking:

  • half-open admits every concurrent caller, not one probe;
  • the shared 'monitoring' breaker key means one endpoint's 503 opens all four;
  • ApiEndpointMapper.js:20 reads VITE_API_BASE_URL directly, bypassing the SSOT (pre-existing).

@github-actions

Copy link
Copy Markdown
Contributor

Notice: 29 open PRs — past the runaway threshold (25)

This is not a request to defer this PR. The actual limit is the open-PR cap (AUTOBOT_OPEN_PR_CAP, default 40), enforced at pre-push against NEW branches only -- pushing more commits to this PR's own branch is never blocked by it.

This notice only means the count is high enough to be worth a glance for a runaway — something opening PRs in a loop, or a merge pipeline that has stalled so nothing is draining.

Currently open:

If the queue is draining normally, ignore this. Otherwise:

  1. Check whether CI is dispatching at all — see the ci-dispatch-watchdog status on these PRs
  2. Merge the ones whose CI has finished and review has passed: gh pr merge <number> --squash --delete-branch
  3. Look for a loop opening near-identical PRs

Warn-only runaway detector — .github/workflows/pr-queue-gate.yml. It never blocks a merge.

@github-actions

Copy link
Copy Markdown
Contributor

✅ SSOT Configuration Compliance: Passing

🎉 No new hardcoded values of either class — ssot and other both block.

Known backlog in pipeline-scripts/hardcoded_values_baseline.txt is suppressed and tracked in #14371.

This was referenced Oct 12, 2026
@mrveiss
mrveiss merged commit 5522914 into main Oct 12, 2026
60 of 71 checks passed
@mrveiss
mrveiss deleted the issue-12363-raw-fetch-tail branch October 12, 2026 19:27
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