fix(linux): preserve actions in approval notifications#895
Conversation
ilysenko
left a comment
There was a problem hiding this comment.
Thanks for the well-isolated, fail-soft implementation. There is one lifecycle issue to address before merge: in bridge mode, close() sets closed = true before the bridge emits its closed event, so emitClose() suppresses the registered close callback. The wrapped Electron notification preserves that event. Please preserve the close-event contract and add a regression test covering programmatic close().
ilysenko
left a comment
There was a problem hiding this comment.
Thanks for fixing the original close-event suppression and adding the regression test. Two blockers remain:
closeRequestedis not respected byshow(),startFallback(), or the pre-shown failure paths. Closing while the bridge is starting can therefore show the Electron fallback afterward. Please prevent fallback/show after close is requested and add a close-before-shown regression test.- The Nix installer does not export the prebuilt
CODEX_NOTIFICATION_ACTIONS_SOURCEand has no Cargo runtime input, sonix run .#installersilently installs without the bridge. Please stage the prebuilt bridge in that path as well.
ilysenko
left a comment
There was a problem hiding this comment.
Reviewed the current head after resolving the notification lifecycle, close/fallback race, stdout-drain, and Nix installer staging issues. Regression coverage is in place and all required checks pass.
Summary
Closes #893.
notification interface
are unavailable
updater rebuilds, and Nix builds
The bridge does not execute commands or make approval decisions. It only
returns the selected notification action to the existing upstream handler.
This is default-on Linux compatibility behavior and does not add an opt-in
Linux feature.
Scope
This change affects approval notifications that already contain upstream
notification actions, such as command and file-change approvals produced by a
Custom approval policy configured through
config.toml.The built-in permissions
Ask for approvalflow does not currently emit adesktop notification and is outside the scope of this change.
Validation
cargo test -p codex-notification-actions-linuxcargo clippy -p codex-notification-actions-linux -- -D warningsnode --test scripts/patches/impl/main-process/notifications.test.jsnode --test scripts/patch-linux-window-ui.test.js— 366 passedbash tests/scripts_smoke.shnix flake check path:$PWD --no-buildnotification-actions-linuxcheckLinux