Send all seven RespondToPermission arguments from the daemon relay

[alexeyzimarev]

Closes #771 — AI-2498

What & why

The daemon relays a permission decided in the desktop app to the server through the RespondToPermission hub method, which has seven positional parameters. The relay sent five, so SignalR rejected every call before the hub body ran: the agent proceeded on the local answer while the web UI kept the permission card and "needs attention". The daemon now sends all seven, with nulls for the ACP option pair a hook decision never carries. The server-side half, a record-based method plus a wire test pinning the positional arity, is AI-2496.

Where to look

The new test hosts a real Kestrel SignalR hub declaring the server's parameter list. A ServerConnection double that overrides the invoke seam can never observe an argument-count mismatch, which is why the existing tests passed with the bug in place and why the daemon test project gains a Microsoft.AspNetCore.App framework reference.

Verification

• Without the fix the new test fails with InvalidDataException: Invocation provides 5 argument(s) but target expects 7., the rejection behind the production daemon log line Relaying the local decision … failed: Failed to invoke 'RespondToPermission' due to an error on the server.; with it, passes.
• Daemon unit suite, run twice: 2995 tests. Installed_codex_schema_matches_the_vendored_pin fails on a machine with Codex 0.153.0 against the 0.147.0 pin; Local_socket_stopv2_with_force… failed once under load and passes in isolation and on the re-run.
• scripts/check-linear-ids.sh exits 0; release AOT publish of the daemon reports zero IL2026/IL3050 warnings.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-04T15:29:13.714876Z	08d1b85	PR opened

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[qodo-code-review]

PR Summary by Qodo

Fix permission relay SignalR argument arity

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Sends all seven positional arguments required by the permission response hub method.
• Uses null ACP option values for local hook decisions.
• Adds a real SignalR wire test to prevent future contract drift.

Diagram

sequenceDiagram
    actor Desktop as Desktop App
    participant Daemon as Daemon Relay
    participant Client as SignalR Client
    participant Hub as Sessions Hub
    participant Web as Web UI
    Desktop->>Daemon: Local decision
    Daemon->>Client: Invoke seven arguments
    Client->>Hub: RespondToPermission
    Hub-->>Client: Applied outcome
    Hub-->>Web: Clear permission card

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use a request DTO

• ➕ Eliminates positional arity drift
• ➕ Makes future optional fields easier to evolve
• ➖ Requires a coordinated server contract and deployment change
• ➖ May break compatibility with existing server versions

2. Share a typed hub contract

• ➕ Provides compile-time method and parameter validation
• ➕ Reduces duplicated client/server contract definitions
• ➖ Couples daemon and server release artifacts
• ➖ Still requires coordinated contract evolution

Recommendation: Keep the seven-argument invocation and real wire-level regression test as the immediate, backward-compatible fix. A record-based request contract is preferable long term, but it requires coordinated server work and should not delay restoring the existing hub contract.

Files changed (3) +95 / -1

Bug fix (1) +4 / -1

ServerConnection.csSend the complete permission response argument list +4/-1

Send the complete permission response argument list

• Extends the RespondToPermission invocation from five to seven positional arguments. The two ACP option fields are sent as null because local hook decisions do not contain them, allowing SignalR to bind and execute the server method.

src/Capacitor.Cli.Daemon/Services/ServerConnection.cs

Tests (1) +86 / -0

ServerConnectionRespondToPermissionWireTests.csVerify permission relay against a real SignalR hub +86/-0

Verify permission relay against a real SignalR hub

• Adds an in-process Kestrel hub with the server's seven-parameter method signature. The test connects through ServerConnection and verifies that the relay reaches the hub successfully with the expected permission identity and behavior.

test/Capacitor.Cli.Daemon.Tests.Unit/Services/ServerConnectionRespondToPermissionWireTests.cs

Other (1) +5 / -0

Capacitor.Cli.Daemon.Tests.Unit.csprojEnable ASP.NET Core wire-test hosting +5/-0

Enable ASP.NET Core wire-test hosting

• Adds the Microsoft.AspNetCore.App framework reference so the daemon test project can host a real Kestrel SignalR endpoint and exercise protocol-level argument binding.

test/Capacitor.Cli.Daemon.Tests.Unit/Capacitor.Cli.Daemon.Tests.Unit.csproj

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. RespondToPermission breaks subject format ✗ Dismissed 📘 Rule violation ⚙ Maintainability

Description

The commit subject contains the non-initial uppercase word RespondToPermission, so it does not
match the required single-clause subject pattern. Rename it using lowercase prose, such as `Send all
seven permission response arguments (#771)`.

Code

test/Capacitor.Cli.Daemon.Tests.Unit/Services/ServerConnectionRespondToPermissionWireTests.cs[14]

+/// <c>RespondToPermission</c> hub method over a real SignalR hub. The protocol layer matches a

Relevance

●●● Strong

Active subject-format rule explicitly requires lowercase prose; replacing the identifier is a
trivial deterministic compliance fix.

PR-#659

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2897961 requires every word after the initial imperative verb to begin with a
lowercase letter. The supplied commit subject, `Send all seven RespondToPermission arguments from
the daemon relay (#771), includes RespondToPermission` as a subsequent uppercase word; the cited
changed line confirms the identifier whose literal spelling was used in the subject.

Rule 2897961: Enforce single-clause imperative commit subject with GitHub issue reference and 80-char limit
test/Capacitor.Cli.Daemon.Tests.Unit/Services/ServerConnectionRespondToPermissionWireTests.cs[14-14]

---

Context sources

✅ Compliance rules (platform): 56 rules

Review mode: ⚖️ Balanced: This changes a runtime SignalR wire contract and adds an integration-style test, creating genuine behavioral and compatibility risk, but the localized scope does not justify extended review.

Tip of the day

💡 Did you know, you can keep summaries lean with Finding overflow, which tucks the rest behind 'View more'

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗