feat(client): implement external control plane v1 baseline for issue #135 - #151
Merged
Conversation
Scope: - add `--control-stdin` to headless `run/script` with explicit validation that rejects non-headless usage - introduce `client-control-stdin.v1` structured response frames multiplexed on stdout alongside the existing headless event envelope - implement the first control dispatcher baseline with `status:get` snapshots and canonical approvals action bridging - track active foreground tasks so `status:get` reports meaningful `active_task` data for both `run` and `script` - update client design/readme, Slice C tasks/evidence, and the Issue #135 master TODO to reflect implementation progress - add unit and integration coverage for flag boundaries, status snapshots, approvals bridging, unsupported actions, handler failure, and script session edge cases Rationale: Slice C can only proceed once hosts have a real write path into headless sessions. This commit lands the smallest useful `--control-stdin` baseline without changing the Slice B event envelope: hosts can now issue structured status and approvals requests over stdin and receive correlated structured results or errors on stdout. MCP control and the remaining host-surface actions stay pending for follow-up work in the same slice.
Scope: - mark task 3.2 complete in openspec/changes/client-external-control-plane-v1/tasks.md after the full unit/integration verification run and evidence update Rationale: The verification and evidence work for the current control-stdin baseline is now complete. Recording the task state separately keeps the implementation commit focused on runtime behavior while preserving an explicit governance checkpoint for the slice progress.
Finish the remaining host-control bridge work for issue #135 Slice C. Key changes: - extend the control-stdin action bridge to include canonical MCP actions mcp:list, mcp:reload, and mcp:show-tool alongside approvals, skills:list, and status:get - add integration coverage for skills:list and MCP control actions, including parameter passthrough for mcp:show-tool and continued structural rejection of mcp:unload - sync client DESIGN/README, the Slice C feature evidence doc, the master TODO ledger, and OpenSpec tasks with the landed control-plane baseline Rationale: The docs-only intent PR for Slice C was already merged, and the remaining gap in the implementation slice was the MCP and skills host-control surface. This commit closes that gap without widening the protocol beyond the current canonical resource:action set, leaving actions:list and startup capability discovery for Slice D. Verification: - ../../.venv/bin/python -m pytest tests/unit/test_client_cli.py -q - ../../.venv/bin/python -m pytest tests/integration/test_client_cli_flow.py -q - openspec validate client-external-control-plane-v1 --type change --strict --json --no-interactive - openspec list - ./scripts/ci/check_governance_evidence_truth.sh
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e826ab092
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address the PR #151 review finding about control-stdin exit hangs. Key changes: - replace the default-executor based control stdin read path with a daemon thread plus asyncio queue bridge so cancelling the control task no longer leaves a blocking stdin worker pinned in the loop executor - close the shared control stdin reader when the control loop exits to keep headless run/script shutdown deterministic - add a regression test that proves cancelling _read_control_stdin_line does not block default-executor shutdown - record the review fix and verification evidence in the Slice C feature doc Rationale: The previous implementation used asyncio.to_thread(sys.stdin.readline). Cancelling the awaiting task does not stop the underlying blocking readline, so asyncio.run could still hang while shutting down the default executor when stdin stayed open. The daemon-reader bridge keeps protocol behavior unchanged while removing that shutdown hazard. Verification: - ../../.venv/bin/python -m pytest tests/unit/test_client_cli.py -q -k 'cancellation_does_not_block_default_executor_shutdown' - ../../.venv/bin/python -m pytest tests/integration/test_client_cli_flow.py -q -k 'control_stdin' - ../../.venv/bin/python -m pytest tests/unit/test_client_cli.py -q - ../../.venv/bin/python -m pytest tests/integration/test_client_cli_flow.py -q - openspec validate client-external-control-plane-v1 --type change --strict --json --no-interactive - openspec list - ./scripts/ci/check_governance_evidence_truth.sh
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36a6c5c1ce
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Wire run-mode approval pending and resolved events into CLISessionState so status:get reports pending_approvals during headless control-stdin sessions. Key changes: - add a regression integration test that waits for approval_pending before issuing status:get and asserts the request id is surfaced in pending_approvals - route run-mode approval handlers through CLISessionState while preserving the existing approval timeout watch behavior - update Slice C feature evidence with the new review-fix command and thread link Rationale: The control plane already exposed pending_approvals in status snapshots, but the run --headless --control-stdin path only updated the timeout watch. That left hosts blind to blocked runs until timeout even though approval_pending had already surfaced on the event stream. This commit aligns the status snapshot with the existing host contract and reviewer feedback.
bouillipx
added a commit
that referenced
this pull request
Mar 2, 2026
Archive the merged client-external-control-plane-v1 change, sync its landed control-plane deltas into the canonical client-host-orchestration spec, and establish the docs-first kickoff baseline for client-capability-discovery-and-host-tests. Key changes: - archive Slice C OpenSpec artifacts under openspec/changes/archive/2026-03-02-client-external-control-plane-v1 and move its feature aggregation doc to docs/features/archive - sync the landed control-plane requirements into openspec/specs/client-host-orchestration/spec.md so the main spec reflects control-stdin, canonical action ids, and status snapshot semantics - update client/DESIGN.md and client/README.md to choose explicit actions:list discovery over unsolicited startup handshake for the next slice - refresh the host-orchestration master TODO ledger: mark CCLI-005/006 done, record Slice C archive evidence, and claim CCLI-007/008 for Slice D - create the client-capability-discovery-and-host-tests OpenSpec change plus its feature evidence doc, covering explicit capability discovery and host protocol regression testing Rationale: PR #151 is already merged, so Slice C must be archived before more implementation work begins. Slice D needs a docs-first baseline on top of the merged control-plane spec, otherwise the next implementation slice would start from an outdated canonical contract and leave hosts choosing between hardcoded action matrices and undefined handshake behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements Slice C of issue #135.
It adds the first structured external control plane for headless
client/sessions:--control-stdinfor headlessrun/scriptclient-control-stdin.v1Why
Slice B made the CLI observable through a versioned headless event envelope, but hosts still could not send structured control requests into a live run.
This slice closes that write-path gap without widening the protocol surface beyond the current canonical
resource:actioncontract.Implemented in this PR:
status:getapprovals:list/poll/grant/deny/revokemcp:list/reload/show-toolskills:listExplicitly not included in this PR:
actions:listmcp:unloadas a host protocol actionScope
Included:
Not included:
TODO / Change Mapping
CCLI-005CCLI-006client-external-control-plane-v1Validation
Executed:
../../.venv/bin/python -m pytest tests/unit/test_client_cli.py -q../../.venv/bin/python -m pytest tests/integration/test_client_cli_flow.py -qopenspec validate client-external-control-plane-v1 --type change --strict --json --no-interactiveopenspec list./scripts/ci/check_governance_evidence_truth.shResults:
44 passed)20 passed)0 issuesclient-external-control-plane-v1 ✓ CompleteReview Focus
Please focus review on:
control-stdinonly exposes the intended canonical action surfacemcp:unloadFollow-up
After this PR merges, the remaining host-orchestration work moves to Slice D:
actions:list