Share menu: collect the note exports, and send a note to the macOS share sheet - #448
Open
Optic00 wants to merge 11 commits into
Open
Share menu: collect the note exports, and send a note to the macOS share sheet#448Optic00 wants to merge 11 commits into
Optic00 wants to merge 11 commits into
Conversation
The share sheet needs a real file on disk whose name the recipient reads. Files land in <temp>/stenoai-share/ and are swept only at the next start, past 24 hours: deleting earlier would pull an attachment out from under an open mail draft, and ShareMenu.popup() never tells the app when that ends.
The handler materialises a note as a PDF or markdown file in the managed share temp directory and pops the native macOS sheet on it. Capability is answered by the main process rather than the renderer's isMac, which is a navigator.platform constant frozen at import and untestable in both directions. The platform is checked twice on purpose: the renderer flag decides what to draw, this check is what stops a stray call from taking the main process down on 'new undefined(...)'.
Both copy icons leave the toolbar and the two file saves leave the ... menu, which keeps its management actions. Adds Save notes as .md, which needs no main-process code: export-transcript already takes an arbitrary string. The notes .md filename carries a -notes suffix so it cannot collide with the transcript's; on the share path both land in one directory where the second write would silently replace the first, including under an open mail draft. A copy's auto-close is cancelled on any open/close in between - the timer belongs to the menu instance the copy happened in, and left pending it shut a freshly reopened menu. Covered by a regression spec.
Rendered only when the main process reports a share sheet, so the absent branch is testable on any OS - that branch is what keeps Windows away from a ShareMenu it does not export. The pending state is not cosmetic: sharing has no dialog, so up to 15 seconds pass between click and sheet and everyone clicks twice. The anchor rectangle is read before the await, since the entry re-renders as Preparing and the menu can reflow.
T2 drives share-note-file through the preload bridge and asserts on disk: directory, filename, PDF magic bytes, verbatim UTF-8, the notes/transcript name split, the filename guard, and that the capability follows the platform. Cross-family review found two real defects, both fixed with a regression test: - The auto-close cancel was not enough for Copy transcript, which awaits the clipboard before scheduling it. A menu dismissed and reopened during that await had no timer to cancel, and the resolving promise then closed the new one. The menu instance now carries an epoch that both the scheduling and the firing check. - The anchor rectangle is read up to 15 seconds before the sheet pops, so a scroll or resize can leave it outside the window. Out-of-range anchors are dropped, since Electron's own placement at least uses current coordinates.
The atomic rename can replace a same-user file planted in the share directory, because the filename is deliberately not randomised. Accepted, but the comment claimed the overwrite is always our own file.
IS_E2E is nothing but an environment variable, so a signed build started with STENOAI_E2E=1 plus STENOAI_E2E_EXPORT_PATH would write a user's export to an attacker-chosen location and never show the save dialog. Same for the diagnostics save path, and for share-note-file returning the written path and skipping the sheet. All four seams now also require !app.isPackaged. Every e2e lane launches the dev binary from source, including the release gate's T1 smoke, so no legitimate run is packaged. IS_E2E itself is unchanged: its other uses gate the tray, dock, scheduler and telemetry, and re-gating those is a wider change than closing this hole.
Collaborator
Author
Merge order note: #426 should land before thisFlagging an overlap rather than proposing a decision - the order is yours. #426 ("save the note that's on screen as PDF, not always the Standard note") touches the same four files as this PR, and two of the overlaps are semantic rather than textual:
Suggested order: #426 first, then rebase this. Same note is on #426. Happy to do the rebase as soon as it lands. |
One conflict, in app/package.json: main's stenolabs#440 appended update-error-copy.test.js to test:unit while this branch appended share-temp.test.js. Both kept — the lists are additive and both files exist. The overlap flagged in the PR's merge-order note is now resolved by main itself: stenolabs#426 landed, so "Save notes as PDF" already follows the note on screen. The semantic half of that integration is the commit that follows this one.
Integrating main after stenolabs#426 landed. Git merged both sides cleanly, and that is the problem: stenolabs#426 taught "Save notes as PDF" to export whichever note is on screen, but "Share notes as PDF" was written on this branch in parallel and still called buildNotesHtml(noteSections). Merged, the two adjacent entries in the same menu disagreed about the same document - with a template report open, sharing sent the Standard structured note. That is the exact bug stenolabs#426 was filed for, one entry lower. Both PDF surfaces now go through one buildNotesPdfHtml(), so they cannot drift apart again. It is a function rather than a memo for the reason stenolabs#426 gives: the branded shell is ~45KB with its base64 font, so it is built on click. Also repoints notes-pdf-export.t1's report case at the Share menu. This branch moves the save entries out of the "..." menu, so the spec's 'More options' click opened a menu that no longer holds "Save notes as PDF" - it timed out on the merge, not on a product fault. Its sibling case in the same file already used the Share fixture. Tests: a share-menu.t1 case asserting the SHARED payload carries the rendered report, the template name, no Standard sections and no leaked reasoning. It needs the whole payload, not the log's 200-char head - a branded PDF's first 200 chars are doctype and font CSS - so the share mock gains STENOAI_E2E_SHARE_PAYLOAD_PATH, the seam STENOAI_E2E_EXPORT_PATH already gives the save path. Verified the case fails without the fix (the shared HTML has no report content) and passes with it; full T1 suite green, 68 passed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
One Share menu, and a note you can actually send to someone
Everything that carries a note out of the app was scattered. Two clipboard actions sat as icons in the header toolbar, two file exports sat inside the
…popover betweenView containing folderandDelete note, and there was no way at all to send a note to another person or device without saving a file and attaching it by hand.This collects all of it into one Share menu, and adds three entries that hand a file to the native macOS share sheet - which brings AirDrop, Mail, Messages and Notes along for free.
What it does
The header toolbar loses both copy icons. In their place is a Share button whose menu holds three groups:
On Windows the third group is not rendered, and five entries remain. The
…popover keeps every management action it owns -View containing folder,Re-transcribe recording,Share with <org>/Unshare,Delete note- unchanged.The org share deliberately stays out of the new menu. It uploads to a server and is a persistent state with an inverse action, where every entry here is a one-off local act. Putting both under one "Share" heading would erase the distinction a privacy-positioned app needs to keep visible.
Save notes as .mdneeds no main-process code.export-transcriptalready takes an arbitrary string and filters to.md, so the entry is the new markdown builder's output handed to the existing handler.How it is built
app/renderer/src/lib/notesMarkdown.ts- a third pure builder on theStructuredNoteSectionsshape thatbuildNotesCopyTextandbuildNotesHtmlalready share, in the same section order. Like them it takes the active template report into account, so what leaves matches what is on screen. Escapes nothing: the input is the model's own prose and is already markdown-ish.app/share-temp.js- resolves the share temp directory and sweeps it. Its own file rather than more lines inmain.jsbecause the sweep is the one piece here with a correctness rule worth a unit test, and because it takes the base path as an argument instead of importingelectron, which is what makes it testable under plainnode:test.share-note-fileinapp/main.js- follows the same split as the three export handlers above it: the renderer builds the content, the main process owns the bytes. No save dialog, because the destination is our own managed directory and the user never picks a location.share-capability- answers "is there a share sheet here", queried once by the renderer.The platform gate is a capability flag, not
isMacThe obvious switch would be the renderer's existing
isMac, and it is unusable. It is a module-level constant derived fromnavigator.platformat import time (app/renderer/src/lib/utils.ts:8), so nothing can flip it at runtime, and T1 runs on Ubuntu in CI. Gating on it would make the "entries present" branch untestable in CI and the "entries absent" branch untestable on a developer's Mac - and the absent branch is precisely the one that keeps Windows away from aShareMenuthat Electron does not export there.The entries hang off a value supplied by the main process instead, which is the more honest source anyway since the capability belongs to the main process. Both branches have a T1 spec.
The platform is checked twice on purpose. The renderer's flag decides what to draw; the handler's own check is what stops a stray call from taking the main process down on
new undefined(...). The handler also verifiesShareMenuis still a function, so a future Electron dropping the export degrades to a hidden menu group rather than a crash.Temp file lifecycle, the one place a mistake costs user data
Files go to
app.getPath('temp')/stenoai-share/with the same atomic tmp+rename as the other handlers, and are never deleted during a running session.Deleting after the sheet closes would destroy the attachment. A user who picks Mail then has an open draft and may type for ten minutes; AirDrop waits for the receiving side to accept.
ShareMenu.popup()has no completion callback, so the app is never told when that ends. Cleaning up on quit carries the same risk - an open draft at quit time is not exotic.So cleanup is a sweep at the next app start of anything older than 24 hours. Files outlive their own session including any draft, the directory still cannot grow without bound, and there is no moment at which the app pulls a file out from under a reader. The unit test's load-bearing case is the 23-hour file that must survive; nothing else in the suite would catch an over-aggressive sweep.
Filenames are user-visible, and two of them used to collide
The temp filename becomes the attachment name the recipient reads, so
defaultExportFilename()'s dated slug (2026-07-30-quartalsplanung.pdf, umlauts transliterated) is used verbatim, deliberately without a random suffix.That surfaced a bug the design did not foresee: notes-as-
.mdand transcript-as-.mdwould both have produced<date>-<slug>.md. Harmless for the save actions, where a dialog stands in between - but on the share path both land in one directory, where the second write silently replaces the first, including under an already-open mail draft whose attachment would quietly become the other document. The notes exports now carry a-notessuffix. Asserted in both T1 and T2.The pending state is not cosmetic
Save notes as PDFshows its dialog first and rasterises afterwards, so the click gets an immediate reaction. Sharing has no dialog: a click is followed by up to 15 seconds of nothing before the sheet appears, and everyone clicks again in that gap. So the clicked entry switches toPreparing…, all three share entries go off, the menu stays open, and only one share may be in flight - guarded in the handler as well as by the disabled attribute, so a re-render between click and state update cannot slip a second one through.Failures go into the existing
exportErrorline. Three are real and all surface: the PDF render can hit its 15 second timeout, the temp directory can be unwritable, andShareMenucan be missing. The startup sweep is the one operation allowed to fail silently - an undeletable temp directory is not a reason to interrupt a launch.Why the diff touches three handlers that have nothing to do with sharing
IS_E2Eis nothing butprocess.env.STENOAI_E2E === '1'. TheSTENOAI_E2E_EXPORT_PATHandSTENOAI_E2E_DIAGNOSTICS_PATHseams inexport-transcript,export-note-pdfandsave-diagnosticsare gated on it alone, so a signed build started with those two variables writes a user's export to an arbitrary location and never shows the save dialog at all.That is pre-existing and predates this branch. It is fixed here because this branch adds a fourth seam of exactly the same shape, and shipping one hardened seam next to three unhardened ones would have been worse than either. All four now also require
!app.isPackagedvia a newALLOW_E2E_PATH_SEAMS;IS_E2Eitself is untouched, since its other uses gate the tray, the dock, the scheduler and telemetry, and re-gating those is a far wider change.!app.isPackagedis safe because every e2e lane launches the dev binary from source (electron.launch({ args: ['.'] })ine2e/fixtures/electron.ts), including the release gate's T1 smoke. Happy to split this into its own PR if you would rather review it separately.Verification
node:test+ 124 vitest,typecheck:rendererclean,lint:renderer36 warnings / 0 errors - identical tomain.ruff check .29, identical tomain; this branch touches zero Python.python -m unittest discover tests497 passed.share-menu.t1(9) drives the menu through mock IPC and asserts both capability branches;share-file.t2(6) drives the real handler through the preload bridge and asserts on disk. Four existing T1 specs moved with the entries and are part of this change.Known gaps, stated rather than left to be assumed
No automated test covers the share sheet itself, only the file handed to it. A native sheet cannot be automated - Playwright can neither see nor dismiss it, and an open sheet blocks the run. Under
IS_E2Ethe handler stops after the write and reports the path. That the sheet opens, that Mail accepts the attachment and that the recipient sees a usable filename was verified by hand and cannot be regression-tested here.The anchor clamp is not unit-covered. The rectangle is read up to 15 seconds before the sheet pops, so a scroll or resize can invalidate it and a window shrink can put it outside the window - worse than passing nothing, since Electron's own placement uses current coordinates. Out-of-range anchors are dropped. The popup is unobservable under the e2e seam, so this is reasoning plus a hand check, not a test.
One disable rule is arguably wrong, and left as designed.
Save notes as .mdand its Share sibling hang offcanExportNotesPdf, which reads the structured note - but the markdown builder carries an open template report, asCopy notesdoes. So on a transcript-only note with a generated report open,Copy notesworks and those two entries are disabled. The existing PDF export has the same gap. One line to widen if you want it widened; not done unilaterally.The atomic rename can replace a same-user file that another process planted in the share directory under one of our names, because the name is deliberately not randomised. Accepted: the filename has to stay the readable one the recipient sees, and anyone who can write there can already read what we put there. The code comment names the exception rather than claiming the overwrite is always our own file.
Deliberately not built
A Windows share equivalent. Windows has
DataTransferManager, Electron does not expose it, and reaching it needs a native addon or a PowerShell detour - a lot of work and a new failure source in a build that ships as alpha. Windows gains the reorganised menu and keeps all five non-sheet actions, which is more than it has today.Keyboard navigation for the popovers. Both the Share menu and the
…menu are plainPopoverplus buttons with no roving focus. That is an accessibility improvement to the existing shared pattern rather than part of this feature, and the entry markup is now a single constant so it can be done in one place.Summary by cubic
Collects all note export actions into a single Share menu and adds macOS share-sheet support to send notes or transcripts as files. Also adds a notes
.mdexport and hardens test-only path seams to avoid misuse in packaged builds.New Features
.md, Save transcript as.md, plus macOS-only entries to Share notes as PDF/.mdand Share transcript.…menu keeps management actions (e.g., View containing folder, Re-transcribe, org share, Delete).share-note-fileto write a PDF or UTF‑8 text into<temp>/stenoai-share/and open the native share sheet; files are anchored to the clicked entry and never deleted during a session. A startup sweep removes files older than 24h.buildNotesMarkdownand a Save notes as.mdpath via existingexport-transcript. Notes.mduses the active report if open. macOS share entries render only whenshare-capabilityfrom main saysShareMenuexists; shows “Preparing…” and allows only one share in flight.Bug Fixes
.mdname collisions by suffixing notes exports with-notesso they don’t overwrite transcript files in the shared directory.STENOAI_E2E_*_PATH) now requireIS_E2Eand!app.isPackagedviaALLOW_E2E_PATH_SEAMS.isMac, preventing hidden crashes on platforms withoutShareMenuand allowing both branches to be exercised in CI.Written for commit 7356d3f. Summary will update on new commits.