Skip to content

fix(embed): bound launch auth handshake (#9947)#10659

Closed
lalalune wants to merge 1 commit into
developfrom
fix/9947-embed-auth-timeout
Closed

fix(embed): bound launch auth handshake (#9947)#10659
lalalune wants to merge 1 commit into
developfrom
fix/9947-embed-auth-timeout

Conversation

@lalalune

@lalalune lalalune commented Jul 1, 2026

Copy link
Copy Markdown
Member

Summary

  • bounds the /embed pre-mount auth exchange with a 10s timeout
  • aborts the fetch when supported and fails closed with network_timeout
  • adds regression coverage for a never-resolving auth request so the app does not remain blank indefinitely

Evidence

  • bun install --frozen-lockfile --ignore-scripts
  • bun run build:core -> 64/64 tasks passed
  • bun run --cwd packages/app test src/embed-bootstrap.test.ts -> 14/14 passed
  • bunx @biomejs/biome check packages/app/src/embed-bootstrap.ts packages/app/src/embed-bootstrap.test.ts .github/issue-evidence/9947-embed-auth-timeout/README.md
  • git diff --check origin/develop...HEAD

Evidence note: .github/issue-evidence/9947-embed-auth-timeout/README.md

Refs #9947

@greptile-apps greptile-apps Bot 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.

Your trial has ended. Reactivate Greptile to resume code reviews.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e16eb946-d1f4-45b6-a286-9906b7721fc5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/9947-embed-auth-timeout

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.

@lalalune

lalalune commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

Reviewed #10659 locally against 44bee705a9c8. The timeout branch is narrowly scoped and I do not see a code-level blocker: the fetch is raced with a bounded timer, the controller is aborted on timeout when available, and the timeout is cleared in finally.

Validation I ran:

  • bun run --cwd packages/app test src/embed-bootstrap.test.ts: passed, 14/14 tests.
  • bunx @biomejs/biome check packages/app/src/embed-bootstrap.ts packages/app/src/embed-bootstrap.test.ts .github/issue-evidence/9947-embed-auth-timeout/README.md: passed.
  • bun run --cwd packages/app typecheck: passed.
  • git diff --check origin/develop...HEAD: passed.

No merge from me yet because GitHub required checks are still queued/pending. Screenshot/video/app audit remains N/A for this specific PR because it changes pre-mount embed auth control flow, not a rendered UI state.

@NubsCarson

Copy link
Copy Markdown
Member

Independent validation pass on current head 44bee705a9:

  • git diff --check origin/develop...HEAD
  • bunx biome check packages/app/src/embed-bootstrap.ts packages/app/src/embed-bootstrap.test.ts .github/issue-evidence/9947-embed-auth-timeout/README.md
  • bun run --cwd packages/app test src/embed-bootstrap.test.ts (14 passed)
  • bun run --cwd packages/app typecheck
  • bun run --cwd packages/app build:web + [verify-chunk-safety] OK

Scope read is narrow and non-overlapping with #10660: this PR only bounds the client /embed auth request path and adds bootstrap coverage/evidence. No local blocker found; GitHub checks are still queued/UNSTABLE.

@NubsCarson

Copy link
Copy Markdown
Member

Post-merge reconciliation against current origin/develop (a3d78672c6): the functional app-code patch here is already landed via #10663 (ed3fd0d1a5).

Verification:

  • git show ed3fd0d1a5 -- packages/app/src/embed-bootstrap.ts packages/app/src/embed-bootstrap.test.ts | git patch-id --stable -> 6719e0f270ef0be7024593e6284d9e20049048e6
  • git show 44bee705a9 -- packages/app/src/embed-bootstrap.ts packages/app/src/embed-bootstrap.test.ts | git patch-id --stable -> 6719e0f270ef0be7024593e6284d9e20049048e6
  • git diff ed3fd0d1a5 44bee705a9 -- packages/app/src/embed-bootstrap.ts packages/app/src/embed-bootstrap.test.ts -> empty

So I would not merge this stale branch as-is: it is based before several newer develop commits and would now mostly duplicate/supersede #10663. The only unique file left is .github/issue-evidence/9947-embed-auth-timeout/README.md; if we still want that evidence artifact, best path is a tiny evidence-only follow-up or cherry-pick of just that file. Otherwise this PR can be closed as superseded by #10663.

@NubsCarson

Copy link
Copy Markdown
Member

Closing this as superseded by #10663. The functional app-code patch from this branch is already on develop via ed3fd0d1a5 / #10663; earlier reconciliation verified the app file patch-id matches and git diff ed3fd0d1a5 44bee705a9 -- packages/app/src/embed-bootstrap.ts packages/app/src/embed-bootstrap.test.ts is empty.

The only remaining unique artifact here is the evidence README, and the validation evidence is preserved in this PR thread. Keeping this open would leave a stale pre-merge branch in the queue and risks duplicating already-landed code.

@NubsCarson NubsCarson closed this Jul 1, 2026
@lalalune lalalune deleted the fix/9947-embed-auth-timeout branch July 1, 2026 04:59
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