Steward hardening: canary truth, cancel-by-id, placement servability

[ttupper92618]

Motivation

The intelligent-fabric steward's first release (#829) deferred a set of verified review findings to #830. This PR lands the code items, hardening the two guarantees the steward's reliability story rests on: the fabric always knows when the steward is unhealthy, and the operator always stays in control of it.

Deliberately excluded: cluster-visible canary state (#830 item 4) — wire-adjacent, needs its own design pass.

Changes

• Canary: a stalled probe is a failed probe. canary_probe captures its deadline scope; a cancelled deadline returns failure even when partial text arrived first. Previously, a runner emitting some text and then wedging cleared the failure run, so the exact degraded state the canary exists to catch could never reach the three-failure teardown threshold.
• Cancel-by-id works on steward turns. The chat surface advertises one outer command id per turn while the harness's inner generation ids stay private; POST /v1/cancel/{outer_id} previously 404'd. Live turns now register their outer id (_register_steward_turn, entry lifetime bracketed by the stream itself), cancel_command routes it to a new StewardHarness.cancel_turn, which cancels the active inner generation and latches the investigation loop closed so the turn stops instead of dispatching its next step.
• Placement invariant reconciles preference edits. A placed steward whose model has been removed from steward_models is torn down; the same invariant re-places from the current list on the next tick. Reordering the list never replaces a working steward (upgrade churn is worse than a working older brain).
• Candidates are checked for servability. steward_candidate_is_servable (pure, tested) requires a text-generation card whose resolved capability profile supports tool calling — the same checks the bundled-defaults CI test applies — so an operator override naming an embedding/speech/tool-less card is skipped with a warning instead of placing a steward that fails every turn.
• vLLM tools-rejected message no longer mentions a "family default" parser that does not exist; it points only at runtime.vllm_tool_call_parser.

Why It Works

Each fix strengthens an existing invariant rather than adding machinery: the canary's success criterion becomes "completed within deadline" (code-checked, like the rest of the probe); the cancel registry lives exactly as long as its turn (registration/removal bracket the passthrough stream, so completion, disconnect, and cancellation all clean up through one finally); reconciliation reuses the invariant's own teardown + next-tick re-place cycle instead of introducing a second placement path; and the servability filter runs before place_instance, so the preference walk's fall-through behavior is preserved.

Test Plan

Manual Testing

• Not exercised on live hardware in this session. Recommended alongside the planned steward acceptance battery: cancel a streaming steward turn by its advertised id, edit steward_models under a live steward, and confirm degraded → teardown on a wedged runner.

Automated Testing

• New: partial-output-then-stall probe fails (never-finishing stream fixture, shrunk deadline); cancel_turn cancels the active inner generation and stops the loop from dispatching further steps; cancel_command routes a registered steward turn instead of 404; servability accepts a tool-calling text card and rejects speech/tool-less cards.
• Updated: the minimal cancel-test API fixture gains the turn registry.
• uv run ruff check clean; basedpyright and full pytest (3,148 passed) clean net of the known Linux-container baseline that reproduces identically on dev.

🤖 Generated with Claude Code

https://claude.ai/code/session_01BU73qqVrPobiDcwcDpzNt4

---

Generated by Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c1f0daa2fb

ℹ️ 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".

[Copilot]

Pull request overview

Hardens the intelligent-fabric steward reliability and operator-control guarantees by tightening canary probe semantics, making cancel-by-id work for steward turns, and preventing unservable steward candidates from being placed.

Changes:

• Treat canary probe timeouts (including “partial output then stall”) as probe failures, with a focused regression test.
• Register steward turn “outer” command IDs for the stream lifetime and route POST /v1/cancel/{command_id} to cancel the steward’s active inner generation and stop further investigation steps.
• Reconcile steward placement against steward_models edits and skip non-tool-capable / non-text candidates; clarify the vLLM tools-without-parser error message.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/skulk/worker/runner/vllm/runner.py	Clarifies the operator-facing error message when tools are requested but no vLLM tool-call parser is configured.
src/skulk/master/tests/test_steward_placement.py	Adds a unit test for steward candidate servability (text + tool-calling required).
src/skulk/master/main.py	Adds steward_candidate_is_servable, reconciles steward placement on preference-list edits, and filters unservable candidates.
src/skulk/api/tests/test_steward_chunk_stream.py	Adds tests ensuring steward cancel stops both the active generation and the investigation loop, and that cancel_command routes registered steward turns.
src/skulk/api/tests/test_steward_canary.py	Expands canary tests and adds a regression test for partial-output-then-stall counting as a failed probe.
src/skulk/api/tests/test_cancel_command.py	Updates the API cancel fixture to include the steward turn registry.
src/skulk/api/steward.py	Makes canary deadline cancellation a hard failure and adds cancel_turn() with a latch to prevent further step dispatch.
src/skulk/api/main.py	Adds a per-turn steward cancel registry and routes cancel-by-id to steward turns via _register_steward_turn + cancel_command handling.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a3d01d7977

ℹ️ 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".

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/skulk/master/main.py:272

• steward_candidate_is_servable() can return true even if card.placement.compatible_backends is empty. In that case the card cannot be placed/served at all, so the servability gate should reject it early (otherwise the invariant will attempt placement and log failures indefinitely).

    # Backend truth, not just model truth: a card whose only platform-
    # servable engine is vllm needs an explicit tool-call parser pin, or
    # the launched server rejects every tools-bearing request and the
    # steward would place but fail every turn.
    servable_engines = {

src/skulk/api/steward.py:982

• StewardHarness.cancel_turn() awaits send_task_cancellation() unshielded/unbounded. This can cause the HTTP cancel-by-id endpoint to hang or raise if the command send is blocked or the request is already in a cancelled scope. The surrounding code already treats cancellation as best-effort and timeboxes it; cancel_turn() should do the same.

        self._active_command_id = None
        await self._api.send_task_cancellation(active)

src/skulk/master/main.py:2060

• When replacing a steward because its model was removed from steward_models, the placement-attempt pacing can delay the re-place by up to 60s (if the steward was just placed). That contradicts the nearby comment/PR intent that it will re-place on the next tick, and can leave the cluster without a steward longer than necessary after an operator preference edit.

                    await self._teardown_steward_instances(stewards)

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/skulk/master/main.py:2113

• The warning message here is inaccurate for some failing candidates (e.g., a vLLM-only text+tools card without runtime.vllm_tool_call_parser also fails steward_candidate_is_servable, but is a tool-calling text card). This makes operator diagnostics misleading when steward placement falls through.

            if not steward_candidate_is_servable(card):
                logger.warning(
                    f"Steward model {model_ref} is not a tool-calling text "
                    "card; skipping"
                )

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c8e5daa5b4

ℹ️ 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".

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 135c23ff69

ℹ️ 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".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: be0e5a03ec

ℹ️ 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".

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/skulk/api/main.py:3734

• cancel_local_command() always sends TaskCancelled and adds the command_id to _cancelled_command_ids (suppressing TaskFinished). If the task is already terminal (e.g., last data-plane chunk dropped), this reproduces the failure mode described elsewhere in this file where sending TaskCancelled / suppressing TaskFinished can leak the master's task/command mapping. Consider checking _command_task_is_terminal(command_id) and, when true, just closing the sender and letting the normal TaskFinished path run.

        await self._send(TaskCancelled(cancelled_command_id=command_id))
        # Suppress the final TaskFinished emitted by local stream cleanup so the
        # worker can observe the Cancelled task and deliver runner-local cancel
        # before event-sourced task deletion happens.
        self._cancelled_command_ids.add(command_id)

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/skulk/master/main.py:2149

• The warning message for an unservable steward candidate is too specific: steward_candidate_is_servable() can return false for reasons other than “not a tool-calling text card” (e.g. vLLM-only candidates missing runtime.vllm_tool_call_parser). This would mislead operators when triaging why a candidate was skipped.

            if not steward_candidate_is_servable(card):
                logger.warning(
                    f"Steward model {model_ref} is not a tool-calling text "
                    "card; skipping"
                )

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/skulk/master/main.py:296

• steward_candidate_is_servable() can return True even when platform_compatible_backends(...) filters the card’s declared backends down to no known/servable engines (e.g. a vision card that only lists served backends, or a card with only unknown backend tags). That defeats the intent of this gate (it will repeatedly attempt placement and log “not possible yet” instead of skipping an unservable candidate) and can create noisy retry behavior under operator overrides.

    servable_engines = {
        engine
        for tag in platform_compatible_backends(
            card.placement.compatible_backends,
            card_serves_vision=card.vision is not None,