Publish stable status item identity before callbacks

[kiranmagic7]

Closes #3201.

CodexBar currently publishes a newly created status item through onCreated while AppKit still exposes its transient Item-0 autosave name. Bartender can persist that callback-time identity and treat each relaunch as a new item.

This assigns the existing codexbar-* autosave name before the callback runs. The regression test fails on the parent source with Item-0 and passes once the stable identity is published first.

Tests:

• CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --filter StatusItemControllerSplitLifecycleTests — 26 passed
• make check — passed
• CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 make test — not green locally because two AdaptiveRefreshTimerTests hit their 30-second scheduling timeout; both failures reproduce unchanged on the untouched upstream base
• Hosted CI — all 9 checks passed, including both macOS test shards

I could not run the closed-source Bartender relaunch check because Bartender is not installed here. Compatibility risk is limited to making the existing stable autosave name observable earlier; the useful review focus is whether any creation callback intentionally depends on AppKit's temporary Item-* name.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed August 26, 2026, 2:34 AM ET / 06:34 UTC.

ClawSweeper review

What this changes

This PR assigns CodexBar’s stable menu-bar autosave identity before invoking the status-item creation callback and adds a regression test for that ordering.

Regression provenance

Possible regression — suspected (reproduction). No predecessor PR is attributed.

Merge readiness

⛔ Blocked until real behavior proof from a real setup is added - 3 items remain

The patch is a focused ordering fix but still lacks real after-fix evidence that Bartender retains saved placement across relaunch, so it should remain open for proof.

Priority: P2
Reviewed head: 7dcbe03ba8c8701ddd3418aeb1fa620b05eca29f

Review scores

Measure	Result	What it means
Overall readiness	🧂 unranked krab (1/6)	The implementation is a focused, source-supported repair, but its claimed external compatibility result has only test and CI evidence.
Proof confidence	🧂 unranked krab (1/6)	Needs real behavior proof before merge: Focused tests and CI are useful supplemental validation, but real Bartender relaunch proof is required before merge; attach redacted before/after evidence and update the PR body, then request @clawsweeper re-review if needed.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: Focused tests and CI are useful supplemental validation, but real Bartender relaunch proof is required before merge; attach redacted before/after evidence and update the PR body, then request @clawsweeper re-review if needed.
Evidence reviewed	5 items	Patch implementation: The branch moves stable autosave-name assignment ahead of the creation callback and adds a test that verifies the callback sees codexbar-merged. Callback consumer: Provider vending uses the callback to register a newly created item; the normal merged-item creation and recovery call sites use the default nil callback. Current-main baseline and release: Current main invokes the callback before assigning the stable name; the branch is unmerged, and the repository’s current release tag is v0.55.1, so the proposed change is not shipped.
Findings	None	None.
Security	None	None.

Live Verification

Command: CODEXBAR_SUPPRESS_TEST_KEYCHAIN_ACCESS=1 swift test --filter StatusItemControllerSplitLifecycleTests

Result: FAIL (failed) — execution before step 1 run: sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.24.0.tgz

sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.24.0.tgz

Assertions:

• FAIL expect_output: status item publishes stable manager identity before creation callback

How this fits together

CodexBar creates macOS status items for its menu-bar interface, then runs creation callbacks to register provider items and continue setup. The autosave name is an identity that menu-bar managers can use to retain item placement across relaunches.

flowchart LR
A[App startup] --> B[Create status item]
B --> C[Assign stable identity]
C --> D[Creation callback]
D --> E[Register item and attach menus]
E --> F[Menu-bar manager placement]

Loading

Before merge

• Add real behavior proof - Needs real behavior proof before merge: Focused tests and CI are useful supplemental validation, but real Bartender relaunch proof is required before merge; attach redacted before/after evidence and update the PR body, then request @clawsweeper re-review if needed.
• Resolve merge risk (P1) - The focused test proves callback ordering, but it does not prove that Bartender observes the stable identity in time or reuses saved placement after a real application relaunch.
• Complete next step (P2) - The remaining merge gate is contributor-owned real behavior proof, not a concrete automated repair.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Patch scope	17 added, 1 changed, 2 files affected	The proposed behavior change is limited to the creation ordering and one focused regression test.

Merge-risk options

Maintainer options:

1. Prove manager compatibility before merge (recommended)
   Run the reported Bartender placement-and-relaunch scenario with the freshly built app and attach redacted evidence that the stored item is reused.

Technical review

Best possible solution:

Attach redacted before/after Bartender relaunch evidence from a freshly built bundle, showing the same saved item placement is reused after restart.

Do we have a high-confidence way to reproduce the issue?

No; the linked report provides a concrete Bartender relaunch path and source proves the ordering, but no after-fix Bartender run confirms the external persistence outcome.

Is this the best way to solve the issue?

Unclear; assigning the stable name first is a narrow maintainable repair, but a real manager relaunch is needed to show that it solves the identity churn.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against dfc176cdf945.

Labels

Label changes:

• add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
• remove rating: 🦪 silver shellfish: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

• P2: The reported relaunch behavior affects saved menu-bar placement but does not prevent the application from running.
• merge-risk: 🚨 compatibility: The patch changes which identity the existing creation callback can observe during status-item setup.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Focused tests and CI are useful supplemental validation, but real Bartender relaunch proof is required before merge; attach redacted before/after evidence and update the PR body, then request @clawsweeper re-review if needed.

Evidence

What I checked:

• Patch implementation: The branch moves stable autosave-name assignment ahead of the creation callback and adds a test that verifies the callback sees codexbar-merged. (Sources/CodexBar/StatusItemController.swift:330, 7dcbe03ba8c8)
• Callback consumer: Provider vending uses the callback to register a newly created item; the normal merged-item creation and recovery call sites use the default nil callback. (Sources/CodexBar/StatusItemController+StatusItemVending.swift:18, 7dcbe03ba8c8)
• Current-main baseline and release: Current main invokes the callback before assigning the stable name; the branch is unmerged, and the repository’s current release tag is v0.55.1, so the proposed change is not shipped. (Sources/CodexBar/StatusItemController.swift:330, dfc176cdf945)
• Feature history: History shows the provider-identity refactor and status-item autosave-name namespacing as prior work on this lifecycle surface. (Sources/CodexBar/StatusItemController.swift:53, 15a855a2132a)
• Contributor-supplied validation: The PR reports focused lifecycle tests, make check, and all hosted CI checks passing, but explicitly says the after-fix Bartender relaunch could not be run. (7dcbe03ba8c8)

Likely related people:

• Peter Steinberger: Authored the ProviderInstanceID refactor central to the current status-item identity surface. (role: recent area contributor; confidence: high; commits: 15a855a2132a; files: Sources/CodexBar/StatusItemController.swift, Sources/CodexBar/StatusItemController+StatusItemVending.swift)
• Koushik Venkatesan: History attributes the status-item autosave-name namespacing change to this contributor. (role: identity-behavior contributor; confidence: medium; commits: c687c8f2bfd6; files: Sources/CodexBar/StatusItemController.swift)
• ss251: History identifies status-item registration-before-callback work as adjacent lifecycle maintenance. (role: adjacent lifecycle contributor; confidence: medium; commits: a3f017f558cb; files: Sources/CodexBar/StatusItemController.swift, Sources/CodexBar/StatusItemController+StatusItemVending.swift)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Attach redacted Bartender before/after relaunch evidence from a freshly built bundle, showing saved placement persists.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)

• reviewed 2026-08-26T06:04:21.085Z sha 7dcbe03 :: needs real behavior proof before merge. :: none

[steipete]

Reviewed head 7dcbe03. This does not yet fix the production path reported in #3201.

Both real creation sites for the merged status item (startup and recovery in StatusItemController.swift) omit onCreated. Moving an optional callback across the autosaveName assignment therefore leaves the observable creation sequence unchanged for codexbar-merged. The new test supplies a merged-item callback that does not exist in either production path, so it proves the proposed helper ordering but not the reported Bartender identity behavior.

The actual callback is used by per-provider vending to register the item immediately after creation, before later setup can reenter vending. That ordering was intentional in a3f017f (#2162); please preserve that registration invariant while finding the real merged-item boundary.

Please revise this around a demonstrated production-path difference, with a regression that follows actual merged startup/recovery and a signed-app Bartender quit/relaunch check showing the same saved item reused. The passing CI is useful, but the current patch should not close #3201.