Skip to content

fix(rooms): say the invite link was copied only when the clipboard took it, else show it to copy by hand - #330

Merged
Birfy merged 4 commits into
mainfrom
p321-followup
Oct 18, 2026
Merged

Birfy merged 4 commits into
mainfrom
p321-followup

Conversation

@Birfy

@Birfy Birfy commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to #321 (Codex P2 on new-chat.js: a missing or failed clipboard counted as a copy). Ran ARCH_DOCS_BASE=origin/main TZ=UTC npm run verify at the tip.

…ok it, else show it to copy by hand

navigator.clipboard?.writeText completed quietly when the clipboard was missing, so the card claimed a copy that never happened and showed no link. copiedToClipboard reports a real copy; otherwise the link is shown in a read-only field.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 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-10-18T10:19:44.932417Z a9ddb43 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.

@Birfy
Birfy added this pull request to the merge queue Oct 18, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 18, 2026

@Birfy Birfy left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fresh review at tip a9ddb43 (posted from the author's account, so it is not an independent PASS and records none).

Verdict: needs change (rebase only; no code defect found). The branch conflicts with main (mergeStateStatus DIRTY), so it cannot enter the queue as is.

Findings

  1. Merge conflict in generated files only. git merge-tree against current main conflicts in patches/ds-bot/0001-core-fixes.patch and patches/ds-bot/0008-client-bundle.patch (the #321 squash landed after this branch's base). Follow the AGENTS.md conflict recipe: merge main, git checkout origin/main -- patches/ds-bot, npm run build in core/, node scripts/ds-bot-patches.mjs generate then check, npm run verify. The new tip will need a fresh review.
  2. Minor (non-blocking): the second new test is a source-text assertion on new-chat.js (regexes on copyInvite). It guards the wiring but would not catch a behavior regression; the first test (copiedToClipboard with no clipboard / non-function / sync throw / rejected write) is behavioral and is the real guard. The repo already uses this style elsewhere, so I would not block on it.

Checked

  • Logic is correct: "copied" is said only when writeText really resolved; a missing clipboard or refusal shows the link in a read-only, select-all field with a clear note. Create-invite failure keeps its separate message. Retrying or "Leave out" clears the link. A late createInvite after unmount only sets state on a closed card.
  • The invite link is shown only to the owner who just made it, in their own UI (it was already copied to the clipboard before); nothing is logged or stored. No grant is written by the client, per-Bot default deny unchanged.
  • UI is simple and uses existing classes (bt-input); both new strings have zh entries (邀请链接, 无法自动复制…) and a test asserts the dictionary keys.
  • core/ and patches/ are consistent at this tip (ds-bot-patches.mjs check ok); build-client.mjs --check ok.
  • New copiedToClipboard test cannot pass on base (the export does not exist there).

Ran at this tip: core full suite (853 pass, 0 fail, Node 24, TZ=UTC); ds-bot-patches.mjs check; build-client.mjs --check; git merge-tree against main.
Not run: clicking Copy in a real browser/WebView (no clipboard case), smoke.
Codex: completed at this tip with no findings.
CI: all checks green at this tip; mergeability DIRTY. Risk tier T3 (core/src/shared/remote-label.js).
Still needed: rebase/regenerate, then an independent PASS on the new tip from a login in review-policy.json other than the author, and Danyang's one-click (T3).

Conflicts were only in generated patches/ds-bot/0001 and 0008: took main's
patches and regenerated from core/ (AGENTS.md conflict recipe).
@Birfy

Birfy commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator Author

Maintenance check at tip 815db7c (posted from the author's account, so this is not an independent PASS).

The rebase finding from the last review is resolved: Merge origin/main at 815db7c merges cleanly into current main (2c9853d), ds-bot-patches.mjs check ok, build-client.mjs --check ok, core/test/new-chat.test.js 8/8, and all CI checks are green. The code is the same as the reviewed a9ddb43 (clipboard fallback only), and it matches the design (English UI + zh, the owner's own invite link only, no grant written by the client).

Still blocked only on the gate: risk tier T3 (core/src/shared/remote-label.js), so it waits for Danyang's one-click approval at this head before it can go into the merge queue.

@Birfy

Birfy commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator Author

Main moved to d6203fd (#341) and conflicted again in the generated 0008-client-bundle.patch only. Merged main at 9c6d6bf with the AGENTS.md recipe (main's patches/ds-bot, npm run build, ds-bot-patches generate + check); TZ=UTC ARCH_DOCS_BASE=upstream/main npm run verify green at that tip. No source change. Still T3: waiting for Danyang's one-click approval at 9c6d6bf.

@Birfy
Birfy added this pull request to the merge queue Oct 18, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 18, 2026
# Conflicts:
#	core/client.js
#	patches/ds-bot/0008-client-bundle.patch
@Birfy
Birfy added this pull request to the merge queue Oct 18, 2026
Merged via the queue into main with commit bcb9277 Oct 18, 2026
8 checks passed
@Birfy
Birfy deleted the p321-followup branch October 18, 2026 14:09
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