feat: add live read-only session sharing clients [risk:high] - #539
Conversation
Model: gpt-6-astra
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Model: gpt-6-astra
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f938fa58a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| args: Parameters< | ||
| NonNullable<Parameters<typeof SessionChatStreamView>[0]['renderMessageRow']> | ||
| >[0] | ||
| ) => <MessageRowView {...args} user={null} />, |
There was a problem hiding this comment.
When the shared transcript contains an unresolved task_proposal and the visitor's isolated store resolves tasksFeatureEnabledAtom to true—for example, when /s is hosted on the app origin and that browser previously enabled the Tasks beta—delegating system rows to the normal MessageRowView mounts TaskProposalNotice, which displays Create task and Ignore controls and imports router/workspace mutation hooks. This makes the anonymous reader expose nonfunctional workspace-write affordances and can require app-router context; route these notices through a read-only presentation or suppress their actions.
AGENTS.md reference: packages/components/src/components/sharing/AGENTS.md:L64-L66
Useful? React with 👍 / 👎.
The dialog stacked its disclosure, a near-identical acknowledgement, a per-row unavailability sentence and several hints into one scrolling column, so the meaning a user needs before sharing was repeated while the buttons acting on it could scroll off a phone screen. The frame is now a fixed header over one scrolling body, and the keyboard hook observes that body rather than the whole panel. The manager pins its action row and the acknowledgement it gates to the bottom of that body. Each meaning is stated once beside what it describes: the disclosure sits on the link card, the selection rule on the selection, and one shared line explains the compact "Not ready" badge instead of repeating a sentence under every ineligible row. The candidate filter is passed in as a node so it renders inside the section it narrows, and only the target list stays in the disabled fieldset so filtering survives an in-flight mutation. The reader header gives the conversation title a full-width line by moving the visitor label up to the workspace line, and drops the standing "Read only · Updates live" text: the surface offers no write affordance, so its status region now announces interruptions only. An appearance control beside the visitor label reuses the app's own theme cycle, `system` default and storage key rather than introducing a reader-specific mode. Model: claude-opus-5[1m]
2f938fa to
97244a9
Compare
Choosing what a link covers meant auditing a checklist of candidate conversations and then ticking a consent box that restated the disclosure sitting directly above it. That is two decisions and an audit for something almost always answered "just this conversation" or "this one and its sub-conversations". Replace the checklist with a single "Include sub-conversations" switch and drop the acknowledgement entirely; the disclosure on the link card is the notice. A conversation with no descendants renders no switch at all, leaving the link, one sentence and the actions. The candidate search field goes with the list it used to filter. The switch is only a projection over the same explicit id set the server still verifies target by target, so the wire contract is unchanged. It can therefore only mean "the sub-conversations that exist and are ready now": turning it on selects the root plus every currently eligible descendant, capped at the link limit, and the helper line says that later ones are not added automatically. The trade is that a partial subset can no longer be expressed. Two states that previously only greyed a button now explain themselves: a root that has not finished syncing says so, and Save appears only once something changed. Model: claude-opus-5[1m]
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8312e7742f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| <Switch | ||
| className="shrink-0" | ||
| checked={sharesChildren} | ||
| disabled={busy || props.conflict || shareableChildren.length === 0} |
There was a problem hiding this comment.
When an active grant still contains a child but every discovered child is currently unavailable, sharesChildren keeps this switch checked while shareableChildren.length === 0 disables it. The selected unavailable child also makes qualified false, disabling both Save and Reset, so the owner cannot repair the grant without revoking the entire link. Keep the checked switch enabled for the off transition even when there are no currently shareable children.
AGENTS.md reference: packages/components/src/components/sharing/AGENTS.md:L17-L23
Useful? React with 👍 / 👎.
…edium] The "Other links including this conversation" section listed grants rooted at a different conversation that happen to cover this one. Its meaning did not survive first contact: readers took it to be about this conversation's own links, and it cost a heading, an explanatory line and a row per grant in a dialog whose point is a single decision. Remove the section. `state.sources` is still fetched and still feeds `copyableShareIds`; only the rendering is gone, so nothing about authorization or revocation changes. This does re-open a documented gap: Q12 asks the management surface to show every still-valid sharing source so that revoking one link is not mistaken for "no longer shared anywhere". An author can now revoke the link shown here and remain readable through a grant rooted elsewhere, with no hint in the UI. The requirement is not reinterpreted away — it is recorded as an open gap in the private docs, and the nearest AGENTS.md forbids presenting this revocation as a full stop. Model: claude-opus-5[1m]
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 516f03df5e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (!abort.signal.aborted) | ||
| timer = setTimeout(() => { | ||
| void refresh(); | ||
| }, 30_000); |
There was a problem hiding this comment.
When a manifest has less than 30 seconds remaining, this fixed delay leaves the existing stream and attachment capability active past validUntil, even though that field is defined as the deadline by which authorization must be revalidated. Moreover, if the next request returns the now-expired manifest, the generic catch only marks the page paused and retains the old reader. Schedule refresh or invalidation for the earlier of this interval and validUntil so an open reader cannot outlive its authorization lease.
Useful? React with 👍 / 👎.
…sk:medium] Every explanatory line was carrying more words than the decision it supports. Roughly halve them, keeping the meanings an author cannot infer from the controls: - Disclosure: drops the enumeration of documents/history/attachments and the E2EE clause, keeping that anyone with the link reads the full conversation and its later updates, and that links can be forwarded. - Sub-conversation helper: two sentences become one line, "N ready now · new ones are not added automatically" — still both facts a bare switch cannot carry. - Not-ready, missing-secret, cap, conflict and both confirmations lose their restatements and keep the consequence. No behaviour change; the gates, counts and conditions are untouched. Model: claude-opus-5[1m]
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 755c280c85
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 'sharing.manager.disclosure', | ||
| 'Anyone with the link reads the full conversation and its later updates. Links can be forwarded.' |
There was a problem hiding this comment.
When an owner decides whether to create or reset a link, this paragraph is the only consent notice, but it merely says that the “full conversation” and updates are readable and that the link can be forwarded. It omits the required warning that recipients receive the complete original document and history—including fields the current UI does not display—and that this is bearer access rather than end-to-end encryption (specs/session-sharing.md:6-10). Because there is no separate acknowledgement, owners can publish more data than the UI informed them about; expand this copy in both locales.
AGENTS.md reference: packages/components/src/components/sharing/AGENTS.md:L17-L23
Useful? React with 👍 / 👎.
Model: gpt-6-astra
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb62e83207
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 'sharing.manager.confirmRevoke', | ||
| 'The link stops working. Already downloaded content is unaffected.' |
There was a problem hiding this comment.
When state.sources contains another active grant rooted at a different conversation, revoking the displayed root link leaves this conversation accessible through that other grant, but the confirmation only says that the link stops working and the manager never surfaces the remaining access. This can lead an owner to believe outside access has ended; disclose the surviving grant/access in the confirmation or post-revoke state.
AGENTS.md reference: packages/components/src/components/sharing/AGENTS.md:L25-L28
Useful? React with 👍 / 👎.
Preserve sharing translations alongside context-copy labels from main. Validation: composition typechecks, lint, CLI/components/shared/backend tests, public script tests, docs, share build and Worker integrations passed. Full check is blocked by main sparkle-packaging test referencing the deleted release-electron workflow. Model: gpt-6-astra
The changelog-only release migration deleted release-electron.yml. Remove assertions tied to that retired workflow while retaining all local Sparkle packaging behavior tests. Validation: Electron 104 tests, full pnpm test, and private composition pnpm check:affected pass. Model: gpt-6-astra
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afac52d3ec
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const rootVersion = root | ||
| ? `${root.shareId}:${root.scopeVersion}:${root.credentialVersion}` | ||
| : 'new'; | ||
| const canManage = !root || root.canManage; |
There was a problem hiding this comment.
When a workspace member or administrator opens another owner's conversation before it has a root grant, root is null, so this unconditionally treats the viewer as able to manage it; if the synchronized root is otherwise eligible, the dialog enables Create and sends a publication request. The sharing contract restricts publishing and target selection to the conversation owner and gives administrators revocation authority only (specs/session-sharing.md:6-24), so this either exposes a mutation the service must reject or bypasses that ownership rule. Carry an explicit create permission in the management result or gate creation against the session owner instead of deriving it from grant absence.
Useful? React with 👍 / 👎.
1 participant
Risk: 🔴 high | Confidence: medium — persisted sharing and public title previews; local security and full automated gate pass, with browser scroll and hosted acceptance limits documented below.
Related issue
Same-repository feature contribution.
Problem / pressure
Conversation owners need a live read-only link for explicitly selected conversations without granting workspace membership. Shared clients need to manage links while the standalone local app retains its cloud-free boundary.
Summary
Visual explanation
Before / after
Update: dialog and reader layout rebuild
The management dialog and the reader shipped their content as flat scrolling
columns, so the disclosure, a near-identical acknowledgement, a per-row
unavailability sentence and several hints repeated the same meaning while the
buttons acting on them could scroll off a phone screen.
SessionShareDialogFrameis now a fixed header over one scrolling body, anduseKeyboardAwareScrollIntoViewobserves that body — the element that actuallyscrolls — instead of the whole panel.
SessionShareManagerpins its action row to the bottom of that body, so theprimary action can no longer be scrolled out of reach.
link card and still names all four things a user cannot infer: original documents,
history and attachments, later updates, and that links are forwardable and not
end-to-end encrypted.
visitor label onto the workspace line, and drops the standing
"Read only · Updates live" text — the surface offers no write affordance, so its
status region now announces interruptions only.
light/dark/system cycle,
systemdefault and storage key rather than areader-specific mode.
Gating, version fences, eligibility rules and credential handling are unchanged.
Follow-up: one switch instead of a checklist
Review feedback: choosing what a link covers should be one question, and there
should be no final consent step.
Include sub-conversations switch. A conversation with no descendants renders
no switch at all, leaving the link, one disclosure sentence and the actions.
save or reset. The disclosure on the link card is the notice.
target by target — the wire contract and authorization model are unchanged.
It therefore means "the sub-conversations that exist and are ready now": turning
it on selects the root plus every currently eligible descendant, capped at
SESSION_SHARE_MAX_TARGETS, and the helper line says later ones are not addedautomatically. Ineligible descendants are never pulled in.
with no reason, and Save renders only once something actually changed.
Trade-off, stated plainly: a partial subset can no longer be expressed. An author
who wants some but not all descendants has no way to say so, and toggling off then
on widens a legacy partial grant to everything currently ready.
docs/session-sharing.mdand
docs/session-sharing-phase-1.mdrecorded per-item selection as the Q10 outcome;they were updated in the private PR so they no longer contradict the code.
session-share-manager.test.tsxnow covers the switch's real obligations: a new linkis created without extra confirmation and does not widen itself, the switch adds every
ready descendant while excluding the unavailable one, turning it off returns to the
root alone, the switch is absent with nothing to include and frozen during a mutation,
and an unshareable root is explained rather than only disabled.
Layout verification
Reviewed from rendered screenshots, not by reading CSS: the manager stories at
1200px and 390px in both themes and in Chinese, and the built reader at 1280px and
390px in both themes. After the switch replaced the checklist the whole dialog fits
a 390px viewport without scrolling. That review is what caught the link status being truncated to
"Link …" by its own buttons on a narrow phone, and the reader title being crushed by
the visitor label; both were fixed and re-shot.
session-share-manager.test.tsxkeeps its four behaviour tests and adds two: theunavailability explanation appears once and only while a candidate is ineligible,
and a mutation in flight freezes the targets while leaving the filter usable.
Theme behaviour is covered in the private Web host against the real built artifact.
Test plan
pnpm check:affected(includes public types, lint, tests and boundary checks).pnpm run docs checkpasses (no errors); specs remain draft and translations pending.@lody/componentstypecheck,pnpm lint:i18n, the sharing componenttests, and the private host's twelve built-page reader tests all pass. Layout itself
was verified from screenshots at desktop and 390px widths in both themes.
pnpm buildwas attempted but requires missing deployment configuration (VITE_PREVIEW_PUBLIC_BASE_DOMAIN). Hosted service acceptance and native-device keyboard behavior remain unverified; no deployment performed.Context handoff
Instructions for reviewing agents
Authoring context
Follow-up: the other-links section is hidden
"Other links including this conversation" listed grants rooted at a different
conversation that also cover this one. Readers took it to be about this
conversation's own links, so it is removed for now.
state.sourcesis still fetched and still feedscopyableShareIds; only therendering is gone. Authorization, revocation and the wire contract are unchanged.
Known gap this re-opens. Q12 asks the management surface to show every
still-valid sharing source, so that revoking one link is not read as "this
conversation is no longer shared anywhere". An author can now revoke the link shown
in the dialog and still be readable through a grant rooted at another conversation,
with nothing in the UI saying so. The requirement was not reinterpreted away: it is
recorded as an open gap in the private
docs/session-sharing.md, the nearestAGENTS.mdforbids presenting this revocation as a full stop, and a test asserts thesection stays absent so its return is a deliberate change rather than a regression.
Follow-up: copy reduced
Every explanatory line was longer than the decision it supports; they are roughly
halved. The disclosure drops the enumeration of documents/history/attachments and
the E2EE clause, keeping that anyone with the link reads the full conversation and
its later updates and that links can be forwarded. The sub-conversation helper
becomes one line, "N ready now · new ones are not added automatically". Not-ready,
missing-secret, cap, conflict and both confirmations keep the consequence and drop
the restatement. No behaviour change — gates, counts and conditions are untouched.
Update: title-only public previews (2026-09-11)
This update preserves the existing dialog/reader/theme work. Creating a link now explicitly permits a public preview of the root conversation title by default; the dialog adds a short English/Chinese notice without another confirmation. Bodies, attachments, child titles and workspace data are not public preview data. Access credentials remain exclusively in the fragment/bearer flow.
The private host uses pinned
takumi-js@2.13.7/wasmin Cloudflare Workers, a fixed private-R2 Noto font, a 1200×630 title-only template, versioned private-R2 caching and a Pages advanced Worker that emits initial HTML OG/Twitter metadata. Preview/image requests revalidate grant/source/identity lifecycle; invalid links do not return old titles. Cached third-party previews cannot be recalled. Title changes follow existing source verification with eventual consistency.Validation for this update: full root
pnpm check:affectedpassed (including all seven gh-shim-script tests on this machine), affected share build and build-config tests passed, 41 Convex sharing tests passed, existing Worker suite passed, and real workerd/WASM/R2/Pages integration passed. Actual Chinese, English and long/special-character PNGs were inspected; two synthetic examples were displayed in the implementation conversation. Security, correctness, scope, simplification and fresh adversarial re-review completed.Current limitations supersede earlier check-status notes: built-reader browser suite is 11/12; the existing long-conversation scroll-restoration assertion fails even when rerun alone. Reader/scroll code is unchanged and that fixture bypasses the new Pages Worker; a baseline build comparison was not performed. Full root build remains blocked by Mobile deployment configuration (
VITE_PREVIEW_PUBLIC_BASE_DOMAIN); isolated affected builds pass. Hosted CPU/memory, crawler and native acceptance remain unverified. Chinese/Latin are covered by the fixed font; unsupported emoji may render missing glyphs.Integration: public commit
fb62e83207a1e09e907c91fdaa3512aeca5e3e62is a new companion dependency. This update does NOT change the existing private gitlink or CLI revision marker (755c280c) and does not revert the historical pointer commit. Integrate the new public revision through the separately authorized post-merge pointer workflow.Deployment documentation lists font checksum/upload, existing bindings, Worker and Pages build/deploy/routes, local/staging/production origins and private cache lifecycle. No merge or deployment performed.