Repository navigation
fix(chief memory): a scripted Import Memories run never lists ~/Downloads or ~/Desktop - #28882
Conversation
…loads or ~/Desktop (red) chief.importMemories with no source runs detect --exports for any caller: a script, an agent or the CLI through the control socket makes cmux list ~/Downloads and ~/Desktop (and raise the macOS folder prompt) before the dialog shows. Only the person's own run in the app may ask for the export zips. This commit moves the choice of the detect arguments into ChiefMemoryImportHandlers.detectArguments(muxHome:invocation:) and offerDetectArguments(muxHome:) with no change in behavior, and adds ChiefMemoryImportExportsTests. onlyThePersonsOwnRunListsTheExportZips fails here: every origin still gets --exports. The next commit fixes it.
…Downloads or ~/Desktop A chief.importMemories run that names no source asked optchat-chief for detect --exports whatever its origin, so a script, an agent or the CLI through the control socket made cmux list ~/Downloads and ~/Desktop, the folders macOS guards, with no action of the person. detectArguments now passes --exports only for ActionInvocation.isPersonsOwnRun: origin user, not carried by the control socket, and naming no caller app, agent chat or terminal. Any other run lists only the agents' own folders; a script still imports a zip by naming it (source zip:/path). The predicate moves from ScreenshotHandlers.isPersonsRun into CmuxNextActions, so the raw-pixel rule and this rule are one rule. Reported by the independent review of #28820.
|
cmux status (one comment, edited in place; each line is updated by the job that owns it) Review App of this head (app.zip, kept 1 day)
Default changesThis pull request changes 6 defaults at
Needs a review. It passes with an independent review receipt on this commit: a comment or review whose first line is Merge receipt: merged as Regenerate the snapshot with Shipped in:
|
|
Review (claude-opus independent reviewer): SHIP at b661016 Correctness checked by reading the diff and every entrypoint; no blocking finding.
Non-blocking:
|
|
Non-green checks at head b661016. None of them come from this PR:
|
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
…ds ~/Desktop until the person shows it (#28907) * test(chief memory): a scripted Import Memories run never lists ~/Downloads or ~/Desktop (red) chief.importMemories with no source runs detect --exports for any caller: a script, an agent or the CLI through the control socket makes cmux list ~/Downloads and ~/Desktop (and raise the macOS folder prompt) before the dialog shows. Only the person's own run in the app may ask for the export zips. This commit moves the choice of the detect arguments into ChiefMemoryImportHandlers.detectArguments(muxHome:invocation:) and offerDetectArguments(muxHome:) with no change in behavior, and adds ChiefMemoryImportExportsTests. onlyThePersonsOwnRunListsTheExportZips fails here: every origin still gets --exports. The next commit fixes it. * fix(chief memory): only the person's own Import Memories run lists ~/Downloads or ~/Desktop A chief.importMemories run that names no source asked optchat-chief for detect --exports whatever its origin, so a script, an agent or the CLI through the control socket made cmux list ~/Downloads and ~/Desktop, the folders macOS guards, with no action of the person. detectArguments now passes --exports only for ActionInvocation.isPersonsOwnRun: origin user, not carried by the control socket, and naming no caller app, agent chat or terminal. Any other run lists only the agents' own folders; a script still imports a zip by naming it (source zip:/path). The predicate moves from ScreenshotHandlers.isPersonsRun into CmuxNextActions, so the raw-pixel rule and this rule are one rule. Reported by the independent review of #28820. * test(screenshots): a Screenshots model not started reads and watches no folder (red) ~/Desktop is macOS's default screenshot folder, which macOS guards. The Screenshots page reads and watches its folders as soon as the page is made, and an agent or a script makes it with no action of the person (cmux browser on its tab, the DevTools relay, screenshots.open with focus: true). This commit adds ScreenshotsModel(listsAtOnce:), isListing and startListing() with no change in behavior (every model still lists at once), and ScreenshotsModelListsOnlyWhenStartedTests, which fails here: a model made with listsAtOnce: false lists and watches ~/Desktop. The next commit fixes it and makes the app's page start listing only when it is on screen. * fix(screenshots): a Screenshots page reads ~/Desktop only once it is on screen, and only the person's run puts it there The Screenshots page read and watched its folders, ~/Desktop (macOS's default screenshot folder, which macOS guards) among them, as soon as the page was made. An agent or a script makes it with no action of the person: cmux browser on its tab and the DevTools relay make the page off screen, and screenshots.open with focus: true put the tab in front. ScreenshotsModel(listsAtOnce: false) now reads and watches nothing until startListing(). The app's page model is made that way and starts listing when the page is first on screen (setContentVisible(true), which only the pane that shows the tab calls). screenshots.open selects the tab only for ActionInvocation.isPersonsOwnRun (the rule of #28882); any other run opens it in the background. A tab the person left on screen is restored on screen and lists at once: that is the person's own view, as Chrome reloads a restored tab. * test(screenshots): a restored or agent-shown Screenshots page lists nothing until the person clicks (red) The review of #28907 found that showing the page is not the person's choice: an agent can show a background Screenshots tab (cmux tab focus, a browser mirror, the Home mirror), and a tab restored on screen reads ~/Desktop at launch, which raises the macOS folder prompt after a privacy reset or on a new Mac (LAUNCH-NO-TCC-PROMPTS). This commit adds the seams ScreenshotsView.showsPrompt and showScreenshots(), and ScreenshotsOpen (the decision of a screenshots.open run) with the old behavior, and the tests: - ScreenshotsPageListsOnlyAfterTheClickTests: a page made by the app and shown (restored on screen, or shown by an agent) lists nothing and shows the prompt until the person clicks; it fails here (showing it lists). - screenshotsOpenFromAScriptWithFocusSaysItOpenedInTheBackground (the handler, on a real window): a scripted run with focus says that it opened the tab in the background; it fails here (it says nothing). - ScreenshotsOpenDecisionTests: only the person's run selects and lists at once, and every other run gets a note; it fails here. * fix(screenshots): a Screenshots page lists nothing until the person clicks Show Screenshots in it The review of #28907 (FIX) found that showing the page is not the person's choice. An agent can show a background Screenshots tab (cmux tab focus, a browser mirror, the Home mirror), and a tab restored on screen read ~/Desktop at launch, which raises the macOS folder prompt after a privacy reset or on a new Mac (LAUNCH-NO-TCC-PROMPTS). Chrome's reload of a restored tab is no precedent here: Chrome passes no OS privacy gate. - ScreenshotsView shows a Show Screenshots prompt (title, one line on why, one button) until the person clicks it; only that click starts the model listing. Shown, focused or restored, the page reads and watches nothing. The prompt goes in one frame, with no animation; its fonts come from the app's Typography. - The app's page model is made with listsAtOnce: false, and setContentVisible no longer starts listing. - screenshots.open: only the person's own run (isPersonsOwnRun) brings the tab to the front; any other run, with --focus too, opens it in the background and says so (result opened: background, and a note), never a silent background open. The decision is ScreenshotsOpen. - ScreenshotsModel.apply does nothing before listing starts. - Four new strings in all 21 locales of the Screenshots catalog. * test(screenshots): the click-gate test checks plain values, so a red run never reflects the view In the red run (step c6a93fd37237b33f8c12418c) the test failed on its own assertion, then the test process crashed while Swift Testing described the next failed expectation: it reflected ScreenshotsView and hit a null NSPasteboard. The handler test after it never ran. Each expectation now checks a Bool that is read first. * test(screenshots): no detection before the click, the click remembered per tab, and screenshots.open says what it did (red) The review of 791f3d7 (FIX) found: - folder detection ran before the click (makePage); it reads other apps' containers and checks folders under ~/Desktop, which can raise a macOS prompt at launch for a restored tab; - the keyboard could not reach Show Screenshots (the hidden grid took focus); - the click was kept on the page, so a page made again (hibernation wake, engine switch) asked again; - ScreenshotsModel listed at once by default; - screenshots.open told a script "opened in the background" for a tab already open and listing; the unselected tab and the no-window refusal had no test. This commit adds the seams ScreenshotsGate (unlocked tabs by key), ScreenshotsView.onShowScreenshots and focusTarget, and an injectable ScreenshotEnvironment for ScreenshotsPageService, all with the old behavior, and the tests ScreenshotsGateTests and ScreenshotsGateWiringTests, which fail here. * fix(screenshots): nothing is detected before the click, the click is kept per tab, and screenshots.open says what it did The fixes for the review of 791f3d7 (FIX): - Folder detection (which reads other apps' containers and checks folders under ~/Desktop) runs only after the person's first Show Screenshots click (ScreenshotsGate.mayDetect); before it, a restored or agent-made page reads nothing at all. - The click is kept per tab key (ScreenshotsGate), so a page made again after a hibernation wake or an engine switch lists with no second click. - While the prompt is up, focusing the tab focuses the Show Screenshots button (Space presses it; no Return key equivalent), not the hidden grid. - ScreenshotsModel lists only when its caller says so (listsAtOnce defaults to false). - The prompt's secondary text uses the palette (Palette.textSecondary). - screenshots.open from a script on a tab already open answers opened: existing with listing true or false (and a note while it waits for the click), not "opened in the background". - SystemScreenshotsWorkspace (the open and reveal adapter) moves from CmuxNextApp into CmuxNextScreenshots. - A What's New highlight, and one new string in all 21 locales. * test(screenshots): the answer for a Screenshots tab already open is tested on ScreenshotsOpen The red run at d5198fd (step bfd0f9bd9fc2d72fa22f0be0) showed that screenshotsOpenSaysWhatItDid passed there: its existing-tab part ran only when the new tab was in the pane, and in a test with no daemon the tab never arrives, so that part was skipped. That part moves to ScreenshotsOpenDecisionTests, on ScreenshotsOpen.result: a new tab opened behind, the tab already open and listing, and the tab already open and waiting for the click. The handler test keeps the background answer. * test(screenshots): Space shows the screenshots without Full Keyboard Access, and the wiring test never reads the test Mac's Desktop (red) The third review (FIX at bf28c6a) found that the Show Screenshots button takes the keyboard only with Full Keyboard Access on, so with the default setting Space did nothing; and that one wiring test ran the real detector after its click, which lists the test Mac's own Desktop. spacePressesShowScreenshotsWhileThePromptIsUp: while the prompt is up the pane itself takes the keyboard and Space shows the screenshots; it fails here (the focus target is the button, and Space does nothing). The click test now uses a fake detection environment. The unused ScreenshotsGate.forget leaves the gate test. * fix(screenshots): Space shows the screenshots with the default keyboard setting; the title is themed; a script with no pane is refused; the highlight validates The fixes for the third review (FIX at bf28c6a): - B: while the prompt is up the pane itself takes the keyboard (acceptsFirstResponder) and Space shows the screenshots: a plain button takes the keyboard only with Full Keyboard Access on. After the click the keyboard goes on to the grid. - A: the What's New highlight has its action line (screenshots.open), so the highlight validator passes. - E: the prompt title takes Palette.textPrimary. - F: screenshots.open with no focused pane is refused (No pane is focused), never a silent success. - D: the unused ScreenshotsGate.forget is gone; a click stays for the life of the process (tab keys are durable). - G: the click test uses a fake detection environment. * test(screenshots): the Space test checks plain values, so a red run never reflects the view Its red run (step 7dd4a41d0e08065a142e97fc) crashed while Swift Testing described a failed expectation on two views (a null NSPasteboard in the reflection), as the click test did before. It now compares Bools. * chore(next): PR autofix The regenerated files from autofix run 41198445289 at e04a9b9 (its pr-autofix-patch artifact): ci-target-graph.json (CmuxNextScreenshots now depends on CmuxNextCompat), action-surfaces.json and actions.md (screenshots.open is an effect binding), the CLI and palette reference docs, and schemas/defaults/cmux-next.json. The run's push job did not run: it needs a non-pull_request event, and pull_request is the only trigger. * chore(next): this PR carries no generated output (take the base's copies) The security of the merge (review FIX at cef95c1): the autofix patch was applied with git apply, not with pr-autofix-apply.sh, so it pushed outputs this pull request does not own (bot-only schemas/defaults, the CLI and palette references, and export rows regenerated against an older base; merged with a newer base, action-surfaces.json and actions.md removed 155 of the base's rows, the #21946 problem). generated_outputs.py drift also says ci-target-graph.json is written by the base autofix only. All six now equal origin/feat-cmux-next; the base autofix regenerates them after this pull request merges. The commit message of cef95c1 was also wrong: the export change there was the base's automations.show, not screenshots.open. --------- Co-authored-by: cmux-bot[bot] <348531757+cmux-bot[bot]@users.noreply.github.com>
Summary
Privacy bug (the independent review of #28820 found it): if a
chief.importMemoriesrun names nosource, the action still runsoptchat-chief memory import-from detect --exports, whatever the origin of the run. Thus a script, an agent or the CLI can make cmux list~/Downloadsand~/Desktop(folders that macOS guards, which also shows the folder prompt) before the dialog opens, with no action from the person.After this change, only the person's own run in the app sends
--exports. That run must have theuserorigin, the control socket must not carry it, and it must name no caller app, agent chat or terminal. A run from any other caller lists only the agents' own folders. A script can still import a zip if it names the zip (source: zip:/path).ChiefMemoryImportHandlers.detectArguments(muxHome:invocation:)andofferDetectArguments(muxHome:)are pure functions that the tests call.ScreenshotHandlers.isPersonsRuntoActionInvocation.isPersonsOwnRun(CmuxNextActions). Thus the raw-pixel rule and this rule are one rule. The behavior of screenshots does not change.ChiefMemoryImportExportsTests: the launch-time offer never sends--exports(the reviewer said that this test was missing). Only a person's own run sends it.Testing
Focused command (fleet):
3ad07bc5df4cbce1d136b6c1(aws-m4pro-1) fails on its own assertions:onlyThePersonsOwnRunListsTheExportZipsrecords 9 issues (5 non-user origins, the socket run that claimsuser, and the 3 named callers all get--exports).theLaunchTimeOfferNeverListsTheExportZipspasses, because the offer was already fixed by fix(chief memory): the launch-time memory offer never lists ~/Downloads or ~/Desktop (restores the #28811 test) #28820.ChiefMemoryImportExportsTests,MaskedScreenshotTests,ChiefMemoryImportTests,ActionOriginTests): step6fa3ae1b30565a39628ee9b8(cmux10s-Mac-mini) passes, all 4 suites (32 tests).cmux-ci compile-check --package CmuxNext --app --recipe-version 2): job2cc884a8c56a0c6c096fcb46passed: CmuxNext 202 s, app 53 s, cmux-tui 37 s.Observed / Not run
Observed:
scripts/cmux-next/prepush-gate.py: no new violations. CmuxNextApp is 2 lines smaller (arch-ratchetapp_lines-2).Not run:
action.run chief.importMemoriesto a tagged build.Proof
No capture: the dialog does not change. Only the folders that a scripted run lists change.
Changelog