Skip to content

feat: add managed antenna descriptor admission - #3152

Draft
morozsm wants to merge 6 commits into
mainfrom
codex/antenna-policy-freeze
Draft

feat: add managed antenna descriptor admission#3152
morozsm wants to merge 6 commits into
mainfrom
codex/antenna-policy-freeze

Conversation

@morozsm

@morozsm morozsm commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Linear: https://linear.app/morozsm/issue/MOR-2306

Summary

  • add one ANTENNA_SWITCH descriptor policy for exact RX antenna operations and their public aliases
  • preserve the selector form set_rx_antenna {antenna, on} as a distinct descriptor that resolves Ant1/Ant2 without losing its public result shape
  • classify set_civ_output_ant as TX_SAFE
  • inject optional managed-TX admission once at the shared execution leaf and fail closed for invalid descriptor policies

Compatibility

  • direct unmanaged callers remain direct when no authority is injected
  • provider/profile support validation remains before execution
  • no provider, profile, Web, CLI, composition, or fake-wire changes
  • no hardware or TX validation

Verification

  • RED: focused run 33826813242 at abe6bc8751f00262afcbe363f3c9e416694303e1 — 21 failed, 129 passed as expected
  • Prior-head GREEN: focused run 33827912833 at 0a7b049b27766cd4f7dd2bcd34f65b105db73cf7 — successful, but stale after independent-review corrections
  • Review correction head: 79eb528db42356009bb6d811ab07769d9300dfa4 — discriminating RED tests added first; selector intent identity is stable through shared admission; Ruff check and format check passed across all five leased files
  • No new project test run or CI dispatch on the correction head while runner operations are unavailable, per coordinator instruction

Activation dependency

Do not merge or activate this PR alone. Registering these descriptors makes the generic consumer path recognize the operations, while the dependent consumer change must inject this same ManagedTxAuthority at the shared leaf. Integrate with the consumer wiring so no fail-open intermediate is deployed.

Fake-wire work and retirement of the superseded TX interlock remain separate follow-ups.

@morozsm

morozsm commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

Agent Review: BLOCKED

Exact head reviewed: 0a7b049b27766cd4f7dd2bcd34f65b105db73cf7 (tree e2d2981de5195e84e7a2f0144232f68318a097c9; 5 files, +339/-18).

  1. [P1] set_rx_antenna is not an Ant1 alias in the existing Web/API contract. In src/rigplane/core/command_dispatch.py:319-326, public_names=("set_rx_antenna", "set_rx_antenna_ant1") routes every set_rx_antenna request through the Ant1 descriptor. _bind_boolean ignores the existing antenna selector, so the current public request { "antenna": 2, "on": true } is canonicalized to set_rx_antenna_ant1 and can actuate the wrong antenna. The existing contract in ControlHandler._enqueue_rc_antenna accepts antenna 1 or 2 and preserves both antenna and on; the new test instead codifies the incorrect Ant1-only interpretation. Required fix: keep the selector command distinct or represent its selector faithfully, and add a discriminating end-to-end handler/drain test proving antenna=2 reaches Ant2.

  2. [P1] The descriptor executor violates the backend-neutral Radio signature and the tests mask it with unspecced mocks. All four antenna descriptors set argument_names=("enabled",) (command_dispatch.py:306,315,324,333), and execute_command_intent invokes the provider by keyword from argument_names (command_dispatch.py:543). But AntennaControlCapable and YaesuCatRadio define these methods as (..., on: bool). A conforming provider that supports an antenna operation therefore receives an unexpected enabled= keyword. The new AsyncMock tests assert that mismatched keyword instead of exercising an autospecced/concrete Radio signature. Required fix: make descriptor execution conform to the public Radio protocol (or add an explicit backend-neutral adapter) and cover it with a signature-enforcing test.

  3. [P2] The migration changes public command-result shapes without compatibility evidence. CommandDescriptor.result() returns argument_names, so these descriptors return { "enabled": ... }; the current handlers return { "on": ... }, and set_rx_antenna returns { "antenna": ..., "on": ... }. Existing tests such as tests/test_handlers_sys_band.py::test_set_civ_output_ant and tests/test_handlers_protocol_gap_399.py::test_set_rx_antenna_ant2 encode those public shapes. Required fix: preserve the established result contract or explicitly update the API contract and all consumers with compatibility acceptance.

The descriptor policy itself is otherwise coherent: exact Ant1/Ant2 operations resolve to ANTENNA_SWITCH, set_civ_output_ant resolves to TX_SAFE, invalid descriptor policies raise, admission is optional and called once at the shared leaf, and unmanaged/profile-rejected paths are structurally retained.

REQUIRED STACK ORDER / DO NOT MERGE ALONE: even after the code findings are fixed, this descriptor-registration commit must land atomically with the MOR-2308 consumer wiring that injects the same ManagedTxAuthority. Current drains execute CommandIntent before the legacy TX interlock; landing descriptor recognition without authority injection would create a fail-open intermediate for antenna writes.

Evidence observed: focused RED run 33826813242; focused GREEN run 33827912833 reported for this exact head; natural quick was queued when reviewed. Review verdict is code-only and does not wait for CI.

@morozsm
morozsm marked this pull request as draft September 4, 2026 02:24
@morozsm

morozsm commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

Agent Review: BLOCKED 6f26e61

P1 — selector intent stores a callable instead of the stable descriptor name.

At src/rigplane/core/command_dispatch.py:515, name=descriptor.method_name assigns _rx_antenna_method itself to CommandIntent.name for set_rx_antenna. execute_command_intent and ManagedTxAuthority admission then cannot resolve the descriptor by name, so antenna=2 cannot reach Ant2 or return the corrected ACK in production.

Required correction: bind descriptor.name into CommandIntent.name and leave dynamic provider method selection to resolve_method_name(intent.params). Then run the discriminating selector and Web ACK tests.

Confirmed: antenna=2 lookup resolves Ant2; public protocol remains on=; result shapes remain {on} and {antenna,on}; one shared descriptor/admission leaf remains; no second registry/resolver or raw fallback found.

CI state as reviewed: PR remains Draft; Agent Review Gate has no exact-head PASS; no exact-head project quick ran; quick-v2 observation failure is base/metadata state. Hard dependency MOR-2308 consumer wiring remains unresolved. DO NOT MERGE ALONE.

@morozsm

morozsm commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator Author

Agent Review: PASS 79eb528

Exact-head code review PASS. bind_command_intent now stores stable descriptor.name; dynamic selector method resolution remains exclusively in resolve_method_name(intent.params). The discriminating specced test binds antenna=2, preserves set_rx_antenna, crosses the single managed admission leaf exactly once, and invokes Ant2 with on=True. Existing Web queue/drain ACK remains {antenna, on}.

Scope verified: exactly five leased files, tree e851179c91f5e4b8ce98e30b96adca83614ca1a5. PR remains Draft. Required project CI is not claimed at this correction head. Hard dependency MOR-2308 consumer wiring remains; DO NOT MERGE ALONE.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant