Skip to content

Merge train round 3 B7: GUI bug fixes (#6025 #6010 #6007) - #6070

Merged
lidge-jun merged 5 commits into
devfrom
codex/train3-b7
Sep 27, 2026
Merged

lidge-jun merged 5 commits into
devfrom
codex/train3-b7

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Summary

Merge train round 3, batch 7: three GUI bug fixes from Ingwannu, each carried as one squashed commit that keeps the author. Batches 1 to 6 landed as #6059, #6061, #6062, #6063, #6066 and #6069.

PR Change Resolves
#6025 Kiro device-login status polling reads the whole response (fetch, a body capped at 64 KiB, and JSON decode) as one cancellable operation under a single 45 s deadline. A server that sends headers but never finishes the body can no longer hang the dialog or the background finalizer, and closing the dialog hands the same read to the finalizer instead of cloning the response. #6021
#6010 The provider deep-link test dispatches its own hashchange only when the hash does not change, because Happy DOM already fires one for a real change. This removes a flake that showed a third navigation in CI. #6009
#6007 With provider-table routing (Codex without sign-in, or client-side compaction), some mobile remote thread lists hide existing openai-tagged history. The dashboard now says so under the switch that turns that routing on, and ocx start/sync print the same warning. History is not rewritten. mitigates #5848, which stays open

The #6007 hint under "Open Codex without signing in" (captured from this branch's dashboard with a temporary home):

Dashboard switch Open Codex without signing in with the new remote history hint beneath it

#6025 and #6010 do not change anything a user sees.

Plan, reviews and evidence: devlog/_plan/260927_merge_train_3/070_batch7.md.

Co-authored-by: Ingwannu ingwannu@users.noreply.github.com

Verification

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Summary by CodeRabbit

  • New Features
    • Added a remote-history compatibility warning to relevant dashboard settings and successful setup messages. It explains that some remote clients may hide existing conversations and how compatible clients can list all providers.
  • Bug Fixes
    • Improved Kiro device-login status checks so stalled or incomplete responses are bounded and cancelled, while completed success replies can still be recognized after the dialog closes.
  • Documentation
    • Added guidance on remote conversation visibility and Kiro device-login behavior.

lidge-jun and others added 5 commits September 27, 2026 17:32
Carried from #6025 into merge train round 3.

Co-authored-by: Ingwannu <ingwannu@users.noreply.github.com>
Carried from #6010 into merge train round 3.

Co-authored-by: Ingwannu <ingwannu@users.noreply.github.com>
Carried from #6007 into merge train round 3.

Co-authored-by: Ingwannu <ingwannu@users.noreply.github.com>
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 27, 2026 08:36
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-27T08:39:45.795675Z 9e92631 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 25ff3175-3c3f-4913-83f1-0f0b6151abbb

📥 Commits

Reviewing files that changed from the base of the PR and between 429f4e0 and 9e92631.

📒 Files selected for processing (28)
  • devlog/_plan/260926_kiro_lb_parity2/100_gui_device_login_and_skip_reason.md
  • devlog/_plan/260927_merge_train_3/070_batch7.md
  • docs-site/src/content/docs/guides/codex-integration.md
  • docs-site/src/content/docs/guides/providers.md
  • gui/src/components/use-kiro-device-login.ts
  • gui/src/i18n/de.ts
  • gui/src/i18n/en.ts
  • gui/src/i18n/fr.ts
  • gui/src/i18n/ja.ts
  • gui/src/i18n/ko.ts
  • gui/src/i18n/ru.ts
  • gui/src/i18n/tr.ts
  • gui/src/i18n/vi.ts
  • gui/src/i18n/zh-TW.ts
  • gui/src/i18n/zh.ts
  • gui/src/kiro-device-login-finalizer.ts
  • gui/src/pages/dashboard-overview-sections.tsx
  • gui/tests/kiro-device-login.test.tsx
  • gui/tests/providers-deep-link.test.tsx
  • gui/tests/vision-sidecar-dashboard.test.tsx
  • src/codex/inject.ts
  • src/codex/inject/routing-target.ts
  • structure/codex-home.md
  • structure/decisions/ADR-5848-provider-table-remote-history-visibility.md
  • structure/decisions/ADR-6021-kiro-status-read-ownership.md
  • structure/gui-and-management-api.md
  • tests/codex-integration/codex-inject-integration.test.ts
  • tests/codex-integration/codex-inject.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The PR bounds and transfers Kiro status reads between the GUI hook and detached finalizer. It also adds provider-table remote-history warnings to CLI output and the dashboard, updates guidance and translations, adjusts a deep-link test helper, and records GUI batch status.

Changes

Kiro status reconciliation

Layer / File(s) Summary
Bounded status reader
gui/src/kiro-device-login-finalizer.ts
A cancellable status read owns fetching, consuming the response body through EOF, parsing JSON, and validating the result. Reads have a 45-second limit and a 64 KiB body cap.
Hook handoff and deadline reconciliation
gui/src/components/use-kiro-device-login.ts, gui/src/kiro-device-login-finalizer.ts, gui/tests/kiro-device-login.test.tsx, docs-site/src/content/docs/guides/providers.md, structure/gui-and-management-api.md, structure/decisions/ADR-6021-kiro-status-read-ownership.md, devlog/_plan/260926_kiro_lb_parity2/100_gui_device_login_and_skip_reason.md
Closing the dialog transfers its in-flight status read to the finalizer. The finalizer bounds retries by flow expiry and cancels pending reads; tests cover stalled bodies, late responses, and expired flows.

Provider-table history visibility

Layer / File(s) Summary
Warning generation and routing output
src/codex/inject/routing-target.ts, src/codex/inject.ts, tests/codex-integration/codex-inject.test.ts
Provider-table targets produce a warning about existing openai-tagged threads that some remote lists may omit. Successful injection messages include the warning, and tests cover routing-target conditions.
Dashboard hint and translations
gui/src/pages/dashboard-overview-sections.tsx, gui/src/i18n/*, gui/tests/vision-sidecar-dashboard.test.tsx
The dashboard displays the remote-history hint for the applicable settings. Ten language catalogs provide the hint, and tests check its visibility.
History-filter guidance and integration coverage
docs-site/src/content/docs/guides/codex-integration.md, structure/codex-home.md, structure/decisions/ADR-5848-provider-table-remote-history-visibility.md, tests/codex-integration/codex-inject-integration.test.ts
Documentation describes the provider-filter behavior and the modelProviders: [] option. Integration tests check warning output and filtered versus unfiltered history queries.

Hashchange test helper

Layer / File(s) Summary
Conditional hashchange dispatch
gui/tests/providers-deep-link.test.tsx
The helper dispatches hashchange only when assigning the requested hash does not change the location.

GUI batch plan record

Layer / File(s) Summary
Batch status and validation record
devlog/_plan/260927_merge_train_3/070_batch7.md
The plan lists three GUI fixes and records their status, validation results, and screenshot setup details.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant GUIHook
  participant KiroFinalizer
  participant StatusReader
  participant ManagementAPI
  GUIHook->>StatusReader: Start status read
  GUIHook->>KiroFinalizer: Transfer in-flight read on close
  KiroFinalizer->>StatusReader: Await result with flow deadline
  StatusReader->>ManagementAPI: Fetch status and consume response body
  StatusReader-->>KiroFinalizer: Return view, missing, or retry
Loading

Possibly related PRs

Suggested labels: bug, gui-screenshot-waived

Merge Risk: ⚪ Minimal · up to 9e926

The Kiro status-read, history-warning, and test-helper changes have no established merge-blocking issue. Normal validation remains appropriate.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 9e926

The status read is now bounded, and closing the dialog hands it to a finalizer without giving the dialog authority to save credentials. No new security issue was established. The assessment is limited by the unavailable prior source and deployment details.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — A malformed or mismatched status response can affect reconciliation of the active GUI flow, but the inspected path does not give that response cross-flow credential-write authority.

Trust Boundaries and Controls

  • observed — The reader treats status JSON as untrusted input, validates its view shape, and rejects a view for another flow. The server separately enforces principal ownership of status lookup.

Resilience and Maintainability Implications

  • observed — A cancellation reply alone cannot report successful credential persistence to the finalizer: it continues reconciliation until status confirms a terminal outcome. Repeated finalization for the same key returns an active or recorded outcome.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The linked issue is #6025, which covers Kiro status reconciliation. The PR also changes the provider deep-link hash test in gui/tests/providers-deep-link.test.tsx for #6010 and adds provider-table r… Remove the #6010 and #6007 changes from this PR, or split them into separate pull requests with their corresponding directly linked issues. Keep this PR limited to the #6025 Kiro status-reconciliation implementation, tests, and related docu…
Docstring Coverage ⚠️ Warning Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 20 files. (8 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately identifies this as merge train round 3, batch B7, and summarizes the primary objective as GUI bug fixes. The referenced issues match the pull request objectives.
Linked Issues check ✅ Passed #6025 requires one shared, cancellable Kiro status-read operation. gui/src/kiro-device-login-finalizer.ts:3-8,22-68 owns fetch, stream EOF, the 64 KiB cap, JSON parsing, validation, timeout, and can…
Full details: Out of Scope Changes check

Explanation

The linked issue is #6025, which covers Kiro status reconciliation. The PR also changes the provider deep-link hash test in gui/tests/providers-deep-link.test.tsx for #6010 and adds provider-table remote-history warnings across src/codex/inject.ts, src/codex/inject/routing-target.ts, dashboard sections, translations, integration tests, and documentation for #6007. These changes do not implement or test Kiro status-read ownership, handoff, deadlines, or cancellation. The PR description mentions these issues, but the supplied linked-issue requirements contain only #6025.

Resolution

Remove the #6010 and #6007 changes from this PR, or split them into separate pull requests with their corresponding directly linked issues. Keep this PR limited to the #6025 Kiro status-reconciliation implementation, tests, and related documentation.

Full details: Docstring Coverage

Explanation

Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 20 files. (8 skipped: 8 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 48 / 80

이 PR은 머지 열차 3라운드의 일곱 번째 묶음이에요. 바탕은 dev입니다. Ingwannu의 GUI 버그 세 개를 커밋 하나씩 가져와요.

Kiro 기기 로그인은 상태 응답의 본문까지 한 번의 읽기로 묶어요. 예전에는 응답 머리만 와도 45초 제한이 풀려서, 본문이 안 끝나면 창이랑 뒤에서 이어 받는 쪽이 그 자리에서 기다렸어요. 지금은 읽기 하나가 요청, 본문, JSON 해석을 같이 해요. 한 번은 45초를 넘기지 않고, 본문은 64KiB에서 끊어요. 창을 닫으면 진행 중인 읽기 하나를 이어 받는 쪽에 넘겨요. 흐름 시간이 끝나면 그 읽기를 취소하고, 다시 시도하기 전 잠도 남은 시간보다 길지 않아요. 마감이 이미 지난 뒤에 넘어온 읽기는 새 폴링 없이 취소해요. 로그인 시작이랑 취소 응답의 본문은 그대로예요.

공급자 주소 테스트는 화면을 안 바꿔요. 주소의 #가 실제로 바뀌면 Happy DOM이 hashchange를 이미 보내요. 테스트가 한 번 더 보내면 같은 이동이 두 번 세졌어요. 주소가 그대로일 때만 테스트가 신호를 직접 보내요.

휴대폰 목록 안내는 지워진 대화랑 숨은 대화를 구분해요. 제공자 표를 쓰면 새 대화 이름은 opencodex가 되고, 페이지가 나뉜 예전 대화는 openai로 남을 수 있어요. 일부 휴대폰은 목록 필터를 비우면 지금 기본 제공자만 보여 줘요. DB 행이랑 rollout 파일은 그대로예요. ocx start와 ocx sync는 제공자 표가 들어간 성공 출력에 경고를 붙여요. 대시보드는 로그인 생략 스위치나 클라이언트 요약 스위치가 켜져 있을 때 같은 뜻을 한 번만 보여 줘요. 이름표는 다시 달지 않아요. #5848은 계속 열려 있어요.

라인 - gui/src/components/use-kiro-device-login.ts 133행. 창이 열려 있는 동안의 상태 읽기는 항상 45초예요. 127행에서 만료를 본 다음에 읽기를 시작해서, 읽는 중에 만료가 지나도 그 읽기는 안 끊겨요. 본문이 안 끝나면 만료 화면은 그 읽기가 끝난 뒤 루프 맨 위의 2초 대기를 한 번 더 지나야 나와요. gui/src/kiro-device-login-finalizer.ts 127행은 남은 시간과 45초 중 짧은 쪽을 써요.

라인 - src/codex/inject/routing-target.ts 54–56행. 경고는 제공자 표를 쓰는지만 보고, 이번 실행에 openai 행이 남았는지는 안 봐요. 로그인 생략으로 이름표를 opencodex로 다시 달면 src/codex/inject.ts 737행은 그 스레드가 보이게 됐다고 하고, 797행은 바로 다음 줄에서 openai 대화가 숨는다고 해요. 클라이언트 요약만 켠 경로(726행)는 이름표를 그대로 두므로 숨김 문장이 그 경로와 맞아요.

메인테이너의 판단이 필요한 지점

창이 열린 동안의 45초를 만료 시각에 맞출지예요. 맞추면 만료 화면은 빨라져요. 만료 직전에 이미 온 done을 창이 직접 받는 시간은 짧아져요.

대시보드 문장(520행, 541행)은 스위치가 저장돼 있으면 나와요. 그 스위치가 이번 경로에서 실제로 제공자 표가 됐는지는 안 봐요. 문서도 그렇게 적어 뒀어요. 표가 빠진 경로에서도 보여줄지, CLI처럼 표가 들어간 뒤에만 보여줄지 정하면 돼요.

이름표를 다시 단 성공 출력에도 숨김 경고를 남길지예요. ADR-5848은 넓은 경고로 적어요. 앱 서버가 이미 제공자를 전부 주는 버전에서도 ocx start마다 그 문장이 나와요.

너의 추천

머지하세요. 바탕 dev가 맞아요. src/types.ts랑 src/config.ts는 안 건드려서, 타입 파일을 나누는 PR과 겹쳐 닫을 건 없어요. 세 고침은 이 묶음에 두세요.

133행의 45초는 무한 대기를 이미 끊어요. 만료에 맞추는 일은 다음으로 두세요. 숨김 경고는 이름표를 안 바꾸는 클라이언트 요약에는 그대로 두세요. 이름표를 다시 단 문장 옆의 경고는 빼도 되고, 넓은 경고로 둬도 돼요. #5848은 열어 두세요.

이 묶음이 들어가면 #6025, #6010, #6007은 같은 내용이라 닫으세요. #6021이랑 #6009도 이 글이 고친 범위로 닫으면 돼요.

이 댓글은 grok-bot이 작성했습니다

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.

2 participants