Skip to content

Agent pane: no system call on the key path to judge who posted an event (P-MAIN S5) - #28065

Merged
lawrencecchen merged 20 commits into
feat-cmux-nextfrom
fix/stall-person-input-pidpath
Nov 1, 2026
Merged

lawrencecchen merged 20 commits into
feat-cmux-nextfrom
fix/stall-person-input-pidpath

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Risk tier: normal, as scripts/ci/risk_tier.py pr --number 28065 grades it (lead decision 2026-11-01 18:03Z: the label follows the repo's risk tool). The change guards permission grants and join consent, so it still has what high would need: an independent SHIP at the exact head and a green fleet compile. risk_tier.py does not list these files as high; that classifier gap goes to cmux-ad.

P-MAIN S5 (found by the S2 stall agent, 2026-10-30). AgentPanePersonInput.isPerson (the agent pane's gesture monitor: every key-down and mouse-down) and PersonInputCheck.isPerson (PersonInputLedger: every key and mouse event of the app) called proc_pidpath on the main thread for each event that another process posted (source pid not 0). That is a system call on the key path.

Why the path is read: only the person may grant in an agent pane (permission Allow, cx-ocdz / cx-1l61) or back a join consent. The HID system's events (pid 0) are the person's. Of the posted events, only the Apple services a person drives the Mac through count (Screen Sharing, Universal Control, Accessibility Keyboard, named by their path under SIP-protected /System), and for the strict join consent check only the WindowServer. A computer-use agent or a script that posts a click must not count. The event itself carries only the pid, not the executable, so the decision cannot come from the event alone (option (a) does not apply).

Fix (option (b)): EventSourceProcesses, one table per app (built in AppServices, injected; no singleton).

  • The key path reads the table under its lock and makes no system call. // perf-hot: marks the three functions.
  • A pid not in the table takes the safe default at once: not the person's. A wrong "no" costs one gesture (the person clicks again); a wrong "yes" would let an automated click grant. The person's own HID input (pid 0) never reaches the table, so local typing and clicks are never affected.
  • The process is read once, on the table's serial queue com.cmuxterm.next.event-source-processes: start time (sysctl(KERN_PROC_PID) p_starttime), path (proc_pidpath), start time again (so both are one process). Neither read has a same-user check, so the WindowServer, launchd and the root Screen Sharing daemon are read as this user (62fc9e9; before it, proc_pidinfo(PROC_PIDTBSDINFO) refused them with EPERM). A live process that is still unreadable keeps the safe default until its exit watch fires; it is not read again for each event.
  • An exit watch (a kqueue EVFILT_PROC with NOTE_EXIT | NOTE_EXEC, registered with EV_RECEIPT and read by a dispatch read source on the same serial queue) is armed before the read. It removes the entry when that process ends or runs a new program (exec: same pid and start time, new image). A pid the kernel gives to a new process, or a new image in the same process, is therefore read again. No timer decides anything (R5). One entry per live posting process, no constant cap (R9).

Lead review, second round:

  • Priming. PersonInputLedger also watches posted mouse moves, scrolls, other drags and modifier changes (primingTypes). For an unknown pid, it queues the table read (EventSourceProcesses.prime: a table lookup, no system call). A Screen Sharing session moves the mouse before it clicks, so its first click already counts.
  • SECURITY, exit watch. The watch is a kqueue EVFILT_PROC/NOTE_EXIT registered with EV_RECEIPT, so a refusal is seen. When the kernel refuses the watch, nothing is kept: no verdict can outlive its process or pass to a process that reuses the pid. What the person sees then: a click or key that Screen Sharing, Universal Control or the Accessibility Keyboard posts never counts as their gesture. A permission Allow through Screen Sharing stays unanswered and they must use the Mac's own keyboard or mouse. Their own local input (HID, pid 0) is never affected. Each new event reads the process again, off the main thread.
  • Fleet probe (SSH diagnostic, uid 501, select.kqueue with EV_RECEIPT): on cmux17, cmux19, cmux21 and cmux23 (macOS 27), NOTE_EXIT attached to launchd (pid 1, root) and to the WindowServer (_windowserver). screensharingd starts only on demand and ran on no host; it runs as root like launchd. theKernelExitWatchAttachesAsThisUser prints the same lines from the fleet test step.
  • plans/cmux-next/arch-ratchet.json: unchanged (feat's file). The PR adds no ratchet exception and no // blocking-allow: escape: the start-time sysctl and proc_pidpath run only on the table's serial queue.

Commits: the seam (no behavior change), red test 1, fix 1, red test 2 (priming, no watch), fix 2; after the base merge, 4cc0a03 moves the wiring into CmuxNextAgentPane (AgentPaneShared, PersonInputLedger.sources) so CmuxNextApp lines do not rise, and drops the exception.

Performance

Main-thread time per event, measured in EventSourceProcessesTests.perEventMainThreadCost (mean over 20,000 events, one fleet Mac, not an exclusive host).

metric before after delta harness or run link
main-thread time per posted event (person-input check) 4333 ns (proc_pidpath per event) 323 ns (table read) -4010 ns perEventMainThreadCost, fleet job 5e30898cbc1d8b0a1826e976 at ff83b4e
main-thread time per mouse move of the person (pid 0), new ledger work 0 ns (not watched) 208 ns +208 ns perEventMainThreadCost, fleet job 5e30898cbc1d8b0a1826e976 at ff83b4e

No capture: no visible change; the person-input check gives feat's verdicts for every posting process, including the WindowServer and root Screen Sharing (see the BLOCK at 072f126 and its fix 62fc9e9), only off the main thread; an unread or unreadable posting process is refused until it is read.

Review

Review (claude-opus-5.5-merge-review-subagent): SHIP at 97fd679 (receipt: #28065 (comment); merge of origin/feat-cmux-next, compile c17fa3d30e8e96886973f04c). Before the merge: SHIP at ff83b4e (#28065 (comment)). Earlier round: FIX at 3f13e1d, all findings fixed. Follow-up (non-blocking): drop the default eventSources: argument of AgentPaneView.init and AgentTabStore.init, so that a new caller cannot get an unprimed private table.

Changelog

none

Review round 1 (claude-opus-review-subagent, FIX at 3f13e1d): fixed the exec case (NOTE_EXEC), fail closed unless exactly one EV_RECEIPT receipt has no error, a kernel-wait test that could hang now fails after 10 s, and perEventMainThreadCost prints the main-thread ns per event (R10). The reviewer's finding that the blocking-allow exception is not needed is wrong for the installed hook: its raw-blocking-ext rule (proc_pid*) refused the push without it.

Testing

Regression: EventSourceProcessesTests (CmuxNextAgentPaneTests), run by scripts/ci/package-test-lane.sh suite Packages/macOS/CmuxNext EventSourceProcessesTests.

Step Ref Job Result
compile 3f13e1d3bec bfa3612a880006dfa9828091 passed (CmuxNext 275 s, app 50 s, cmux-tui 27 s)
red 1 (per-event read on main) b89a7aaa105 18c989cb925e7f1104c5446e failed as expected: theKeyPathReadsOnlyTheTable (101 reads, on main), aReusedPidIsReadAgain, aPidWithNoProcessIsNotKept, theStrictCheckReadsOnlyTheTable
red 2 (no priming, kept without a watch) d9c4ed99a67 abffd290d5e174c34c5dcebe failed as expected: only aMouseMoveFillsTheTableBeforeTheFirstClick and aProcessWithNoExitWatchIsNeverKept
green 3f13e1d3bec 98d9a0eafbca030eb8d0fa80 7/7 passed on aws-m4pro-6; exit-watch-proof: launchd pid 1 as uid 501: attached, WindowServer pid 164 as uid 501: attached, screensharingd not running
compile ff83b4e1f5d ddbe9951d28d3c8897f4a6b0 passed (CmuxNext 255 s, app 76 s, cmux-tui 39 s)
green at the head ff83b4e1f5d 5e30898cbc1d8b0a1826e976 9/9 passed; theKernelWatchFiresOnExec passed; per event on main: person's mouse move 208 ns (new), posted event from the table 288-323 ns, base proc_pidpath 4333 ns
red 3 (no exec notice) 38ce555794a ebac231666cc7f1a9e7489d6 NO verdict: worker lease lost during compile, 0 test lines
red 3 infra-rerun 38ce555794a a74dba670ac66b514e5e0355 failed as expected on cmux11s-Mac-mini: only theKernelWatchFiresOnExec (Expectation failed: await Self.fires(fired.stream) after 10.029 s); the other 8 passed

Observed / Not run

Observed: the jobs above; an SSH kqueue probe as uid 501 on cmux17, cmux19, cmux21, cmux23 (macOS 27) attached NOTE_EXIT to launchd (root) and WindowServer (_windowserver).
Not run: the typing bench (perf claim: none); screensharingd ran on no host (it starts on demand; it runs as root like launchd); no local Mac build or test (repo rule).

…cted EventSourceProcesses

No behavior change: AgentPanePersonInput.isPerson and PersonInputCheck.isPerson read the posting
process's path through one seam, EventSourceProcesses.path(of:), which still calls proc_pidpath
(AgentPanePersonInput.executablePath, now with the start time around it) on the caller's thread for
each event. The app builds one table in AppServices (eventSources) and passes it to
PersonInputLedger, the join consent sheet's check and each AgentPaneView, so tests can inject the
resolver; the next commit's regression test uses it.
…nt (red)

P-MAIN S5: AgentPanePersonInput.isPerson and PersonInputCheck.isPerson call proc_pidpath on the
main thread for each key and mouse event a process posts. EventSourceProcessesTests injects a
resolver that records each read and its thread, and fails here: every event reads the process,
on the main thread. It also covers a pid reused by a new process (a new start time is read
again) and a pid with no process (not kept).
…osting process

P-MAIN S5 (found by the S2 stall agent): AgentPanePersonInput.isPerson (the agent pane's gesture
monitor, every key- and mouse-down) and PersonInputCheck.isPerson (PersonInputLedger, every key
and mouse event of the app) called proc_pidpath on the main thread for each event a process
posted (source pid not 0: Screen Sharing, Universal Control, a computer-use agent, a script).

EventSourceProcesses is now a table: the key path reads it under its lock and makes no system
call. A pid not read yet takes the safe default at once (not the person's: no gesture or grant is
backed on a guess; the person's HID input, pid 0, never reaches the table), and the process is read
once on the table's serial queue (start time, path, start time again). An exit watch armed before
the read, on the same queue, removes the entry when that process ends, so a pid the kernel gives to
a new process is read again. No timer decides anything (R5); one entry per live posting process,
no constant cap (R9).

Regression: EventSourceProcessesTests (red at the previous commit, green here).
@lawrencecchen lawrencecchen added the risk:normal Risk tier (docs/ci/risk-tiers.md) label Oct 30, 2026
@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

cmux status (one comment, edited in place; each line is updated by the job that owns it)

Review 4b705a7: HQ feed card · gallery before/after runs · checks

App of this head (app.zip, kept 1 day)

gh workflow run reload-build.yml -R manaflow-ai/cmux --ref feat-cmux-next -f tag=pr28065 -f ref=fix/stall-person-input-pidpath -f platform=macos -f backend_mode=local

red: cmux-next checks (god files, concurrency, crash safety, l10n); base red: the same failure on the base fd9dbf9; does not block

Shipped in:

…t without an exit watch (red)

Lead review of #28065: (1) the first posted click of a new source (a Screen Sharing session)
must not be lost to the safe default when the session moved the mouse first: the ledger now sees
mouse moves, scrolls and modifier changes (primingTypes) and EventSourceProcesses.prime(_:) must
queue the read, with no system call on the event path. Red here: prime does nothing.
(2) SECURITY: a verdict must never outlive its process. The exit watch now reports whether the
kernel attached it (a kqueue EVFILT_PROC/NOTE_EXIT with EV_RECEIPT); with no watch nothing may be
kept. Red here: the entry is kept without a watch.
theKernelExitWatchAttachesAsThisUser prints, on a fleet Mac, whether the watch attaches as a
normal user to launchd, the WindowServer and screensharingd (exit-watch-proof: lines), and checks a
child's exit reaches the handler.
… no verdict without an exit watch

- PersonInputLedger also watches mouse moves, scrolls, other drags and modifier changes
  (primingTypes); for a posted one it queues the read of an unknown pid
  (EventSourceProcesses.prime, a table lookup, no system call). A remote session moves the
  mouse before it clicks, so its first click already counts.
- SECURITY: the exit watch is a kqueue EVFILT_PROC/NOTE_EXIT registered with EV_RECEIPT, so a
  refusal is seen. A pid whose watch the kernel refuses is never kept: its events are not the
  person's, and the next event reads it again off the main thread. No old verdict can pass to a
  process that reuses the pid.

Regression: EventSourceProcessesTests (red at the previous commit, green here).
…ocking-allow exception

Lead decision on #28065: the exception stays, with an owner and an expiry.
@lawrencecchen lawrencecchen added risk:high Risk tier (docs/ci/risk-tiers.md) and removed risk:normal Risk tier (docs/ci/risk-tiers.md) labels Oct 30, 2026
@blacksmith-sh

blacksmith-sh Bot commented Oct 30, 2026 •

Copy link
Copy Markdown

Found 13 test failures on Blacksmith runners:

Failures

Test View Logs
/tmp/tmp5hziiy2w/summary-missing.json (live):/summary-missing.json (live): View Logs
/tmp/tmpz_sci91f/summary-fail.json (live):/summary-fail.json (live): View Logs
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows/
[new_workspaces] debug.new_workspace timed 0 of 50 new workspace rows
View Logs
[new_workspaces] the window draws no frames/[new_workspaces] the window draws no frames View Logs
[tabs] debug.tab_switch timed 0 of 40 switches/
[tabs] debug.tab_switch timed 0 of 40 switches
View Logs
[tabs] the window draws no frames/[tabs] the window draws no frames View Logs
main.DeveloperIdAppTests.test_download_prints_the_active_profile_only/
test_download_prints_the_active_profile_only
View Logs
nightly must build and hand off the macOS 26 helper and Xcode 26.5 app before publishin
g/
nightly must build and hand off the macOS 26 helper and Xcode 26.5 app before publishin
g
View Logs
stubs: /System/Library/Frameworks/Security.framework/Versions/A/Security has no stub/
Security has no stub
View Logs
stubs: CoreFoundation.tbd: install-name /System/Library/Frameworks/
CoreFoundation.framework/Versions/A/CoreFoundation, missing 1: ['_CFRetain']/
CoreFoundation, missing 1: ['_CFRetain']
View Logs

...and 3 more test failures. View all on Blacksmith

Fix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@github-actions github-actions Bot added risk:normal Risk tier (docs/ci/risk-tiers.md) and removed risk:high Risk tier (docs/ci/risk-tiers.md) labels Oct 30, 2026
…s; per-event cost (red)

Review of #28065 at 3f13e1d (claude-opus-review-subagent, FIX):
- finding 3: a table entry survives an exec (same pid and start time, new image). Red here:
  theKernelWatchFiresOnExec, a child that execs /bin/sleep, gets no notice (NOTE_EXIT only).
- finding 5: theKernelExitWatchAttachesAsThisUser could hang on a nil watch; it now requires the
  watch, and both kernel waits fail after 10 s instead of hanging (test deadline only).
- finding 1 (R10): perEventMainThreadCost prints the main-thread ns per event of the new work on
  the person's mouse moves, of a posted event read from the table, and of the base's per-event
  proc_pidpath (person-input-cost: lines). A measurement, not a time assertion.
…losed on an odd kevent receipt

Review of #28065 at 3f13e1d:
- finding 3: the watch is EVFILT_PROC NOTE_EXIT | NOTE_EXEC, so an entry leaves when its process
  runs a new program (same pid and start time, new image) as when it ends; the next event reads
  the new image off the main thread.
- finding 4: the watch counts as attached only for exactly one EV_RECEIPT receipt with EV_ERROR
  and data 0; anything else is a refusal (nothing kept).

Regression: EventSourceProcessesTests.theKernelWatchFiresOnExec (red at the previous commit).
…atch's source

The compiler reports it unnecessary (DispatchSourceRead is Sendable).
@cursor

cursor Bot commented Oct 30, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot 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.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-opus-5.5-review-subagent): SHIP at ff83b4e
R1: pass - one path: both checks read only EventSourceProcesses.path(of:) and the one read in read(_:) fills it. The only change in output: a pid that is not read yet gets the safe default (not the person's). That is fail-closed, and mouse moves, scrolls and modifier changes prime the pid before most first clicks. R5: pass - correctness comes from order on one serial queue: the watch is armed before the read, and the exit handler runs after it. It does not come from timing. Tests use settled(). The 10 s deadline is in tests only and fails a test, it decides nothing. R9: pass - no constant cap. The bound is stated: one entry and one kqueue fd per live process that posted an event, and each leaves when its process ends or execs.
exception reviewed: SHIP - the // blocking-allow: on proc_pidinfo (EventSourceProcesses.swift:163) is true: the call runs only in read(_:), which only queue.async reaches (:98). The worktree's own scripts/cmux-next/guardrails.py and origin/feat-cmux-next have no rule that names proc_pid*, so the escape is needed only for the installed hook copy. That copy holds raw-blocking-ext from b1d28bf/4510b8253fff (on cp/21 and cp-scratch branches, not on feat-cmux-next). The escape stays correct when that rule lands, and the ratchet counts it, so the entry is needed while the comment stays. - expiry "2026-12-31: when CmuxNextCompat gets a system-call seam (blockingCall(.systemCall)), wrap the read in it and delete this entry"
perf claim: none
main-thread time (job 5e30898cbc1d8b0a1826e976): new ledger work on the person's mouse move 208 ns/event; posted event from the table 288 ns (posted move), 323 ns (isPerson on a posted click); old proc_pidpath per posted event 4333 ns

Findings (file:line at ff83b4e, EventSourceProcesses.swift unless named):

  1. Pid reuse: pass. Each entry has an EVFILT_PROC watch, armed in read (:129) before resolve (:133). The exit handler runs on the same serial queue after this item, so it always finds and removes the entry it was armed for (exited, :143-150, matched by watch id). If a pid is reused before the read, the new process is read, and the old watch then removes that entry. Safe.
  2. Exit vs read order: pass. If the process ends between arm and read, the kqueue stays readable and the dispatch read source (level-triggered, resumed at :179) delivers after the current item. If the process ends before arm, the receipt fails with ESRCH and nothing is kept.
  3. .reading: pass. It is set only from nil (:93-95), so one read per pid at a time. exited matches only .read with its own watch id, so an old exit cannot remove a newer .reading, and a .read entry is never overwritten (no lost watch).
  4. Refused watch keeps nothing: pass (:129-131); aProcessWithNoExitWatchIsNeverKept covers it. No process: the entry is removed and the watch is cancelled (:133-137).
  5. Exec: pass. NOTE_EXIT | NOTE_EXEC (:188), and theKernelWatchFiresOnExec proves the kernel delivers it. Red job ebac231666cc7f1a9e7489d6 at 38ce555 gave NO verdict: the worker lease was lost during compile and it ran 0 test lines. From the source at 38ce555 (fflags UInt32(NOTE_EXIT) only, line 185), the test would fail there: the exec gives no notice, the child keeps sleeping 30 s, and fires returns false at 10 s. The test is unchanged from 38ce555 to head. Residual risk (not blocking): between an exec and its handler hop, the old path still answers. That matters only if a listed Apple service (screensharingd, Universal Control, AssistiveControl) execs into other code.
  6. EV_RECEIPT and fd: pass. Only count == 1 with EV_ERROR and data 0 is treated as attached; any other result closes the fd (:193-196). On success the fd is closed only by the source's cancel handler (:178), which runs once. source.cancel() from the event handler, from exited and from the miss path in read is idempotent. No leak. Tables are never torn down, but the app has one table for its whole run.
  7. Lock: pass. Main holds the Mutex only for a dictionary lookup or insert (:89-97). queue.async, watchExit, resolve and every cancel() run outside the lock.
  8. Sendable and isolation: pass. Final Sendable class with only let storage (Sendable closures, DispatchQueue, Mutex). PersonInputLedger is @mainactor, and AppServices.personInput is now lazy; it is started in AppDelegate.swift:343.
  9. Priming: pass. observe (AgentPanePersonInput.swift:147) records only types, which are unchanged from the base. primingTypes (:109) do not overlap them and only call prime, so last is never touched (aMouseMoveFillsTheTableBeforeTheFirstClick checks this). postingPID (:51) drops 0 and this process's pid, so HID input never reaches the table.
  10. HID pid 0 and own pid: unchanged. In the agent pane, 0 is the person and, in DEBUG, this process's own pid is the person. In PersonInputCheck, 0 with the HID state is the person and this process's own pid is not. New: pid_t(exactly:) replaces truncatingIfNeeded, which tightens the check (an out-of-range pid can no longer alias a real one).
  11. Callers: all updated. git grep for PersonInputCheck(, PersonInputLedger(, currentIsPerson, AgentPanePersonInput.record, isPerson( and AgentPaneView( shows that the one production AgentPaneView construction (AgentTabs.swift:532) and the one AgentTabStore construction (AgentTabs+Wiring.swift:113) pass services.eventSources, and JoinConsent+App.swift:91 uses the shared table.
  12. Tests: deterministic. They order work with settled() and fake kernels; the real-kernel tests use child processes (defer terminate) and the 10 s deadline exists only for the failure case. The cost test asserts no time.

Non-blocking notes:

  • The default eventSources: EventSourceProcesses = EventSourceProcesses() on AgentPaneView.init (AgentPaneView.swift:147) and AgentTabStore.init (AgentTabs.swift:233) is a trap: a later production caller that omits the argument gets a private table that is never primed, so posted Screen Sharing clicks fail closed. This is not a security risk.
  • threading-model.md:148 says "one item per new posting process". With a refused watch or a missing process, one read is queued per event; that is bounded by the event rate, not by processes.
  • The PR is labeled risk:normal. It gates permission grants and join consent, so it is in the risk:high tier; this receipt is meant as that tier's evidence.
  • My one read of the 5e30898cbc1d8b0a1826e976 log was cut off by my own filter before the cost lines. The numbers above are the ones the run reported to the requester; I did not read them a second time.

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Oct 30, 2026
…erson-input-pidpath

# Conflicts:
#	Packages/macOS/CmuxNext/Sources/CmuxNextApp/AgentTabs.swift
#	plans/cmux-next/arch-ratchet.json
@lawrencecchen lawrencecchen removed the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Oct 30, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Merge of feat-cmux-next pushed as 072f126. There were 3 conflicts:

  • AgentPaneView+ContextMenu.swift: the result is feat's attachment-source recording with this PR's currentIsPerson(sources:) guard.
  • AgentTabs.swift and AgentTabs+Wiring.swift: the result keeps paneShared and feat's daemons: and updates:.

I removed ready-to-land. I do not add it back, because the independent review of this head is BLOCK: #28065 (comment).

The finding is in this PR's own code, not in the merge. EventSourceProcesses.startTime(pid) uses proc_pidinfo(PROC_PIDTBSDINFO). The kernel refuses that call (EPERM) for a process that another user owns, for example the WindowServer, launchd or a root screensharingd. proc_pidpath has no such check. So events that these processes post never count as the person's input anymore:

  • The join consent check and the Settings full-access check always refuse them.
  • A person on Screen Sharing cannot answer Allow in an agent pane.

The reviewer suggests PROC_PIDUNIQIDENTIFIERINFO (p_uniqueid) or sysctl(KERN_PROC_PID). Add a fleet test that runs the real read against the WindowServer and pid 1. The fleet compile and EventSourceProcessesTests at this head are running and will be in the wave report.

…sses; a refused read is not read again per event (red)

Review of #28065 at 072f126 (BLOCK): proc_pidinfo(PROC_PIDTBSDINFO) has a same-user check, so
the WindowServer, launchd and root screensharingd read as nil and never count as the person's;
and each of their events started a new read and a new kernel watch. aPidWithNoProcessIsNotKept
becomes aProcessThatEndedBeforeItsReadIsNotKept (its watch fires: the kernel refuses a watch on
a pid with no process).
…ROC_PID), which has no same-user check; keep an unreadable live process until its exit

proc_pidinfo(PROC_PIDTBSDINFO) refuses another user's process (EPERM), so the WindowServer,
launchd and root screensharingd were never read and never counted as the person's (join consent,
the Settings full-access switch, Allow over Screen Sharing). sysctl(KERN_PROC_PID) p_starttime is
public on macOS 14+ and reads them as uid 501 (fleet probe: cmux17, aws-m4pro-1, macOS 27);
PROC_PIDUNIQIDENTIFIERINFO also reads them but is not in the public SDK headers. A live process
that is still unreadable is kept as unreadable (the safe default) until its exit watch fires, so
its events no longer post a read and a new kqueue each.

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

5 issues found across 17 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="plans/cmux-next/threading-model.md">

<violation number="1" location="plans/cmux-next/threading-model.md:152">
P2: This queue has no back-pressure despite the enforced threading model requiring every queue to be bounded. Add a finite admission bound and fail closed for unknown PIDs once it is reached, while releasing the entry and watch on process exit.</violation>

<violation number="2" location="plans/cmux-next/threading-model.md:152">
P3: The one-item-per-process claim is false when exit-watch registration fails: each later event re-enqueues a read after `read(_:)` removes the entry. Document this retry path instead of claiming no back-pressure.</violation>
</file>

<file name="Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/EventSourceProcessesTests.swift">

<violation number="1" location="Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/EventSourceProcessesTests.swift:141">
P2: This test does not exercise the documented watch/read race because `fake.end(4343)` runs only after the read has settled. Make the fake end the process during `resolve`, so the test catches an exit callback racing with a nil read and verifies the entry is not retained.</violation>
</file>

<file name="Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/EventSourceProcesses.swift">

<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/EventSourceProcesses.swift:148">
P1: This publication can briefly let a reused PID or newly exec'd image inherit the previous process's verdict: the exit/exec callback shares the read queue, so it cannot invalidate the entry while `resolve` is running, and `path(of:)` can read the newly published stale entry before that callback runs. Revalidate the watched identity or invalidate it atomically before exposing the resolved path.</violation>
</file>

<file name="Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneShared.swift">

<violation number="1" location="Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneShared.swift:23">
P3: The shared initializer drops `testRoot`, `chromium`, and `terminations` from `AgentPaneView`'s designated initializer. Callers using shared pane objects therefore silently get the default page-host and termination behavior and cannot inject test dependencies; add and forward these parameters.</violation>
</file>

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

state.withLock { $0.entries[pid] = .unreadable(watch: watch, cancel: cancel) }
return
}
state.withLock { $0.entries[pid] = .read(process, watch: watch, cancel: cancel) }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: This publication can briefly let a reused PID or newly exec'd image inherit the previous process's verdict: the exit/exec callback shares the read queue, so it cannot invalidate the entry while resolve is running, and path(of:) can read the newly published stale entry before that callback runs. Revalidate the watched identity or invalidate it atomically before exposing the resolved path.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/EventSourceProcesses.swift, line 148:

<comment>This publication can briefly let a reused PID or newly exec'd image inherit the previous process's verdict: the exit/exec callback shares the read queue, so it cannot invalidate the entry while `resolve` is running, and `path(of:)` can read the newly published stale entry before that callback runs. Revalidate the watched identity or invalidate it atomically before exposing the resolved path.</comment>

<file context>
@@ -0,0 +1,221 @@
+            state.withLock { $0.entries[pid] = .unreadable(watch: watch, cancel: cancel) }
+            return
+        }
+        state.withLock { $0.entries[pid] = .read(process, watch: watch, cancel: cancel) }
+    }
+
</file context>

- Input: `com.cmuxterm.next.event-source-processes` (``EventSourceProcesses``: the executable path and start time of
each process that posts key and mouse events, read once per process with `proc_pidinfo`/`proc_pidpath`, user-initiated,
serial; it also runs that process's exit watch, which removes the entry. The key path only reads the table, never
a system call; one item per new posting process, so no back-pressure)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: This queue has no back-pressure despite the enforced threading model requiring every queue to be bounded. Add a finite admission bound and fail closed for unknown PIDs once it is reached, while releasing the entry and watch on process exit.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At plans/cmux-next/threading-model.md, line 152:

<comment>This queue has no back-pressure despite the enforced threading model requiring every queue to be bounded. Add a finite admission bound and fail closed for unknown PIDs once it is reached, while releasing the entry and watch on process exit.</comment>

<file context>
@@ -146,6 +146,10 @@ matches the label's literal text; `<x>` matches an interpolation). Its owner row
+- Input: `com.cmuxterm.next.event-source-processes` (``EventSourceProcesses``: the executable path and start time of
+  each process that posts key and mouse events, read once per process with `proc_pidinfo`/`proc_pidpath`, user-initiated,
+  serial; it also runs that process's exit watch, which removes the entry. The key path only reads the table, never
+  a system call; one item per new posting process, so no back-pressure)
 - Diagnostics: `cmux-next.hang-episodes`, `cmux-next.hang-episodes.symbols`, `com.cmuxterm.app.next.run-marker`,
   `com.cmuxterm.app.next.crash-forwarder`, `com.cmuxterm.app.next.owner-panic-forwarder`,
</file context>

#expect(!AgentPanePersonInput.isPerson(event, sources: table))
await table.settled()
#expect(table.cached(4343) == nil)
fake.end(4343) // its exit watch fires

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: This test does not exercise the documented watch/read race because fake.end(4343) runs only after the read has settled. Make the fake end the process during resolve, so the test catches an exit callback racing with a nil read and verifies the entry is not retained.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Tests/CmuxNextAgentPaneTests/EventSourceProcessesTests.swift, line 141:

<comment>This test does not exercise the documented watch/read race because `fake.end(4343)` runs only after the read has settled. Make the fake end the process during `resolve`, so the test catches an exit callback racing with a nil read and verifies the entry is not retained.</comment>

<file context>
@@ -0,0 +1,361 @@
+        #expect(!AgentPanePersonInput.isPerson(event, sources: table))
+        await table.settled()
+        #expect(table.cached(4343) == nil)
+        fake.end(4343) // its exit watch fires
+        await table.settled()
+        fake.set(4343, EventSourceProcess(start: 300, path: Self.screenSharing))
</file context>

Comment thread plans/cmux-next/threading-model.md Resolved

extension AgentPaneView {
/// A pane over the app's shared objects (``AgentPaneShared``); the other parameters are as in the designated init.
public convenience init?(model: AgentPaneModel, source: AgentPaneSource? = nil, renderRate: AgentPaneRenderRate = .capped,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The shared initializer drops testRoot, chromium, and terminations from AgentPaneView's designated initializer. Callers using shared pane objects therefore silently get the default page-host and termination behavior and cannot inject test dependencies; add and forward these parameters.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneShared.swift, line 23:

<comment>The shared initializer drops `testRoot`, `chromium`, and `terminations` from `AgentPaneView`'s designated initializer. Callers using shared pane objects therefore silently get the default page-host and termination behavior and cannot inject test dependencies; add and forward these parameters.</comment>

<file context>
@@ -0,0 +1,28 @@
+
+extension AgentPaneView {
+    /// A pane over the app's shared objects (``AgentPaneShared``); the other parameters are as in the designated init.
+    public convenience init?(model: AgentPaneModel, source: AgentPaneSource? = nil, renderRate: AgentPaneRenderRate = .capped,
+                             pageHost: Bool = false, shared: AgentPaneShared) {
+        self.init(model: model, source: source, renderRate: renderRate, pageHost: pageHost, openPanel: shared.openPanel,
</file context>

- Input: `com.cmuxterm.next.event-source-processes` (``EventSourceProcesses``: the executable path and start time of
each process that posts key and mouse events, read once per process with `proc_pidinfo`/`proc_pidpath`, user-initiated,
serial; it also runs that process's exit watch, which removes the entry. The key path only reads the table, never
a system call; one item per new posting process, so no back-pressure)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The one-item-per-process claim is false when exit-watch registration fails: each later event re-enqueues a read after read(_:) removes the entry. Document this retry path instead of claiming no back-pressure.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At plans/cmux-next/threading-model.md, line 152:

<comment>The one-item-per-process claim is false when exit-watch registration fails: each later event re-enqueues a read after `read(_:)` removes the entry. Document this retry path instead of claiming no back-pressure.</comment>

<file context>
@@ -146,6 +146,10 @@ matches the label's literal text; `<x>` matches an interpolation). Its owner row
+- Input: `com.cmuxterm.next.event-source-processes` (``EventSourceProcesses``: the executable path and start time of
+  each process that posts key and mouse events, read once per process with `proc_pidinfo`/`proc_pidpath`, user-initiated,
+  serial; it also runs that process's exit watch, which removes the entry. The key path only reads the table, never
+  a system call; one item per new posting process, so no back-pressure)
 - Diagnostics: `cmux-next.hang-episodes`, `cmux-next.hang-episodes.symbols`, `com.cmuxterm.app.next.run-marker`,
   `com.cmuxterm.app.next.crash-forwarder`, `com.cmuxterm.app.next.owner-panic-forwarder`,
</file context>
Suggested change
a system call; one item per new posting process, so no back-pressure)
a system call; normally one item per new posting process, but watch-registration failures retry on later events)

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Fix for the BLOCK at 072f126 (#28065 (comment)). Head: 62fc9e9.

Cause. EventSourceProcesses.startTime read proc_pidinfo(PROC_PIDTBSDINFO), which refuses another user's process (EPERM). The WindowServer, launchd and the root screensharingd therefore never counted as the person's. Each of their events also started a new read and a new kqueue.

Fix (62fc9e9).

  • The start time now comes from sysctl(KERN_PROC_PID) kp_proc.p_starttime. That call has no same-user check and is public on macOS 14 and later (<sys/sysctl.h>, kinfo_proc). The path read is still proc_pidpath.
  • I did not use PROC_PIDUNIQIDENTIFIERINFO. It reads these processes too, but it is not in the public SDK headers: grep of Xcode 26.6's MacOSX.sdk sys/proc_info.h and libproc.h finds no match. The pid plus start time, with the exit watch on the same serial queue, already names one process.
  • A live process that is still unreadable (the watch attached, but the read gave nil) becomes an unreadable entry. It keeps the safe default until its exit or exec fires the watch, so it is not read again for each event.

Fleet probe (read-only Python ctypes over SSH, uid 501, macOS 27, cmux17 and aws-m4pro-1, before the change):

launchd       pid=1    PROC_PIDTBSDINFO errno=1 (EPERM) | PROC_PIDUNIQIDENTIFIERINFO 56 bytes | sysctl KERN_PROC_PID 648 bytes, start 1792556088.401567 | kqueue EVFILT_PROC NOTE_EXIT|NOTE_EXEC receipt data 0 (attached)
WindowServer  pid=171  PROC_PIDTBSDINFO errno=1 (EPERM) | PROC_PIDUNIQIDENTIFIERINFO 56 bytes | sysctl KERN_PROC_PID 648 bytes, start 1792556097.996341 | kqueue attached
gone          pid=99999 sysctl KERN_PROC_PID size 0 (no process -> nil)

The exit watch (kernelExit) attaches to another user's process, so the unreadable entry ends with the process.

Tests (scripts/ci/package-test-lane.sh suite Packages/macOS/CmuxNext ..., fleet cmux-ci run):

  • Red at f1eed23 (--red, step d680c7f5f668b5417f8ad41c, aws-m4pro-2): theRealReadNamesProcessesOtherUsersOwn fails with real-read-proof: launchd pid 1 as uid 501: nil and the same for the WindowServer. aRefusedReadIsNotReadAgainUntilItsProcessEnds fails with reads 5 == 1. No crash.
  • Green at 62fc9e9 (step e6d9426903a01fddca42ac46, aws-m4pro-7): EventSourceProcessesTests 11/11, JoinConsentTests 41/41, SettingsWideningSwitchPersonGateTests 9/9. Its output:
    real-read-proof: launchd pid 1 as uid 501: start 1791478860980980 path /sbin/launchd
    real-read-proof: WindowServer pid 76853 as uid 501: start 1792334010233763 path /System/Library/PrivateFrameworks/SkyLight.framework/Versions/A/Resources/WindowServer
    exit-watch-proof: launchd pid 1 as uid 501: attached and exit-watch-proof: WindowServer pid 76853 as uid 501: attached
  • Compile-check at 62fc9e9 (recipe 2, --app, job 9a0e866de43d99ac101b1237): CmuxNext, app and cmux-tui pass.
  • prepush-gate.py: no new violations.

Not run: scripts/cmux-next/bench-typing.py, which the hook names for the perf-hot path(of:). On the key path the change adds only one enum case to the switch, and the reads stay off the main thread. Screen Sharing was not running on the test host (exit-watch-proof: screensharingd not running), so the root screensharingd read is shown only through launchd, which is also root.

aPidWithNoProcessIsNotKept became aProcessThatEndedBeforeItsReadIsNotKept: the kernel refuses a watch on a pid with no process, so the real case is a process that ends between its watch and its read, and there the watch fires. I corrected the PR body's "same verdicts" line.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-opus-review-subagent-b): FIX at 62fc9e9

I am an independent reviewer. I did not write this code. I read the full diff from the merge base 4f957ed to the head, the repair alone (git diff 072f126e312 HEAD), the PR body and all comments. File:line references are at 62fc9e9 unless named.

Finding to fix

1. The head does not compile when it lands on the current feat-cmux-next. The branch is 713 commits behind origin/feat-cmux-next (0e4ed40). #29626 (701a03f, 2026-10-31) added a new caller in the file chooser callback chosen:
origin/feat-cmux-next:Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPaneView+ContextMenu.swift:149: AgentPanePersonInput.record(model.transport.gestures).
This PR makes sources: a required argument of record (AgentPanePersonInput.swift:44, no default). git merge-tree --write-tree HEAD origin/feat-cmux-next merges with no text conflict (tree 59115644097), so GitHub says MERGEABLE, but the merged tree has this one call with no sources:. The squash merge would make feat-cmux-next fail to compile ("missing argument for parameter 'sources'"). The compile receipt 9a0e866de43d99ac101b1237 is for the head alone, not for the merge with the current feat.
Fix: merge origin/feat-cmux-next (scripts/merge-main.sh) and change that line to AgentPanePersonInput.record(model.transport.gestures, sources: eventSources). That keeps feat's behavior: on feat, record already records only when the event in dispatch is the person's (byPerson: true, event: NSApp.currentEvent). Then compile the merge head. In the merged tree I found no other caller of the changed APIs that is not updated (currentIsPerson, record, isPerson, PersonInputCheck(, PersonInputLedger(, AgentTabStore(, AgentPaneView(; Sources and Tests).

The repair itself: pass

  • The BLOCK at 072f126 is fixed. startTime (EventSourceProcesses.swift:177-187) now reads sysctl(KERN_PROC_PID) kp_proc.p_un.__p_starttime. That sysctl has no same-user check. proc_pidpath also has none. I read the fleet logs myself (cmux-ci log --full):
    • Red, step d680c7f5f668b5417f8ad41c at f1eed23: real-read-proof: launchd pid 1 as uid 501: nil, WindowServer pid 163 as uid 501: nil; aRefusedReadIsNotReadAgainUntilItsProcessEnds failed with fake.reads.count → 5 == 1. 11 tests, 6 issues, all in these two tests.
    • Green, step e6d9426903a01fddca42ac46 at 62fc9e9: launchd pid 1 ... start 1791478860980980 path /sbin/launchd, WindowServer pid 76853 ... path .../SkyLight.framework/Versions/A/Resources/WindowServer; EventSourceProcessesTests 11/11, JoinConsentTests 41/41, SettingsWideningSwitchPersonGateTests 9/9.
      So the two red tests fail without the fix and pass with it. The start values are plausible epoch microseconds, so the struct field is read at the right offset.
  • Struct and call. kinfo_proc is the public <sys/sysctl.h> struct, imported through Darwin, so Swift uses the SDK layout. The call passes the full stride and requires the returned size to equal it, and tv_sec > 0. A missing pid (size 0), a short or a failed answer gives nil. A bigger struct in a future kernel would give ENOMEM, which is nil (fail closed). UInt64(clamping:) cannot trap.
  • No main-thread block. startTime is private. Its one caller is system (:167). system runs only as resolve in read(_:) (:132), and only queue.async (:106) reaches read. The test calls system from Task.detached. The main thread only takes the Mutex for a dictionary operation.
  • unreadable always ends. It is written only after watchExit returned a cancel (:137-146), so an EVFILT_PROC NOTE_EXIT | NOTE_EXEC watch is attached to the pid. exited (:152-164) now matches .unreadable by its watch id as for .read, removes it and cancels the source (the fd closes). If the process ended during the read, the kqueue is already readable and the handler runs after this item on the same serial queue. If system gave nil because the start time changed, the watched process has ended, so its watch fires.
  • No false "yes". A posted event from a pid that is not read, .reading or .unreadable gives nil (:97-105), so it is not the person's. A script's pid is read with its own path, which is not in personServices or windowServer. Pid 0 and the DEBUG own-pid rule are as on feat. pid_t(exactly:) with source > 0 is stricter than feat's truncatingIfNeeded.
  • Feat's sources pass again. The WindowServer (strict check) and root processes are read (green log above). Universal Control and AssistiveControl run as the user, and were never affected. The root screensharingd is shown only through launchd (root); it did not run on the test host.
  • Pid reuse and exec. Pid plus p_starttime (stable across exec), with the watch armed before the read on the same serial queue and NOTE_EXEC in the filter. A reused pid after an exit is read again (aReusedPidIsReadAgain).
  • Rules. No try!, no new main-thread blocking call, // perf-hot: marks on the three key-path functions, the new queue is in threading-model.md, one table built in the composition root (AppServices.swift:162), no new singleton.

Notes (do not block)

  • plans/cmux-next/threading-model.md:150 still says the queue reads "with proc_pidinfo/proc_pidpath". It is now sysctl(KERN_PROC_PID)/proc_pidpath. Line 152 "one item per new posting process" is still not exact when the kernel refuses the watch (one item per event); that is now only a pid with no process.
  • An .unreadable entry lasts for the whole life of its process. A one-time sysctl or proc_pidpath failure on the WindowServer would refuse its posted HID events until logout. This fails closed and I know of no live case where those reads fail transiently, but a log line on the unreadable path would make it visible.
  • macOS 14 and 15 did not run the real read (the fleet Macs run 26 and 27). kinfo_proc and kern.proc.pid have kept the same ABI for many releases, and the size check fails closed.
  • The PR has the risk:normal label, and the body says "Treated as risk:high". The body's Review and Testing sections stop at 97fd679/ff83b4e1f5d; add the f1eed23 red and 62fc9e9 green steps and the 9a0e866de43d99ac101b1237 compile.
  • The time between an exit or exec and its handler on the queue is the same small TOCTOU window that earlier reviews accepted; a pid reuse inside it needs the pid counter to wrap.
  • The planned follow-up still applies: drop the default eventSources: of AgentPaneView.init (AgentPaneView.swift:172) and the default paneShared: of AgentTabStore.init (AgentTabs.swift:260).

Observed / Not run

Observed: the diffs; git merge-tree against origin/feat-cmux-next 0e4ed40 and a git grep of the merged tree; the full fleet logs of steps d680c7f5f668b5417f8ad41c (red) and e6d9426903a01fddca42ac46 (green); the state of compile 9a0e866de43d99ac101b1237 (done, head only).
Not run: no Swift build or test anywhere; no compile of the merge with the current feat; no new fleet probe (the green log already runs the real Swift read against launchd and the WindowServer); no Screen Sharing session.

…ource table (the merge of feat's #29626)

Feat's 701a03f added AgentPanePersonInput.record(model.transport.gestures) in the chosen
callback; this PR makes sources: required, so the merged code did not compile. The check is the
same as feat's, read from the injected table. threading-model.md names the sysctl read.
@cursor

cursor Bot commented Nov 1, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot 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.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-opus-review-subagent-c): SHIP at dc20356

I am an independent reviewer. I did not write this code. I read the full diff from the merge base 9f5e2e1 to the head (17 files), the merge 8a24c2b (git show --cc: no hand-resolved hunks), the repair dc20356, the PR body, the BLOCK at 072f126 (issuecomment-6549459596) and the FIX at 62fc9e9 (issuecomment-6556363410).

Findings

None that block. The FIX finding at 62fc9e9 is fixed.

What I checked

  • The FIX finding. AgentPaneView+ContextMenu.swift:149 is now AgentPanePersonInput.record(model.transport.gestures, sources: eventSources). eventSources is the pane's let (AgentPaneView.swift:75). The closure uses implicit self after guard let self, the same as model on the next lines, so it compiles as those do.
  • chosen behaves as on feat. On feat (701a03f), record used the defaults if: true, answer: false, event: NSApp.currentEvent, and checked isPerson(event). The new call uses the same defaults with the same event. For the person's HID input (pid 0) the verdict is the same. For a posted event the verdict now comes from the shared table, the same as every other record call in this PR. A process that has not been read yet gets the safe default, "not the person's", once. No test depends on chosen recording a gesture (AgentPaneAttachDroppedTests calls stash(..., kind: .choose) directly).
  • Every caller in the merged tree passes the table. git grep over Packages/ for AgentPanePersonInput.(record|isPerson|currentIsPerson), PersonInputCheck(, PersonInputLedger(, AgentPaneView( and AgentTabStore(:
    • Sources: 12 record/isPerson/currentIsPerson calls in CmuxNextAgentPane. All pass sources: eventSources (Commands:63, ContextMenu:123,124,133,149,287,293,300, Fork:11,20, Popup:40, TabRowTools:9, Transport:177,198).
    • The app wiring: AppServices.swift:165 builds the one table. It is used by JoinConsent+App.swift:97 and by AgentTabs+Wiring.swift:114 (AgentPaneShared(openPanel:eventSources:)). AgentTabs.swift:578 passes shared: paneShared.
    • No PersonInputLedger() or PersonInputCheck() call with no arguments is left. No AgentTabStore(..., openPanel:) call is left.
    • Tests compile through the defaults, or pass sources: explicitly (JoinConsentTests:794,1100, EventSourceProcessesTests).
  • Feat after the merge. origin/feat-cmux-next moved to e3e3507 (2 commits after 9f5e2e1). git merge-tree --write-tree HEAD origin/feat-cmux-next merges with no conflict (tree d6abbc1331a). In that tree, every call of the changed APIs still passes sources:. The new feat commits add no caller.
  • No feat behavior was lost. git diff 9f5e2e15c39 HEAD touches only the PR's 17 files. In AgentPaneView+ContextMenu.swift the only changes are the five added sources: arguments. Feat's stashSource, AttachmentSource.claimed and the .choose path are unchanged. AgentTabs.swift and AgentTabs+Wiring.swift keep feat's daemons:, updates:, peerApprover: and agentAvatars(openPanel:).
  • The authorization logic did not change after the last review. git diff 62fc9e9212e HEAD is empty for AgentPanePersonInput.swift, EventSourceProcesses.swift, AgentPaneShared.swift, JoinConsent+App.swift and EventSourceProcessesTests.swift. The properties verified at 62fc9e9 still apply:
    • No false "yes": a pid that is .reading, .unreadable or not in the table answers nil. A scripted poster resolves to its own path. An entry exists only while its EV_RECEIPT exit watch (with NOTE_EXIT | NOTE_EXEC) is attached.
    • The WindowServer and root processes are read: startTime uses sysctl(KERN_PROC_PID) and the path uses proc_pidpath, and neither has a same-user check. The green fleet step e6d9426903a01fddca42ac46 printed real reads of launchd and the WindowServer.
    • Feat added no other event-source pid read: git grep eventSourceUnixProcessID finds only AgentPanePersonInput.swift.
  • Rules.
    • threading-model.md:155 now names sysctl(KERN_PROC_PID)/proc_pidpath.
    • The main thread does not block. path(of:) takes the Mutex only for a dictionary operation, and the system calls run only on com.cmuxterm.next.event-source-processes.
    • No try!.
    • // perf-hot: marks are on the key-path functions.
    • There is one table, built in the composition root, and no singleton.
    • The repair adds no CI gate, job or test.

Notes (do not block)

  • risk:high evidence: CLAUDE.md requires a green compile of this exact head with this receipt. The fleet compile and tests at dc20356 are still running. Before ready-to-land, post their job ids and results, and update the PR body's Review and Testing sections. They still stop at 97fd679/ff83b4e1f5d, and do not name the f1eed23 red step or the 62fc9e9 green step.
  • The label is risk:normal, but the body says "Treated as risk:high". Make the label match the body, or say in the body why it does not.
  • threading-model.md:155-157, "one item per new posting process, so no back-pressure": this is not exact when the kernel refuses the watch (one item per event). After 62fc9e9 that applies only to a pid with no process. This is the same as the earlier note.
  • The planned follow-up still applies. AgentPaneView.init (eventSources: default, AgentPaneView.swift:174), AgentPaneShared.init and AgentTabStore.init (paneShared: default, AgentTabs.swift:265) still let a new production caller get a private table that nothing primes. That fails closed: posted input is refused, and HID input is not affected.
  • An .unreadable entry lasts for the whole life of its process, and nothing logs it. A log line on that path would make a transient read failure of the WindowServer visible. This is the same as the earlier note.

Not run

No Swift build or test (repo rule: nothing on this machine; the fleet jobs at this head are pending). I did not do the GUI test of the file chooser through Screen Sharing.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Fleet evidence at dc20356 (merge of feat plus the chosen call's sources:):

  • Compile-check (recipe 2, --app), job 8de3618440b0250dd1901119: CmuxNext, app and cmux-tui pass.
  • Tests, step 0d1446151801913d8afd33e3 (aws-m4pro-1): EventSourceProcessesTests, JoinConsentTests and SettingsWideningSwitchPersonGateTests pass.
  • prepush-gate.py: no new violations.
    Not run: scripts/cmux-next/bench-typing.py, which the hook names for the perf-hot isPerson and path(of:). The key path only reads the table (one extra switch case after 62fc9e9).

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 1, 2026
…merge2

# Conflicts:
#	Packages/macOS/CmuxNext/Sources/CmuxNextApp/AgentTabs+Wiring.swift
@lawrencecchen lawrencecchen removed the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 1, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-opus-review-subagent-d): SHIP at 4b705a7

I am an independent reviewer. I did not write this code. I read git show --cc HEAD, the PR delta git diff fd9dbf936f3 HEAD (17 files; fd9dbf9 is the merge base and the second parent), the PR body, the BLOCK at 072f126 (issuecomment-6549459596), the FIX at 62fc9e9 (issuecomment-6556363410) and the SHIP at dc20356 (issuecomment-6556402325). File:line references are at 4b705a7.

Findings

None that block.

What I checked

  • The hand-resolved conflict (AgentTabs+Wiring.swift:113-122). The result is feat's text at fd9dbf9 with one change: openPanel: services.openPanel becomes paneShared: AgentPaneShared(openPanel: services.openPanel, eventSources: services.personInput.sources). Feat's daemons:, updates:, peerApprover: and the full tabs.cloudRepositoryLocal closure (Cloud: add repository picker and recovery UI #30357: DaemonError.notConnected, cloud.repo.local, 30 s timeout, JSONEncoder) are byte-identical to feat. The imports already include CmuxNextAgentPane and CmuxNextDaemon.
  • No feat behavior was lost. For the 17 PR files, the +/- lines of git diff dc203569ae2 HEAD are identical to feat's own delta git diff 9f5e2e15c39 fd9dbf936f3 on the same files. So the merge applied feat's changes to these files and nothing else: AgentTabStore.cloudRepositoryLocal and its hand-off to each model (AgentTabs.swift, onCloudRepositoryLocal / onCloudRepositoryRemote), AppServices.agentLanes and daemon.installRecovery(), and the popup.hide → hideRequest(params) change in AgentPaneView+Popup.swift. None of them interacts with the PR's change. All other files equal feat's tip.
  • Every caller in the merged tree passes the shared table (git grep over Packages/):
    • AgentPanePersonInput.record/isPerson/currentIsPerson: 15 Sources calls, all with sources: eventSources (Commands:63, ContextMenu:123,124,133,149,287,293,300, Fork:11,20, Popup:40, TabRowTools:9, Transport:177,198).
    • PersonInputCheck(: AgentPanePersonInput.swift:128 and JoinConsent+App.swift:97, both over the app's table. PersonInputLedger(: only AppServices.swift:166 builds it. No call with no arguments is left.
    • AgentPaneView(: the one Sources caller is AgentTabs.swift:588 with shared: paneShared. AgentTabStore(: the one Sources caller is the resolved line above. No AgentTabStore(..., openPanel:) is left. Feat added no new init parameter to either type.
    • Tests compile through the defaults or pass sources: (JoinConsentTests:794,1100, EventSourceProcessesTests).
    • git grep eventSourceUnixProcessID finds only AgentPanePersonInput.swift (lines 26, 52, 90): feat added no other poster check.
    • origin/feat-cmux-next is still fd9dbf9. git merge-tree --write-tree HEAD origin/feat-cmux-next gives the same results.
  • Authorization properties. AgentPanePersonInput.swift, EventSourceProcesses.swift, AgentPaneShared.swift and EventSourceProcessesTests.swift are unchanged since dc20356 (feat does not touch them).
    • No false "yes": a pid that is not in the table, .reading or .unreadable gives nil (EventSourceProcesses.swift:96-108), so the event is not the person's. source > 0 and pid_t(exactly:) (AgentPanePersonInput.swift:31, :92) are stricter than feat's truncatingIfNeeded. An entry exists only while its EV_RECEIPT NOTE_EXIT | NOTE_EXEC watch is attached (:132-149, :204-220).
    • The WindowServer and root processes are read: startTime uses sysctl(KERN_PROC_PID) and the path uses proc_pidpath (:167-187). Neither has a same-user check (green fleet step e6d9426903a01fddca42ac46 printed real reads of launchd and the WindowServer as uid 501).
    • The strict check still needs the HID state ID and accepts only pid 0 or the WindowServer path (AgentPanePersonInput.swift:87-93).
  • Rules (CLAUDE.md, REVIEW.md, PATTERNS.md, REVIEW-STANDARD.md). The main thread makes no system call and takes the Mutex only for dictionary work. The system calls run on com.cmuxterm.next.event-source-processes, which is listed in threading-model.md:154. There is one table, built in the composition root and injected; no singleton. No try!. The five key-path functions have // perf-hot: marks. The merge adds no CI gate, job or test.

Notes (do not block)

  • Evidence: the fleet compile and tests at this head are pending (the PR checks also show "fleet test-compile plan" pending). Post their job ids before ready-to-land. The PR body's Review and Testing sections still stop at 97fd679/ff83b4e1f5d and do not name the 62fc9e9 or dc20356 steps.
  • The label is risk:normal, but the body says "Treated as risk:high". Make the label and the body agree.
  • plans/cmux-next/codebase-health.md section 6 (generated by codebase-health.py --write) does not list the five new key-dispatch markers. No CI check reads it; regenerate it in a later docs pass.
  • The known follow-up still applies: the defaults of AgentPaneView.init(eventSources:) (AgentPaneView.swift:174), AgentPaneShared.init and AgentTabStore.init(paneShared:) (AgentTabs.swift:269) let a new production caller get a private table that nothing primes. This fails closed (posted input refused; HID input not affected).
  • An .unreadable entry stays for the life of its process with no log line (as in earlier reviews). threading-model.md:157 "one item per new posting process" is not exact when the kernel refuses the watch.

Observed / Not run

Observed: the diffs and git show --cc above; the delta comparison of the 17 files; git grep over the merged tree and over git merge-tree with origin/feat-cmux-next; the PR checks list.
Not run: no Swift build or test (repo rule, and the fleet jobs at this head are pending); no Screen Sharing session.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Merge of feat-cmux-next fd9dbf9 (scripts/merge-main.sh), pushed as 4b705a7. One conflict, in AgentTabs+Wiring.swift: the result keeps this PR's paneShared: AgentPaneShared(openPanel:, eventSources: services.personInput.sources) and feat's cloudRepositoryLocal closure (#30357).

  • Compile-check (recipe 2, --app), job 16d4b09c7c17aa6d3d3bf618: CmuxNext, app and cmux-tui pass.
  • Tests, step 93871c2dc2969d1dcb4c07e5 (aws-m4pro-7): EventSourceProcessesTests, JoinConsentTests and SettingsWideningSwitchPersonGateTests pass.
  • prepush-gate.py: no new violations.

@lawrencecchen lawrencecchen added the ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds label Nov 1, 2026
@lawrencecchen
lawrencecchen merged commit ffed4d8 into feat-cmux-next Nov 1, 2026
211 of 226 checks passed
@lawrencecchen
lawrencecchen deleted the fix/stall-person-input-pidpath branch November 1, 2026 12:20
lawrencecchen added a commit that referenced this pull request Nov 1, 2026
Conflicts: #28065 changed the AgentTabStore init next to #30357's
cloudRepositoryLocal closure (kept #28065's init, dropped the closure);
#30831 edited a #30357 line in EnvironmentSection.tsx and added a test
of #30357's attached-repository rows to session-info-environment.test.tsx
(both files keep their pre-#30357 bytes; the test covers removed code).
teamleaderleo added a commit that referenced this pull request Nov 1, 2026
Conflict in AgentPaneView+ContextMenu.swift: base #28065 made the person check
take the pane's EventSourceProcesses table; the Chromium drop keeps its own event
choice (droppedEvent) and person gate, now through isPerson(event, sources:).

Agent-Session: https://cmux.com/deeplink/agent?id=aid-f71664ebfb6cf5206479

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 4b705a78 Deployed Nov 1, 2026 by lawrencecchen via wait for same-key tree publisher #96012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-land Reviewed at this head; next-autoland merges it into feat-cmux-next when its only reds are base reds risk:normal Risk tier (docs/ci/risk-tiers.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant