fix(auto): expose active task class in MCP meta#1196
Conversation
![ouroboros-agent[bot]](https://avatars.githubusercontent.com/in/3091302?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: APPROVE
Reviewing commit
9c26e12for PR #1196
Review record:
20d2a9cd-7ae6-40af-a4fd-1d9fddd3db9f
Blocking Findings
No in-scope blocking findings remained after policy filtering.
Non-blocking Suggestions
None.
Design Notes
Unable to assess architecture or changed behavior because the patch, comments, and source files were inaccessible through the available tools.
Policy Notes
- Omitted 1 finding(s) that referenced files outside the current PR changed-files scope.
Recovery Notes
First recoverable review artifact generated from codex analysis log.
Reviewed by ouroboros-agent[bot] via Codex deep analysis
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: APPROVE
PR #1196
Branch: fix/auto-active-task-class-meta | 2 files, +33/-0 | CI: Bridge TypeScript pass 15s https://github.com/Q00/ouroboros/actions/runs/26325192762/job/77501498596
Scope: architecture-level
HEAD checked: 9c26e12e5e548869fc19a18b3cbbbddca9d1d177
What Improved
- MCP
ouroboros_autometadata now includesactive_task_class, so clients no longer need to inspect local state files to observe the inferred L1 task class. - Regression tests cover both populated and ambiguous/unavailable task-class metadata, including preservation of explicit
null.
Issue #N/A Requirements
| Requirement | Status |
|---|---|
Surface AutoPipelineResult.active_task_class through MCP metadata |
Satisfied |
Preserve null when task-class inference is ambiguous, unavailable, or skipped |
Satisfied |
| Add regression coverage for populated and empty active-task-class metadata | Satisfied |
Prior Findings Status
| Prior Finding | Status |
|---|---|
| Prior review context | MODIFIED — No prior review concerns were available in the provided metadata or current checkout, so none were maintained, modified, or withdrawn. |
Blockers
| # | File:Line | Severity | Confidence | Finding |
|---|
Follow-ups
| # | File:Line | Priority | Confidence | Suggestion |
|---|---|---|---|---|
| — | — | — | — | None. |
Test Coverage
- Ran
SETUPTOOLS_SCM_PRETEND_VERSION=0.39.2 uv run pytest tests/unit/auto/test_surface.py tests/unit/auto/test_pipeline_task_class_envelope.py tests/unit/mcp/tools/test_start_auto.py -q: 122 passed. - Ran
SETUPTOOLS_SCM_PRETEND_VERSION=0.39.2 uv run ruff check src/ouroboros/mcp/tools/auto_handler.py tests/unit/auto/test_surface.py: passed. - Added coverage in
tests/unit/auto/test_surface.pyexercises_result_metadirectly and theAutoHandler.handleMCP result envelope.
Design / Roadmap Gate
Affected-boundary review covered the direct ouroboros_auto MCP response, the _result_meta contract, AutoPipelineResult.active_task_class production from pipeline state, and the background start_auto/ouroboros_job_result transport path through JobManager.result_meta. The added field is emitted at src/ouroboros/mcp/tools/auto_handler.py:1208, and None is preserved by the job metadata sanitizer at src/ouroboros/mcp/job_manager.py:72. No current-HEAD contract, persistence, replay, or consumer-boundary blocker was found.
Merge Recommendation
No current-HEAD blocker found. Despite the default review posture, this patch is narrowly scoped and verified across the relevant MCP metadata boundaries.
Review-Metadata:
verdict: APPROVE
github_event: COMMENT
review_kind: full
merge_eligible: false
head_sha: 9c26e12
source_read_ok: true
diff_read_ok: true
blocking_count: 0
shaun0927
left a comment
shaun0927
left a comment
There was a problem hiding this comment.
Cross-checked against #1188 (which added active_task_class to AutoPipelineResult + persistence) and #1157 (which names this PR as the remaining L1 MCP-metadata cleanup). The unconditional meta["active_task_class"] = result.active_task_class is exactly the right shape — PR body justifies it explicitly: clients need to distinguish "inference was ambiguous/unavailable" (null) from "protocol field absent" (key missing). Two tests cover populated + null. No risk.
Heads-up: #1205 silently writes the same field with if ... is not None, which would downgrade this contract back to "key omitted on null". I requested changes on #1205 to drop that hunk so this PR can own the surface. Recommend landing this one before #1205 rebases.
Drops the five hunks called out in PR review against #1157 SSOT / merged design decisions, while keeping every legitimate fix in this PR: - Restore DeferredProbe.passed=True (#1181 contract: probe-PASS placeholder so the L3 verifier flags the gap without failing the grade). - Restore `_inference.single` consumer call so unmatched ledgers still apply the safe LIBRARY fallback (#1177 + #1188 decision). - Drop the `if error is None` last_error_code clearing — sibling PR #1194 owns this hunk with the stricter `next_phase is not BLOCKED` condition. - Restore `env.setdefault(...)` for the three plugin runtime env vars introduced by #1193 so downstream entrypoints can still pre-seed. - Drop the conditional `active_task_class` MCP meta block — sibling PR #1196 owns it with unconditional null surfacing (clients need to tell ambiguous-inference apart from a missing protocol field). Kept fixes: `_matches_web_service` independence, `defaulted_sections` CLI rendering, `status --limit > 0` validation, mechanical-eval evidence linkage, TimeoutExpired stdout/stderr bytes-safe decode, workflow lifecycle same-timestamp restart ordering, MCP meta surfacing for `interview_closure_mode` + `runtime_probe_evidence`, plugin-mode Ralph `product_status=not_verified_complete` downgrade. Verification: - uv run pytest tests/unit/auto/test_pipeline_task_class_envelope.py tests/unit/auto/test_domain_inference.py tests/unit/auto/test_surface.py tests/unit/orchestrator/test_runtime_evidence.py tests/unit/cli/test_status_run.py tests/unit/cli/test_auto_command.py tests/unit/plugin/test_firewall.py tests/unit/orchestrator/test_workflow_lifecycle_events.py tests/integration/test_mechanical_eval_projection.py tests/unit/auto/test_stop_reason_code.py -q → 243 passed - uv run ruff check src tests → passed - uv run ruff format --check src tests → passed
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: APPROVE
Metadata
| Field | Value |
|### Recovery Notes
First recoverable review artifact generated from codex analysis log.
---|---|
| PR | #1196 |
| HEAD checked | 9c26e12e5e548869fc19a18b3cbbbddca9d1d177 |
| Request ID | req_1779668967_22 |
| Review record | 6f4f01ee-c525-442e-8f13-1b459a6bf849 |
What Improved
- Exposes
AutoPipelineResult.active_task_classthrough MCPMCPToolResult.metaso clients can consume the task-class inference already produced by the auto pipeline.
Issue Requirements
| Requirement | Status |
|---|---|
| No linked issue requirement captured | N/A |
Prior Findings Status
No prior human or inline review concerns were present in the provided comment artifacts.
Blockers
No in-scope blocking findings remained after policy filtering.
Follow-up Findings
| # | File:Line | Priority | Confidence | Suggestion |
|---|---|---|---|---|
| None. |
Non-blocking Suggestions
None.
Test Coverage Notes
- Reviewed changed tests in
tests/unit/auto/test_surface.py; they cover populated and emptyactive_task_classmetadata. - Could not execute tests:
pytest,uv, andpythonare not installed on PATH in this source snapshot environment.
Design Notes
The change is a narrow additive MCP metadata exposure of an existing pipeline result field. It matches the existing _result_meta pattern for always-present client progress fields.
Design / Roadmap Gate
Affected boundary is the auto pipeline result to MCP metadata contract. AutoPipelineResult.active_task_class is already optional, persisted on AutoPipelineState, and populated from state in AutoPipeline._result; exposing it as str | None in metadata does not create a persistence, replay, or compatibility failure. The added tests cover the null and non-null contract.
Directional Notes
Maintainer memory emphasized MCP handlers as thin envelopes and state/contract consistency. This PR aligns with that: state and pipeline already carry active_task_class; the handler now forwards it.
Test Coverage
- Reviewed changed tests in
tests/unit/auto/test_surface.py; they cover populated and emptyactive_task_classmetadata. - Could not execute tests:
pytest,uv, andpythonare not installed on PATH in this source snapshot environment.
Merge Recommendation
Approve. I found no blocking runtime, API contract, persistence, or test coverage issue in the changed boundary.
Review-Metadata:
verdict: APPROVE
head_sha: 9c26e12
request_id: req_1779668967_22
review_profile: memory-aware-zero-trust-v2
advisory_memory_only: true
Reviewed by ouroboros-agent[bot] via Codex deep analysis
2 participants
Summary
AutoPipelineResult.active_task_classthrough MCP_result_meta.nullwhen inference was ambiguous or unavailable, so clients can distinguish an empty computed value from a missing protocol field.Review context
This follows up the post-v0.39.1 L1 task-class sequence. PR #1188 added
active_task_classtoAutoPipelineResultand persisted state, but the public MCP metadata envelope omitted it. That meant MCP clients could not observe the L1-e result-envelope surface without reaching into local state files.Validation
SETUPTOOLS_SCM_PRETEND_VERSION=0.39.2 uv run pytest tests/unit/auto/test_surface.py tests/unit/auto/test_pipeline_task_class_envelope.py -q-> 91 passedSETUPTOOLS_SCM_PRETEND_VERSION=0.39.2 uv run ruff check src/ouroboros/mcp/tools/auto_handler.py tests/unit/auto/test_surface.py-> passedSETUPTOOLS_SCM_PRETEND_VERSION=0.39.2 uv run ruff format --check src/ouroboros/mcp/tools/auto_handler.py tests/unit/auto/test_surface.py-> passedgit diff --check-> passed