Skip to content

feat(desktop): expose bot onboarding retry health - #4746

Open
testikun wants to merge 12 commits into
apache:mainfrom
testikun:codex/issue-4745-bot-onboarding-health
Open

feat(desktop): expose bot onboarding retry health#4746
testikun wants to merge 12 commits into
apache:mainfrom
testikun:codex/issue-4745-bot-onboarding-health

Conversation

@testikun

@testikun testikun commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Expose finite, redacted timeout/network/rate-limit/server retry health from the existing main-process Bot onboarding poll owner.
  • Clear the projection on recovery, cancellation, expiry, denial, terminal failure, and supersession without changing retry thresholds, intervals, provider protocols, or persistence.
  • Show concise localized backoff status in the existing QR onboarding modal while preserving the QR and secret-free IPC boundary.

Fixes #4745

Verification

  • Core and Desktop focused onboarding tests: 22 passed.
  • Redaction regressions: 27 passed.
  • Electron retry-health E2E: 1 passed.
  • Desktop build and typecheck.
  • Core typecheck.
  • Renderer architecture check.
  • npm run lint.
  • npm run format:check.
  • npx knip --workspace apps/desktop.
  • npm run check:asf-headers.
  • git diff --check.

Visual evidence

Built and launched current head 3a37a66f3 in a visible local Electron App. The real QR onboarding modal kept the QR available while showing only the bounded retry category, consecutive count, and retry delay. This is a direct App capture, not a mockup or generated image.

Real Maka bot onboarding retry health with the QR preserved

The Electron test also asserts that fixture provider details do not appear in the renderer.

Review focus

The existing poll loop remains the only retry authority. retryHealth contains only a finite category, consecutive count, and retry timing; raw provider errors and credentials remain main-process-only.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex implemented the bounded Core/Desktop contract, main-process projection, localized UI state, deterministic lifecycle/redaction coverage, and E2E visual fixture. The human contributor remains responsible for review and submission.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/M Under 500 readable lines label Sep 4, 2026
@testikun
testikun force-pushed the codex/issue-4745-bot-onboarding-health branch 2 times, most recently from 3a37a66 to 20a2442 Compare September 5, 2026 02:06

@hqhq1025 hqhq1025 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed exact head 0c2008256f8dcf94a26435e951610f0c0898d481.

This change adds a finite, redacted retry-health projection and localized onboarding status, with focused lifecycle and Electron coverage. I found one P2 state-machine issue in the post-confirmation error path, so I do not recommend merging this head yet.

Validation included a clean install, build:test, Desktop typecheck/build, Core (825/825) and Desktop (2165/2165) dist suites, focused onboarding/redaction tests, renderer architecture (98/98), both affected Electron journeys, lint, format, Biome, ASF headers, diff check, hosted test, and a clean patch-preserving merge onto current main (7743890729b26a7e95f653af8b93ccbe42ad480f). I did not exercise live provider APIs or native macOS/Windows UI behavior.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

Comment thread apps/desktop/src/main/bot-onboarding-main.ts Outdated
@testikun
testikun force-pushed the codex/issue-4745-bot-onboarding-health branch from 0c20082 to 8cbdb12 Compare September 6, 2026 06:32
@testikun

testikun commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

@hqhq1025 Addressed the P2 from your review comment in commit 8cbdb12e.

The retry classifier is now enabled only after the provider poll promise rejects. Once the provider returns confirmed, failures from credential persistence or runtime effects are terminal error snapshots with no retryHealth, so the renderer cannot promise another provider poll for a non-pollable connecting session. Added a regression test covering a timeout-shaped post-confirmation runtime-effects failure.

Targeted diff check passed; the branch was rebased onto current apache/main and pushed to codex/issue-4745-bot-onboarding-health.

@testikun
testikun force-pushed the codex/issue-4745-bot-onboarding-health branch from 5990e5b to 8f43713 Compare September 6, 2026 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/M Under 500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(bots): expose bounded redacted onboarding retry health

2 participants