Skip to content

base red: arch-ratchet after #36133 - #36207

Merged
lawrencecchen merged 3 commits into
feat-cmux-nextfrom
fix/tip-red-flicker-log
Nov 5, 2026
Merged

lawrencecchen merged 3 commits into
feat-cmux-nextfrom
fix/tip-red-flicker-log

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Takes back #36133: moves its flicker log from a static to one instance owned by AgentTabStore.wired (the composition root) to fix the arch-ratchet stateful_singletons +2 tip red; the log's behavior and its off-main writes do not change.

Fixes the arch-ratchet tip red on feat-cmux-next after #36133 (culprit 27fa467): [stateful_singletons] +2 (CmuxNextAgentPane +2), from AgentPageFlickerLog.shared and its static var occlusion.

The log is now one instance that the composition root makes and passes in (plans/cmux-next/PATTERNS.md, composition root):

  • AgentTabStore.wired(to:) (built once per AppServices) makes it with AgentPageFlickerLog.forAppRun(). That is nil outside an app bundle, as shared was. The store passes it to every pane through AgentPaneShared.flickerLog.
  • AgentPaneView keeps it (flickerLog). Its bridge, its navigation delegate and AgentPageTerminations record through the pane's log. A pane with no log (tests) writes nothing.
  • The window occlusion watch is now state of the log instance. Each pane asks for it and the first one starts it. It takes its NotificationCenter so a test can post to its own.
  • No arch exception, and the recorded margin does not change.

Threading does not change. The caller's turn only builds the JSON line. The file create, append and roll-over run on the log's utility DispatchQueue, never on the main thread. flush() (queue.sync) is called only by tests.

Red on base, green on head

Command: python3 scripts/cmux-next/arch-ratchet.py --base bacb88f2349f --head b1b7a3ff9d02 (the tip range), exit 1:

arch-ratchet: [stateful_singletons] stateful singletons (stored static or file-scope global of a stateful type): +2 (CmuxNextAgentPane +2)
  Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPageFlickerLog.swift:35: static let shared: AgentPageFlickerLog? = {
  Packages/macOS/CmuxNext/Sources/CmuxNextAgentPane/AgentPageFlickerLog.swift:141: @MainActor private static var occlusion: MainActorObservation?

arch-ratchet.py --audit --rev e3a11f5de68 (current base): stateful_singletons 201 over the recorded 199.

Head: arch-ratchet.py --base origin/feat-cmux-next --head HEAD gives stateful_singletons -2 (CmuxNextAgentPane -2), no architecture count rises, exit 0. --audit --rev HEAD reports no stateful_singletons rise (199). prepush-gate.py: no new violations.

Regression test: AgentPageFlickerLogTests.everyPaneAsksForTheWindowWatchAndAFlipIsOneLine. Two panes ask for the watch and one occlusion flip writes one line. The test does not compile on base, because the watch was a static on shared, and shared is nil in tests.

Fleet compile: cmux-ci compile-check --app --package CmuxNext --ref c425e04a4f841067068c5e166a7d0b06f87924b8, job 1879bbb659bbbe4fd139e0c2: CmuxNext passed, app passed (also passed at 9640563, job 43da2f7b56759e66e5ca241d)
Fleet test: cmux-ci run --class light --script scripts/ci/package-test-lane.sh --arg suite --arg Packages/macOS/CmuxNext --arg AgentPageFlickerLogTests, ref c425e04, step 9915cf9d004fc8ef48a8ea61 on aws-m4pro-7: 4 tests in AgentPageFlickerLogTests passed, the new test included

Changelog

none

Observed / Not run

  • Observed: the arch-ratchet commands above (red on base, green on head); prepush-gate.py; the fleet compile and the fleet test run above.
  • Not run: no tagged app build to confirm that the ring file still fills in a real app run. Only the wiring moved: the lines and the file path are the same.
  • Out of scope: Agent pane: an always-on flicker log in a small ring file, matched to UTC time #36133 also added one observer_sites site (the occlusion watch). That row is part of older debt that no single commit raised (+15 at the tip), and the arch ratchet does not report it red. This PR keeps the count the same.

…wo statics

Fixes the arch-ratchet tip red after #36133: [stateful_singletons] +2
(CmuxNextAgentPane: AgentPageFlickerLog.shared and its static occlusion
watch).

The agent tab store's wiring (AgentTabStore.wired, built once per
AppServices) makes the app run's log with AgentPageFlickerLog.forAppRun()
(nil outside an app bundle, as `shared` was) and hands it to every pane
through AgentPaneShared.flickerLog. A pane keeps it (AgentPaneView.flickerLog)
and its bridge, navigation delegate and AgentPageTerminations record through
it. The window occlusion watch is the log's own instance state: each pane
asks for it, the first starts it. A pane with no log (tests) writes nothing.

Threading is unchanged: a caller's turn only builds the JSON line; the
file create, append and roll-over run on the log's utility queue, never
the main thread.

Test: AgentPageFlickerLogTests.everyPaneAsksForTheWindowWatchAndAFlipIsOneLine.

Red on base b1b7a3f: arch-ratchet.py --base bacb88f --head
b1b7a3f refuses stateful_singletons +2; --audit at b1b7a3f counts
201 over 199. Green on this commit.
@lawrencecchen lawrencecchen added the risk:low Risk tier (docs/ci/risk-tiers.md) label Nov 5, 2026
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

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

Review c425e04: 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=pr36207 -f ref=fix/tip-red-flicker-log -f platform=macos -f backend_mode=local

web and size checks: green

Shipped in:

The feat-cmux-next push run https://github.com/manaflow-ai/cmux/actions/runs/41992440601 at c293fc3 failed: cmux-next crash ends the app.
Those jobs last passed at 3deee62. This pull request is one of 3 merged since: #36285, #36207, #36251.
If the failure is in your change, fix forward on feat-cmux-next. Pull requests run only the tiers their paths reach (docs/ci/cmux-next-tiers.md); label a batch PR full-ci to run them all before merging.

@github-actions github-actions Bot added risk:normal Risk tier (docs/ci/risk-tiers.md) and removed risk:low Risk tier (docs/ci/risk-tiers.md) labels Nov 5, 2026
@blacksmith-sh

This comment has been minimized.

@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Review (claude-review-subagent): SHIP at c425e04

Independent correctness review of the exact head. The first pass at 9640563 found one HOLD issue: the new test's NSWindow was closed with close() while isReleasedWhenClosed was still true, so AppKit could over-release it. c425e04 fixes this with closeAfterTest(). No findings remain.

  • Recording sites: the bridge (diag.flicker), the navigation commit, web-content-ended (AgentPageTerminations) and the window occlusion watch all go through view.flickerLog. The app makes its only pane through AgentTabStore.wired (AppServices.agentTabs, lazy, one per app), so it makes one log for each run. forAppRun() is nil outside an app bundle, so tests and tools write nothing.
  • Lifetime: the watch starts once per log, and the observation captures [weak self], so there is no retain cycle.
  • Swift 6: the class is @MainActor, so it is implicitly Sendable. The members that tests and the queue touch are nonisolated and Sendable. The queue.async closure captures only [Data], URL and nonisolated statics. public is set where CmuxNextApp needs it.
  • Threading: the caller's turn only builds the line. Create, append and roll-over stay on the utility queue. Only tests call flush().
  • Composition root (PATTERNS P1): the store that AppServices makes builds the log once and passes it in through init. The PR adds no arch exception. arch-ratchet gives stateful_singletons -2, with no rises.
  • Evidence at this head: fleet compile job 1879bbb659bbbe4fd139e0c2 passed. Fleet test step 9915cf9d004fc8ef48a8ea61 passed AgentPageFlickerLogTests (4 tests).

@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 5, 2026
@lawrencecchen

Copy link
Copy Markdown
Contributor Author

Classification of the non-green checks on this head:

@cursor

cursor Bot commented Nov 5, 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.

@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 5, 2026 07:43 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 5, 2026 07:43 Inactive
@cmux-bot
cmux-bot Bot temporarily deployed to ci-routing-agent November 5, 2026 07:43 Inactive
@lawrencecchen
lawrencecchen merged commit cf6dec5 into feat-cmux-next Nov 5, 2026
167 of 170 checks passed
@lawrencecchen
lawrencecchen deleted the fix/tip-red-flicker-log branch November 5, 2026 07:55
lawrencecchen added a commit that referenced this pull request Nov 5, 2026
…wn occlusion observer) (#36502)

* base red: arch-ratchet observer_sites +1 from #36133 (flicker log's own occlusion observer)

The tip's arch ratchet (bacb88f..tip) refuses observer_sites +1 (CmuxNextAgentPane +1):
AgentPageFlickerLog.watchWindows registered its own MainActorObservation of
NSWindow.didChangeOcclusionStateNotification for every window (27fa467, #36133; #36207
moved it from a static to the log instance, and the site stayed).

The app already observes every window's occlusion once: SurfaceInvariantMonitor
.observeWindowOcclusion (AppServices wiring). That observer now hands each flip to the
agent flicker log (services.madeAgentTabs?.paneShared.flickerLog?.recordOcclusion(of:)),
the arch rule's right pattern (observe once per app, tell app subscribers directly). The
log keeps no observer: watchWindows() only turns window lines on, as the first pane did
before, and recordOcclusion(of:) writes the same window-occlusion line (window, visible,
type). madeAgentTabs never makes the agent tab store, so a flip before agent tabs exist
starts nothing. AgentTabStore.paneShared is no longer private, so the App can read it.

Arch ratchet on this commit over origin/feat-cmux-next: observer_sites -1
(CmuxNextAgentPane -1); app_lines +1 (the forwarding line), carried by a dated exception
in the next commit.

Test: AgentPageFlickerLogTests.aFlipIsOneLineOnceAPaneAsks replaces
everyPaneAsksForTheWindowWatchAndAFlipIsOneLine (watchWindows(center:) is gone): no line
before a pane asks, one line per flip after two panes ask.

* arch exception: app_lines +1 for the flicker log's occlusion forward (observer_sites -1)

The one App line is the forward from SurfaceInvariantMonitor, the app's one window
occlusion observer, to the agent flicker log. It replaces the log's own observer
(observer_sites -1). Reviewed: Review (claude review subagent): SHIP at
b113011. Expires 2026-11-19.

This branch was previously deployed

1 inactive deployment
ci-routing-agent — 2cbae0e9 Deployed Nov 5, 2026 by cmux-bot[bot] via wait for same-key tree publisher #115050
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