Rectify: SkillResult.session_id Channel B Backfill & Prompt Contract Enforcement

[Trecek]

Summary

Three related bugs cluster around the same structural weakness: contracts that exist semantically but are not enforced structurally at the boundary where they matter.

run_headless_core computes effective_session_id as a Channel B fallback for SkillResult.session_id, correctly passes it to flush_session_log, but never writes it back to the returned SkillResult. Callers (including tools_github.py) receive session_id="" on all stale and empty-stdout timeout paths, even when Channel B discovered the real UUID from the JSONL filename. The fix moves resolution to construction time via a new _resolve_session_id helper and eliminates the post-hoc enrichment anti-pattern entirely.

A secondary fix corrects _prompts.py:74 which instructs Claude to call open_kitchen('{recipe_name}') in positional form — now enforced to keyword form open_kitchen(name=...). The sous-chef routing table is also extended to a 2D key (retry_reason, subtype) so stale sessions (watchdog kills) are re-executed rather than routed to on_context_limit.

Architecture Impact

State Lifecycle Diagram

%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 60, 'curve': 'basis'}}}%%
flowchart TB
    classDef cli fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;
    classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff;
    classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff;
    classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff;
    classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff;
    classDef gap fill:#ff6f00,stroke:#ffa726,stroke-width:2px,color:#000;
    classDef terminal fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;

    subgraph Sources ["INIT_ONLY SOURCE FIELDS"]
        direction LR
        ChannelA["ClaudeSessionResult<br/>━━━━━━━━━━<br/>.session_id (str)<br/>stdout-parsed UUID<br/>(Channel A — preferred)"]
        ChannelB["SubprocessResult<br/>━━━━━━━━━━<br/>.channel_b_session_id (str)<br/>JSONL filename stem<br/>(Channel B — fallback)"]
    end

    subgraph Gate ["★ RESOLUTION GATE — _resolve_session_id (NEW)"]
        direction TB
        Resolver["★ _resolve_session_id(session, result)<br/>━━━━━━━━━━<br/>if session and session.session_id:<br/>    return session.session_id<br/>return result.channel_b_session_id<br/>Single source of truth for priority rule"]
    end

    subgraph BuildSites ["● CONSTRUCTION SITES — _build_skill_result (MODIFIED)"]
        direction TB
        StaleSuccess["Stale recovery success path<br/>━━━━━━━━━━<br/>session_id=stale_session.session_id<br/>(stdout parse wins; no fallback needed)"]
        StaleFailure["● Stale failure path<br/>━━━━━━━━━━<br/>session_id=_resolve_session_id(None, result)<br/>NOW uses Channel B fallback"]
        TimeoutPath["● TIMED_OUT + empty-stdout path<br/>━━━━━━━━━━<br/>ClaudeSessionResult(session_id=<br/>    result.channel_b_session_id)<br/>Seeds session for normal constructor"]
        NormalPath["Normal path<br/>━━━━━━━━━━<br/>session_id=session.session_id<br/>(unchanged — already correct)"]
    end

    AntiPattern["effective_session_id [REMOVED]<br/>━━━━━━━━━━<br/>was: local var computed post-construction<br/>was: used for flush_session_log only<br/>was: NEVER written back to SkillResult"]

    subgraph Contract ["● INIT_ONLY CONTRACT — SkillResult.session_id (ENFORCED)"]
        direction TB
        SkillResultNode["● SkillResult<br/>━━━━━━━━━━<br/>.session_id: str  ← INIT_ONLY<br/>Set once at _build_skill_result<br/>Never mutated post-return<br/>Invariant: non-empty when Channel B has UUID"]
    end

    subgraph Consumers ["DOWNSTREAM CONSUMERS"]
        direction LR
        FlushLog["flush_session_log<br/>━━━━━━━━━━<br/>session_id=skill_result.session_id<br/>(previously: effective_session_id)"]
        ToolsGithub["tools_github.py<br/>━━━━━━━━━━<br/>propagated to caller<br/>& _read_session_diagnostics"]
        DebugLog["debug log<br/>━━━━━━━━━━<br/>session_id now accurate<br/>on all termination paths"]
    end

    ChannelA --> Resolver
    ChannelB --> Resolver
    Resolver --> StaleFailure
    Resolver --> TimeoutPath
    StaleSuccess --> SkillResultNode
    StaleFailure --> SkillResultNode
    TimeoutPath --> SkillResultNode
    NormalPath --> SkillResultNode
    AntiPattern -.->|"eliminated"| SkillResultNode
    SkillResultNode --> FlushLog
    SkillResultNode --> ToolsGithub
    SkillResultNode --> DebugLog

    class ChannelA,ChannelB cli;
    class Resolver newComponent;
    class StaleSuccess,NormalPath phase;
    class StaleFailure,TimeoutPath handler;
    class SkillResultNode stateNode;
    class FlushLog,ToolsGithub,DebugLog output;
    class AntiPattern gap;

Loading

Color Legend:

Color	Category	Description
Dark Blue	Source Fields	ClaudeSessionResult.session_id (Channel A) and SubprocessResult.channel_b_session_id (Channel B)
Green	New Component	★ _resolve_session_id — new resolution helper enforcing priority rule
Purple	Unchanged Path	Stale recovery success and normal paths (unmodified)
Orange	Modified Path	● Stale failure and ● TIMED_OUT paths now use Channel B fallback
Teal	Contract Field	● SkillResult.session_id — INIT_ONLY field with enforced contract
Dark Teal	Consumers	Downstream callers that now receive accurate session UUID
Yellow/Orange	Eliminated	effective_session_id local variable — anti-pattern removed

Process Flow Diagram

%%{init: {'flowchart': {'nodeSpacing': 40, 'rankSpacing': 50, 'curve': 'basis'}}}%%
flowchart TB
    classDef terminal fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;
    classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff;
    classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff;
    classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff;
    classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff;
    classDef gap fill:#ff6f00,stroke:#ffa726,stroke-width:2px,color:#000;

    START([run_skill invoked])

    subgraph Subprocess ["SUBPROCESS EXECUTION"]
        direction TB
        Launch["run_headless_core<br/>━━━━━━━━━━<br/>Launch Claude subprocess<br/>Channel A (stdout) + Channel B (JSONL)"]
        RaceResolve["resolve_termination<br/>━━━━━━━━━━<br/>RaceAccumulator signals<br/>→ TerminationReason"]
        TermDecision{"Termination?<br/>━━━━━━━━━━<br/>STALE / TIMED_OUT / NATURAL"}
    end

    subgraph BuildResult ["● _build_skill_result (MODIFIED — session_id backfill)"]
        direction TB
        StaleRecovery{"● STALE<br/>━━━━━━━━━━<br/>stdout has valid result?"}
        StaleSuccess["Recovered from stale<br/>━━━━━━━━━━<br/>success=True<br/>retry_reason=NONE<br/>subtype=recovered_from_stale"]
        StaleFailPath["● Stale failure<br/>━━━━━━━━━━<br/>success=False<br/>needs_retry=True<br/>retry_reason=RESUME<br/>subtype=stale<br/>session_id=_resolve_session_id(None, result)"]
        TimeoutBranch["● TIMED_OUT<br/>━━━━━━━━━━<br/>empty stdout? → bare ClaudeSessionResult(<br/>    session_id=result.channel_b_session_id)<br/>non-empty? → parse + force subtype=TIMEOUT"]
        NormalBranch["Normal completion<br/>━━━━━━━━━━<br/>parse_session_result(stdout)<br/>→ ClaudeSessionResult"]
        ComputeOutcome["_compute_outcome<br/>━━━━━━━━━━<br/>SUCCEEDED / RETRIABLE / FAILED<br/>(recovery steps applied first)"]
    end

    subgraph Gates ["POST-OUTCOME GATES"]
        direction TB
        PathGate{"path_contamination?"}
        ContractGate{"CONTRACT_RECOVERY?<br/>━━━━━━━━━━<br/>write evidence + adjudicated_failure"}
        ZeroWriteGate{"zero_writes?<br/>━━━━━━━━━━<br/>success=True but write_count=0"}
        BudgetGuard["Budget guard<br/>━━━━━━━━━━<br/>retries exhausted?<br/>→ flip needs_retry=False<br/>   retry_reason=BUDGET_EXHAUSTED"]
    end

    SkillResultOut["SkillResult<br/>━━━━━━━━━━<br/>(success, needs_retry,<br/>retry_reason, subtype,<br/>session_id — now accurate)"]

    subgraph SousChef ["● SOUS-CHEF ROUTING (MODIFIED — 2D key: retry_reason × subtype)"]
        direction TB
        NeedsRetryDecision{"needs_retry=True?"}
        RetryReasonDecision{"● retry_reason?<br/>━━━━━━━━━━<br/>2D routing key"}
        ResumeStale["● resume + stale<br/>━━━━━━━━━━<br/>Watchdog kill (NOT context limit)<br/>→ RE-EXECUTE step<br/>   (decrement retries)"]
        ResumeContextLimit["resume + !stale<br/>━━━━━━━━━━<br/>Context limit hit<br/>→ on_context_limit<br/>   (or on_failure if none)"]
        DrainRace["drain_race<br/>━━━━━━━━━━<br/>→ on_context_limit<br/>   (or on_failure if none)"]
        OtherReasons["empty_output / path_contamination<br/>/ early_stop / zero_writes<br/>━━━━━━━━━━<br/>→ on_failure (never context limit)"]
        RetryExhausted{"Retries exhausted?"}
    end

    COMPLETE([COMPLETE — success])
    CONTEXT_LIMIT([on_context_limit route])
    FAILURE([on_failure route])
    EXHAUSTED([on_exhausted route])

    START --> Launch
    Launch --> RaceResolve
    RaceResolve --> TermDecision
    TermDecision -->|"STALE"| StaleRecovery
    TermDecision -->|"TIMED_OUT"| TimeoutBranch
    TermDecision -->|"NATURAL_EXIT"| NormalBranch
    StaleRecovery -->|"recovery OK"| StaleSuccess
    StaleRecovery -->|"no recovery"| StaleFailPath
    StaleSuccess --> SkillResultOut
    StaleFailPath --> BudgetGuard
    TimeoutBranch --> ComputeOutcome
    NormalBranch --> ComputeOutcome
    ComputeOutcome --> PathGate
    PathGate -->|"yes"| BudgetGuard
    PathGate -->|"no"| ContractGate
    ContractGate -->|"yes"| BudgetGuard
    ContractGate -->|"no"| ZeroWriteGate
    ZeroWriteGate -->|"yes"| BudgetGuard
    ZeroWriteGate -->|"no"| SkillResultOut
    BudgetGuard --> SkillResultOut
    SkillResultOut --> NeedsRetryDecision
    NeedsRetryDecision -->|"no"| COMPLETE
    NeedsRetryDecision -->|"yes"| RetryReasonDecision
    RetryReasonDecision -->|"resume"| ResumeStale
    RetryReasonDecision -->|"resume + !stale"| ResumeContextLimit
    RetryReasonDecision -->|"drain_race"| DrainRace
    RetryReasonDecision -->|"other"| OtherReasons
    ResumeStale --> RetryExhausted
    RetryExhausted -->|"no — re-execute"| Launch
    RetryExhausted -->|"yes"| EXHAUSTED
    ResumeContextLimit --> CONTEXT_LIMIT
    DrainRace --> CONTEXT_LIMIT
    OtherReasons --> FAILURE

    class START,COMPLETE,CONTEXT_LIMIT,FAILURE,EXHAUSTED terminal;
    class Launch,NormalBranch,StaleSuccess phase;
    class StaleRecovery,TermDecision,NeedsRetryDecision,RetryReasonDecision,PathGate,ContractGate,ZeroWriteGate,RetryExhausted stateNode;
    class StaleFailPath,TimeoutBranch handler;
    class ComputeOutcome,BudgetGuard detector;
    class ResumeStale,ResumeContextLimit,DrainRace,OtherReasons newComponent;
    class SkillResultOut output;

Loading

Color Legend:

Color	Category	Description
Dark Blue	Terminal	Start, complete, context-limit, failure, and exhausted terminal states
Purple	Phase	Normal completion paths and recovery success
Teal	Decision	Routing decision points (termination, needs_retry, retry_reason)
Orange	Modified Path	● Stale failure and ● TIMED_OUT paths with session_id backfill
Red	Gate	_compute_outcome and Budget guard validation gates
Green	Routing Rule	Sous-chef dispatch rules (2D key: retry_reason × subtype)
Dark Teal	Output	SkillResult produced from _build_skill_result

Operational Diagram

%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 60, 'curve': 'basis'}}}%%
flowchart TB
    classDef cli fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;
    classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff;
    classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff;
    classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff;
    classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff;
    classDef gap fill:#ff6f00,stroke:#ffa726,stroke-width:2px,color:#000;

    subgraph CLI ["CLI ENTRY POINTS"]
        direction LR
        OrderCmd["autoskillit order<br/>━━━━━━━━━━<br/>[recipe] — recipe name or picker<br/>app.py:order command"]
        CookCmd["autoskillit cook<br/>━━━━━━━━━━<br/>Free-form interactive session<br/>(no _build_orchestrator_prompt)"]
    end

    subgraph OrderFlow ["ORDER COMMAND WORKFLOW"]
        direction TB
        RecipeResolve["Recipe resolution<br/>━━━━━━━━━━<br/>load_recipe + validate_recipe<br/>user picker if no recipe given"]
        SubsetGate["Subset gate<br/>━━━━━━━━━━<br/>show disabled subsets<br/>user opt-in prompt"]
        PreviewRender["show_cook_preview<br/>━━━━━━━━━━<br/>flow diagram + ingredients table<br/>rendered to terminal"]
        UserConfirm{"[Enter/n]<br/>━━━━━━━━━━<br/>launch confirm?"}
    end

    subgraph PromptBuild ["● PROMPT CONSTRUCTION (_build_orchestrator_prompt MODIFIED)"]
        direction TB
        PromptFn["● _build_orchestrator_prompt(recipe_name)<br/>━━━━━━━━━━<br/>Reads sous-chef/SKILL.md (bundled)<br/>Builds --append-system-prompt string"]
        OpenKitchenLine["● Mandatory first-action line<br/>━━━━━━━━━━<br/>open_kitchen(name='{recipe_name}')<br/>keyword arg form — FIXED<br/>(was: positional → Claude guessed param name)"]
        OrchestratorRules["Orchestrator rules injected<br/>━━━━━━━━━━<br/>tool restrictions (AskUserQuestion only)<br/>routing rules, failure predicates<br/>context-limit + stale retry routing"]
    end

    subgraph SessionLaunch ["SESSION LAUNCH"]
        direction TB
        LaunchCook["_launch_cook_session<br/>━━━━━━━━━━<br/>claude --plugin-dir {pkg_root}<br/>--tools AskUserQuestion<br/>--append-system-prompt {prompt}"]
        ClaudeSession["Claude Code interactive session<br/>━━━━━━━━━━<br/>MCP server: autoskillit<br/>Kitchen gated until open_kitchen(name=...) called"]
    end

    subgraph KitchenActivation ["MCP KITCHEN — open_kitchen tool"]
        direction TB
        OpenKitchenTool["open_kitchen(name=...)<br/>━━━━━━━━━━<br/>Reveals 40 kitchen MCP tools<br/>Loads recipe from recipes/ directory<br/>name= kwarg — unambiguous param"]
        PreviousGap["open_kitchen('{recipe_name}') [OLD]<br/>━━━━━━━━━━<br/>Positional arg → Claude infers param<br/>Sometimes guesses 'recipe_name' not 'name'<br/>→ tool call fails, wastes retry"]
    end

    subgraph Monitoring ["OBSERVABILITY"]
        direction TB
        SessionLogs["Session diagnostics logs<br/>━━━━━━━━━━<br/>~/.local/share/autoskillit/logs/<br/>sessions/{session_id}/"]
        TelemetryLogs[".autoskillit/temp/<br/>━━━━━━━━━━<br/>Token/timing summaries<br/>Audit logs, PR bodies"]
    end

    OrderCmd --> RecipeResolve
    RecipeResolve --> SubsetGate
    SubsetGate --> PreviewRender
    PreviewRender --> UserConfirm
    UserConfirm -->|"confirmed"| PromptFn
    UserConfirm -->|"abort"| ABORT([ABORT])
    PromptFn --> OpenKitchenLine
    PromptFn --> OrchestratorRules
    OpenKitchenLine --> LaunchCook
    OrchestratorRules --> LaunchCook
    LaunchCook --> ClaudeSession
    ClaudeSession --> OpenKitchenTool
    PreviousGap -.->|"eliminated"| OpenKitchenTool
    ClaudeSession --> SessionLogs
    ClaudeSession --> TelemetryLogs
    CookCmd -.->|"no prompt injection"| ClaudeSession

    class OrderCmd,CookCmd cli;
    class RecipeResolve,SubsetGate,PreviewRender phase;
    class UserConfirm stateNode;
    class PromptFn handler;
    class OpenKitchenLine newComponent;
    class OrchestratorRules phase;
    class LaunchCook handler;
    class ClaudeSession stateNode;
    class OpenKitchenTool output;
    class PreviousGap gap;
    class SessionLogs,TelemetryLogs output;
    class ABORT detector;

Loading

Color Legend:

Color	Category	Description
Dark Blue	CLI	autoskillit order and cook entry points
Purple	Phase	Recipe resolution, subset gating, preview render, orchestrator rules
Orange	Handler	_build_orchestrator_prompt and _launch_cook_session
Green	Modified	● Mandatory first-action line — keyword argument form fix
Teal	Decision/State	User confirm gate and Claude session state
Dark Teal	Output	open_kitchen tool and observability logs
Yellow/Orange	Eliminated	Old positional-argument form — ambiguous param guess
Red	Terminal	Abort path

Closes #527

Implementation Plan

Plan file: /home/talon/projects/autoskillit-runs/remediation-20260326-210914-908214/.autoskillit/temp/rectify/rectify_session_id_channel_b_backfill_2026-03-26_210914_part_a.md

🤖 Generated with Claude Code via AutoSkillit

[Trecek]

[warning] tests: test_make_result_exposes_channel_b_session_id tests the conftest _make_result helper rather than production code. It verifies test infrastructure, not the production SubprocessResult constructor contract. Remove or replace with a test that exercises production code directly.

[Trecek]

Investigated — this is intentional. The test calls _make_result() from tests/conftest.py (lines 123-140), which directly constructs the production SubprocessResult — it is not an opaque mock. Asserting result.channel_b_session_id == 'test-uuid-123' verifies the production dataclass field is correctly threaded through the conftest helper. If this wiring were broken, all four companion Channel B path tests in commit 6f283e0 would silently receive '' instead of the injected UUID, making their assertions meaningless. This is an intentional fixture-contract guard, not a test of test infrastructure.

[Trecek]

[warning] cohesion: Asymmetric session_id resolution — stale path (L516) calls _resolve_session_id(None, result) while the timeout path here calls result.channel_b_session_id directly, bypassing the helper. Both produce identical results when session=None, but _resolve_session_id is not the single authoritative resolver for all non-session paths. Use the helper consistently or inline both.

[Trecek]

Investigated — this is intentional. _resolve_session_id(None, result) trivially reduces to result.channel_b_session_id (the session!=None guard is skipped, line 436 returns directly). However, the two paths operate at different abstraction levels: the stale path (L510) builds a SkillResult and returns early — the helper applies Channel A > Channel B precedence at that final construction point. The timeout path (L534) builds a ClaudeSessionResult that flows through further processing including potential stdout recovery (L530) and synthesis recovery (L551); using the helper here would be cosmetically consistent but architecturally misleading, since the timeout path may subsequently override session_id downstream. The asymmetry is intentional design, not an accidental inconsistency.

[Trecek]

AutoSkillit review found 5 blocking issues. See inline comments. (Note: changes_requested verdict — GitHub prevents requesting changes on own PR, posting as comment.)