Skip to content

Implementation Plan: Strengthen Exception-Grade HIGH Findings (groupD)#559

Merged
Trecek merged 2 commits intointegrationfrom
test-audit/552/groupD
Mar 28, 2026
Merged

Implementation Plan: Strengthen Exception-Grade HIGH Findings (groupD)#559
Trecek merged 2 commits intointegrationfrom
test-audit/552/groupD

Conversation

@Trecek
Copy link
Copy Markdown
Collaborator

@Trecek Trecek commented Mar 28, 2026

Summary

Five test files receive targeted assertion strengthening to eliminate exception-grade HIGH
audit findings. All changes are in test code only — no production code is modified. The
work covers four distinct improvement types: structural __module__ assertions replacing
bare callable() checks (findings 1.7, 1.18), explicit no-raise contract docstrings on
assertion-free swallow tests (finding 1.9), a post-call state assertion on a no-file
scenario test (finding 1.12), and tmp_path fixture isolation for five load_config()
call sites that previously bypassed xdist-safe isolation (finding 1.23).

Architecture Impact

Module Dependency Diagram

%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 70, 'curve': 'basis'}}}%%
graph 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 terminal fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;

    subgraph L3 ["L3 — SERVER"]
        direction LR
        TOOLS_EXEC["server.tools_execution<br/>━━━━━━━━━━<br/>run_cmd, run_python, run_skill"]
        HELPERS["server.helpers<br/>━━━━━━━━━━<br/>_notify (error swallowing)"]
        TOOLS_KITCHEN["server.tools_kitchen<br/>━━━━━━━━━━<br/>_close_kitchen_handler"]
    end

    subgraph L1E ["L1 — EXECUTION"]
        direction LR
        PROCESS_FACADE["execution.process<br/>━━━━━━━━━━<br/>facade re-exports ≥21 symbols"]
        PROC_KILL["execution._process_kill<br/>━━━━━━━━━━<br/>kill_process_tree"]
        PROC_PTY["execution._process_pty<br/>━━━━━━━━━━<br/>pty_wrap_command"]
        PROC_JSONL["execution._process_jsonl<br/>━━━━━━━━━━<br/>_jsonl_contains_marker"]
        PROC_IO["execution._process_io<br/>━━━━━━━━━━<br/>create_temp_io"]
        PROC_MONITOR["execution._process_monitor<br/>━━━━━━━━━━<br/>_heartbeat, _session_log_monitor"]
        PROC_RACE["execution._process_race<br/>━━━━━━━━━━<br/>RaceAccumulator, resolve_termination"]
    end

    subgraph L1C ["L1 — CONFIG"]
        direction LR
        CONFIG["config.settings<br/>━━━━━━━━━━<br/>load_config, AutomationConfig"]
    end

    subgraph L0 ["L0 — CORE (zero autoskillit imports)"]
        direction LR
        TYPE_PROTOCOLS["core._type_protocols<br/>━━━━━━━━━━<br/>GatePolicy, HeadlessExecutor"]
        TYPES_HUB["core.types<br/>━━━━━━━━━━<br/>Re-export hub (fan-in: high)"]
    end

    subgraph Tests ["TESTS (modified ●)"]
        direction TB
        T_EXEC["● tests/server/test_tools_execution.py<br/>━━━━━━━━━━<br/>Finding 1.9: contract docstrings"]
        T_KITCHEN["● tests/server/test_tools_kitchen.py<br/>━━━━━━━━━━<br/>Finding 1.12: state assertion"]
        T_PROC["● tests/execution/test_process_submodules.py<br/>━━━━━━━━━━<br/>Finding 1.7: __module__ assertions"]
        T_TYPES["● tests/core/test_types_structure.py<br/>━━━━━━━━━━<br/>Finding 1.18: __module__ assertion"]
        T_CONFIG["● tests/config/test_config.py<br/>━━━━━━━━━━<br/>Finding 1.23: tmp_path isolation"]
    end

    T_EXEC -->|"imports _notify"| HELPERS
    T_EXEC -->|"imports run_skill"| TOOLS_EXEC
    T_KITCHEN -->|"imports _close_kitchen_handler"| TOOLS_KITCHEN
    T_PROC -->|"imports kill/pty/jsonl/io/monitor/race"| PROCESS_FACADE
    T_PROC -->|"direct sub-module imports"| PROC_KILL
    T_PROC -->|"direct sub-module imports"| PROC_PTY
    T_PROC -->|"direct sub-module imports"| PROC_JSONL
    T_PROC -->|"direct sub-module imports"| PROC_IO
    T_PROC -->|"direct sub-module imports"| PROC_MONITOR
    T_PROC -->|"direct sub-module imports"| PROC_RACE
    T_TYPES -->|"imports GatePolicy, HeadlessExecutor"| TYPE_PROTOCOLS
    T_TYPES -->|"imports hub symbols"| TYPES_HUB
    T_CONFIG -->|"imports load_config"| CONFIG

    PROCESS_FACADE -->|"re-exports"| PROC_KILL
    PROCESS_FACADE -->|"re-exports"| PROC_PTY
    PROCESS_FACADE -->|"re-exports"| PROC_JSONL
    PROCESS_FACADE -->|"re-exports"| PROC_IO
    PROCESS_FACADE -->|"re-exports"| PROC_MONITOR
    PROCESS_FACADE -->|"re-exports"| PROC_RACE
    TYPES_HUB -->|"re-exports"| TYPE_PROTOCOLS
    TOOLS_EXEC -->|"imports"| CONFIG
    TOOLS_EXEC -->|"imports"| TYPES_HUB

    class TOOLS_EXEC,HELPERS,TOOLS_KITCHEN handler;
    class PROCESS_FACADE,PROC_KILL,PROC_PTY,PROC_JSONL,PROC_IO,PROC_MONITOR,PROC_RACE phase;
    class CONFIG cli;
    class TYPE_PROTOCOLS,TYPES_HUB stateNode;
    class T_EXEC,T_KITCHEN,T_PROC,T_TYPES,T_CONFIG newComponent;
Loading

Color Legend:

Color Category Description
Green Tests (●) Modified test files — assertion-strengthened
Orange Server (L3) Server-layer tool handlers
Purple Execution (L1) Execution sub-modules and facade
Dark Blue Config (L1) Configuration layer
Teal Core (L0) Foundation types hub — high fan-in

Implementation Plan

Plan file: /home/talon/projects/autoskillit-runs/impl-groupD-20260328-084622-264528/.autoskillit/temp/make-plan/groupD_strengthen_exception_grade_HIGH_findings_plan_2026-03-28_084622.md

🤖 Generated with Claude Code via AutoSkillit

Token Usage Summary

Step input output cached count time
group 2.7k 25.0k 721.4k 1 9m 3s
plan 462 84.7k 14.8M 9 41m 2s
verify 98 55.7k 3.3M 6 18m 12s
implement 536 45.2k 10.3M 9 25m 25s
fix 60 10.0k 1.6M 2 11m 53s
Total 3.8k 220.7k 30.7M 1h 45m

- Finding 1.7: add __module__ assertions to all 6 process sub-module export tests
- Finding 1.9: replace implicit docstrings with explicit no-raise contract docstrings in 3 swallow tests
- Finding 1.12: add post-call gate-file state assertion to test_close_kitchen_no_file_no_error
- Finding 1.18: replace callable() checks with __module__ assertions in test_protocols_importable_from_sub_module
- Finding 1.23: add tmp_path isolation to 5 load_config call sites in test_config.py

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator Author

@Trecek Trecek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AutoSkillit PR Review — Verdict: changes_requested

@pytest.mark.anyio
async def test_notify_swallows_runtime_error_from_ctx(self):
"""RuntimeError from ctx.info (no active MCP session) is swallowed."""
"""Contract: must not raise even when ctx.info raises RuntimeError
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[critical] tests: Duplicate docstring line: the opening line of the multi-line docstring for test_notify_swallows_runtime_error_from_ctx appears twice in the diff. In the resulting Python file, the function docstring is truncated to the first line only; the second triple-quoted string literal is silently discarded as a standalone expression. Remove the duplicate docstring line so the full message is preserved in one contiguous triple-quoted string.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigated — this is intentional. Lines 782-784 of tests/server/test_tools_execution.py contain a single well-formed triple-quoted multi-line docstring: opening line 783 and continuation on line 784 within one triple-quoted block. Commit a276a0f shows a clean -1/+2 replacement where the old single-line docstring was expanded to two lines within one triple-quoted block. The unified diff hunk shows two '+' lines for one logical replacement — these are not two separate string literals. No duplicate, no truncation in the actual file.

Comment thread tests/core/test_types_structure.py
Copy link
Copy Markdown
Collaborator Author

@Trecek Trecek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AutoSkillit review found 2 blocking issues. See inline comments.

@Trecek Trecek added this pull request to the merge queue Mar 28, 2026
Merged via the queue into integration with commit 8aafa5c Mar 28, 2026
2 checks passed
@Trecek Trecek deleted the test-audit/552/groupD branch March 28, 2026 17:27
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