Repository navigation
Preserve query-on-exit for accepted network connections - #308
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The change is a small, self-contained correctness fix that matches GNU Emacs behavior, removes no dangling references, and is fully covered by the updated parity regression test.
Pull request overview
This PR fixes the query-on-exit behavior for network connections accepted by a listening server, aligning neomacs with GNU Emacs's server_accept_connection. Previously, an accepted client process inherited the listening server's query_on_exit_flag (e.g. a server created with :noquery t). In GNU Emacs, accepted clients instead keep make_process's default (query-on-exit-flag = t) and do not inherit the server's :noquery. This is part of addressing issue #305 (pinentry-emacs support).
Changes:
- Stop copying
query_on_exit_flagfrom the listening server to accepted client processes, so clients retain the default (true), with an explanatory comment referencing GNU behavior. - Update the corresponding parity regression test to assert the server keeps
:noquerywhile the accepted client reportsquery-on-exit-flag=t.
File summaries
| File | Description |
|---|---|
| neovm-core/src/emacs_core/system/process/mod.rs | Removes query_on_exit_flag from the server→client field copy in the accept loop and adds a comment explaining the GNU-matching default. |
| neovm-core/src/emacs_core/system/process/tests/mod.rs | Extends the local-stream server-accept test to verify server :noquery and the client's default query-on-exit flag. |
I verified that the accepted client now falls back to the constructor default query_on_exit_flag: true (mod.rs:5777), that the removed variable has no remaining references in the accept loop, and that the updated expected result string ("OK (listen t t t open t t 1 t t t t t)") matches the new 13-element result list. The change is small, self-contained, and covered by the updated parity test.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
eval-exec
left a comment
eval-exec
left a comment
There was a problem hiding this comment.
Confirmed against GNU Emacs process.c and reproduced red/green locally. Accepted clients must retain make_process query-on-exit defaults instead of inheriting the listener :noquery value. The implementation is correct and the public Lisp regression test covers the contract.
|
I will follow this merge on main with the longer-term accepted-process cleanup: replace the raw query-on-exit bool with a named policy enum, introduce an accepted-client creation spec that only exposes the fields GNU actually inherits, and add parity coverage for the surrounding inheritance/copy rules plus the pinentry handshake seam. GNU references: make_process and server_accept_connection. |
PR #308 fixed the immediate pinentry regression by preserving GNU make_process's query-on-exit default for accepted clients. The surrounding accept path still represented that policy as a bool and assembled a client by manually copying listener fields, making the same class of bug easy to reintroduce. Represent exit behavior as ExitQueryPolicy and make accepted connection creation consume an explicit AcceptedNetworkClientSpec. AcceptedNetworkClientInheritance is a deliberate GNU-derived allow-list: filter, sentinel, a shallow-copied plist, coding systems, conditional coding inheritance, and thread. Exit query policy, listener buffers, log callbacks, status, and live I/O cannot be inherited through that type. Match the adjacent GNU server_accept_connection rules as part of the refactor. A custom-filter client has no buffer; a default-filter client gets a distinct buffer named with the peer suffix; inherit-coding-system remains enabled only when that client buffer exists; and process plists are shallow copies instead of aliases. Add public Lisp regression coverage for Unix and TCP acceptance, independent plists, both buffer branches, conditional coding inheritance, and the historical GNU pinentry.el greeting handshake. This tests the package-level failure mode in addition to the raw query flag.
Closes #305