Rectify: Stale-Hooks Check Infinite Prompt Loop

[Trecek]

Summary

Three compounding defects in the hook drift detection and stale-check prompt system create an inescapable infinite prompt loop after autoskillit install. The architectural weakness is twofold: (1) hook identity comparison uses full absolute paths rather than stable basename identifiers, creating environment-dependent fragility; (2) the interactive prompt flow lacks the verify-persist-return pattern that the binary-version path implements correctly. The fix introduces a basename-normalized hook identity model in L0 and a verified prompt action pattern in the stale-check flow, making the entire class of bugs structurally impossible.

Architecture Impact

Module Dependency Diagram

%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 70, 'curve': 'basis'}}}%%
graph TB
    %% CLASS DEFINITIONS %%
    classDef cli fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;
    classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff;
    classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff;
    classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff;
    classDef integration fill:#c62828,stroke:#ef9a9a,stroke-width:2px,color:#fff;
    classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff;

    subgraph Tests ["TESTS"]
        direction LR
        T_HR["★ test_hook_registry<br/>━━━━━━━━━━<br/>NEW — unit tests for<br/>hook identity model"]
        T_SC["● test_stale_check<br/>━━━━━━━━━━<br/>Stale-check + drift<br/>YES-path tests"]
        T_DR["● test_doctor<br/>━━━━━━━━━━<br/>Doctor hook-health<br/>check tests"]
    end

    subgraph L3_CLI ["L3 — CLI"]
        direction LR
        APP["cli/app.py<br/>━━━━━━━━━━<br/>CLI entry point<br/>dispatches commands"]
        HOOKS["● cli/_hooks.py<br/>━━━━━━━━━━<br/>Hook sync to<br/>settings.json"]
        STALE["● cli/_stale_check.py<br/>━━━━━━━━━━<br/>Version + drift<br/>check at startup"]
        DOCTOR["● cli/_doctor.py<br/>━━━━━━━━━━<br/>12 health checks<br/>including hook drift"]
        CLI_INIT["cli/__init__.py<br/>━━━━━━━━━━<br/>Re-exports:<br/>HookDriftResult"]
    end

    subgraph L3_SRV ["L3 — SERVER"]
        direction LR
        SRV_HELP["server/helpers.py<br/>━━━━━━━━━━<br/>Deferred hook<br/>diagnostic warning"]
    end

    subgraph L0 ["L0 — FOUNDATION"]
        direction LR
        HR["● hook_registry.py<br/>━━━━━━━━━━<br/>HookDef, HOOK_REGISTRY<br/>generate_hooks_json<br/>Fan-in: 6"]
        HOOKS_PKG["hooks/__init__.py<br/>━━━━━━━━━━<br/>Re-exports HOOK_REGISTRY<br/>HookDef, generate_hooks_json"]
        CORE["core/<br/>━━━━━━━━━━<br/>pkg_root, get_logger<br/>atomic_write, Severity<br/>Fan-in: high"]
    end

    %% === TEST → PRODUCTION DEPENDENCIES === %%
    T_HR -->|"imports 5 symbols"| HR
    T_SC -->|"imports HookDriftResult"| HR
    T_SC -->|"imports _stale_check module"| STALE
    T_SC -->|"imports cli namespace"| CLI_INIT
    T_DR -->|"imports cli namespace"| CLI_INIT

    %% === L3 CLI INTERNAL WIRING === %%
    APP -->|"deferred: run_stale_check()"| STALE
    APP -->|"deferred: run_doctor()"| DOCTOR
    CLI_INIT -->|"re-exports HookDriftResult"| HR
    CLI_INIT -->|"re-exports _check_hook_health"| DOCTOR

    %% === L3 CLI → L0 DEPENDENCIES === %%
    HOOKS -->|"_build_hook_entry,<br/>_claude_settings_path,<br/>_load_settings_data,<br/>_is_own_hook"| HR
    HOOKS -->|"HOOK_REGISTRY"| HOOKS_PKG
    HOOKS -->|"atomic_write, pkg_root"| CORE

    STALE -->|"deferred: _count_hook_registry_drift"| HR
    STALE -->|"deferred: _claude_settings_path"| HOOKS
    STALE -->|"get_logger, pkg_root"| CORE

    DOCTOR -->|"_count_hook_registry_drift,<br/>canonical_script_basenames,<br/>find_broken_hook_scripts"| HR
    DOCTOR -->|"_claude_settings_path,<br/>_load_settings_data"| HOOKS
    DOCTOR -->|"Severity, _ROOT_GITIGNORE_ENTRIES"| CORE

    %% === L3 SERVER → L0 === %%
    SRV_HELP -.->|"deferred: _claude_settings_path,<br/>_count_hook_registry_drift,<br/>find_broken_hook_scripts"| HR

    %% === L0 INTERNAL === %%
    HR -->|"pkg_root"| CORE
    HOOKS_PKG -->|"re-exports"| HR

    %% CLASS ASSIGNMENTS %%
    class APP,HOOKS,STALE,DOCTOR,CLI_INIT cli;
    class SRV_HELP phase;
    class HR stateNode;
    class HOOKS_PKG,CORE handler;
    class T_HR newComponent;
    class T_SC,T_DR output;

Loading

Closes #680

Implementation Plan

Plan file: /home/talon/projects/autoskillit-runs/remediation-20260408-183501-840011/.autoskillit/temp/rectify/rectify_stale-hooks-infinite-prompt_2026-04-08_190500.md

🤖 Generated with Claude Code via AutoSkillit

Token Usage Summary

Step	uncached	output	cache_read	cache_write	count	time
investigate	24	7.9k	507.9k	46.0k	1	6m 5s
rectify	11.3k	21.1k	948.9k	77.4k	1	9m 51s
dry_walkthrough	24	16.1k	1.5M	92.5k	1	5m 37s
implement	73	25.6k	5.5M	103.7k	1	10m 4s
prepare_pr	9	5.8k	137.1k	31.3k	1	1m 40s
run_arch_lenses	44	17.4k	1.1M	128.8k	3	7m 16s
compose_pr	9	4.0k	153.6k	22.1k	1	1m 5s
Total	11.5k	97.9k	9.8M	501.7k		41m 41s

[Trecek]

AutoSkillit PR Review — Verdict: changes_requested. 3 actionable findings (warning), 3 informational.

[Trecek]

[warning] defense: No subprocess returncode inspection before _verify_hooks_result. If autoskillit install exits non-zero (e.g. permission error, missing binary), _verify_hooks_result reads stale settings.json and may silently write a hooks_snoozed record instead of notifying the user that the install itself failed.

[Trecek]

Investigated — this is intentional. The design uses outcome-based verification rather than returncode inspection, mirroring the binary update path (lines 312-332). _verify_hooks_result (lines 217-239) re-reads actual drift from settings_path via _count_hook_registry_drift after the subprocess exits. If install exits non-zero and hooks remain unwritten, drift detection catches it (drift.missing > 0 or drift.orphaned > 0 → writes hooks_snoozed and returns False). The ground truth is the actual state of settings.json, not the process exit code — see commit e38dedc.

[Trecek]

[warning] defense: _is_own_hook uses a broad substring match ("autoskillit" in command) that could misclassify user hooks whose paths contain "autoskillit" (e.g. /home/autoskillit_user/my_guard.py). Pre-existing behavior (moved from _hooks.py) but now in a higher-visibility L0 location.

[Trecek]

[info] tests: Foreign path string hardcodes python3.13 version. Consider using a clearly synthetic placeholder (e.g. /foreign/install/hooks) to avoid accidental staleness.

[Trecek]

Valid observation — flagged for design decision. The test logic uses Path(cmd.split()[-1]).name (basename extraction) so the python3.13 version in the path has zero effect on pass/fail. The path is already synthetic (/home/user/ prefix). Whether to replace with a fully opaque placeholder is a style preference that warrants human input.