feat(plugin): close standalone surface gaps for v5.4.0 features

[JeremyDev87]

Summary

Closes the 4 plugin/standalone surface gaps identified in the v5.4.0 evaluation:

• Permission forecast: prompt-aware pattern analysis (SHIP/TEST/INSTALL/DELETE/REVIEW) mirroring MCP permission-forecast.ts, replacing static mode-only defaults
• PLAN template: staged planning flow (Discover→Design→Plan) replacing legacy "Output full plan in every response"
• Clarification budget: persist questionBudget in HUD state so standalone rounds decrement correctly
• Council scene eye glyph: load real agent eye fields from .ai-rules/agents/*.json, replacing ●‿● placeholders

Test plan

• permission_forecast.generate_standalone_forecast() with 6 prompt variants verified
• PLAN template assertions: staged flow present, old text removed
• HUD state budget round-trip: init(3) → decrement(2) → decrement(1)
• Council scene loads real glyphs: 4/5 unique faces for PLAN, 4/4 for EVAL
• pytest hooks/tests/ — 293 passed
• pytest hooks/lib/test_mode_engine.py — 63 passed
• Full plugin CI: lint, format, typecheck, coverage, circular, build — all passed
• Security audit: all 3 workspaces clean

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
codingbuddy-landing	Ready	Preview, Comment	Apr 8, 2026 5:49am

[JeremyDev87]

Quality Review — Purpose Alignment

Overall: GOOD
Logic: pass
Error Handling: pass
Design: warn
Maintainability: warn

---

Gap 1: Permission Forecast — Prompt-Aware Predictions

Verdict: CLOSED with minor gaps

The implementation correctly mirrors the MCP permission-forecast.ts logic. The five regex patterns (_SHIP_RE, _TEST_RE, _INSTALL_RE, _DELETE_RE, _REVIEW_RE) match their TypeScript counterparts faithfully, the bundle definitions align, and the generate_standalone_forecast function now accepts an optional prompt parameter that enriches the static mode defaults.

Both call sites in user-prompt-submit.py (MCP preview at line 87 and standalone at line 127) pass prompt=prompt, so the prompt-aware forecast activates in both backend paths.

Issues found:

• [MEDIUM] _DELETE_RE adds "destructive" class but no bundle (permission_forecast.py:182-183). When a user says "ACT delete the old migration", the forecast will show the raw class name destructive without a descriptive bundle like (Delete files). The MCP TS version has the same gap (it's parity-correct), but both sides leave the user with a bare class label. Not a regression, but an incomplete experience for the stated goal of "prompt-aware predictions."

• [MEDIUM] No unit tests for the new prompt parameter. The test file tests/test_permission_forecast.py has 22 tests but zero exercise the prompt= path (e.g., generate_standalone_forecast("ACT", prompt="ACT install react-query") to verify network class appears). The PR body claims "6 prompt variants verified" but these tests are not visible in the diff. If they exist elsewhere, they should be referenced.

---

Gap 2: PLAN Template — Staged Planning Flow

Verdict: FULLY CLOSED

The PLAN template in mode_engine.py (lines 65-84) now describes the Discover/Design/Plan staged flow. The old text ("Output full plan in every response") is completely removed. The checklist items align with the three stages. The rules section correctly says "Start at Discover" and "Advance stages only after user confirms direction."

No issues found. Clean replacement.

---

Gap 3: Clarification Budget — Persistence Across Rounds

Verdict: CLOSED with a design concern

The budget is added to _EXTENDED_DEFAULTS in hud_state.py (line 24) with default value 3. The user-prompt-submit.py standalone path reads it via read_hud_state(fill_defaults=True) (line 102), passes it to build_instructions (line 109), then re-evaluates clarification to determine whether to decrement (lines 116-123). The decremented value is persisted back via update_hud_state(questionBudget=new_budget).

This means: round 1 reads 3, fires clarification, persists 2. Round 2 reads 2, fires clarification, persists 1. Round 3 reads 1, fires clarification, persists 0. Round 4 reads 0, budget exhausted, proceeds to planning. The across-rounds persistence works correctly.

Issues found:

• [MEDIUM] init_hud_state() does not include questionBudget (hud_state.py:81-99). The init_hud_state function (called at session start) hardcodes its data dictionary without questionBudget. This means a fresh session has no questionBudget key in the file — the value only appears when fill_defaults=True is used or after the first update_hud_state call. This works because the reading code always uses fill_defaults=True, but it creates a schema inconsistency: every other extended field (phase, councilActive, councilStage, etc.) is present in both _EXTENDED_DEFAULTS AND init_hud_state, but questionBudget is only in _EXTENDED_DEFAULTS. If any future code reads HUD state without fill_defaults=True, it will silently miss the budget. Suggestion: add "questionBudget": 3 to the init_hud_state data dict for consistency.

• [LOW] Double evaluation of evaluate_clarification_standalone. The function is called once inside build_instructions (line 479 of mode_engine.py) and again in user-prompt-submit.py (line 117) purely to decide whether to decrement the budget. This is logically correct (same inputs produce the same deterministic output) but wasteful. The result from build_instructions could signal whether clarification was triggered (e.g., via a return-value wrapper or a flag) to avoid the redundant re-evaluation.

---

Gap 4: Council Scene — Real Agent Eye Glyphs

Verdict: FULLY CLOSED

The _load_agent_eye method (mode_engine.py:385-401) correctly reads visual.eye from agent JSON files. The slug normalization (lowercase, space/underscore to hyphen) matches the file naming convention used throughout the agent definitions. The _make_face method (mode_engine.py:403-408) composes {eye}‿{eye} with a clean fallback to ●‿● for agents that lack the visual.eye field.

I verified that all council preset agents have eye glyphs defined:

• technical-planner: ⎔
• architecture-specialist: ⬡
• test-strategy-specialist: ⊙
• code-quality-specialist: ●
• security-specialist: ◮
• code-reviewer: ⊛
• performance-specialist: ⊗
• accessibility-specialist: ✦
• auto-mode: ◊

All agents in COUNCIL_PRESETS will resolve to real glyphs, not the fallback. The fallback path exists only for agents that lack a visual block, which is the correct defensive behavior.

The test at line 440-444 was appropriately loosened — it no longer asserts a specific glyph character (which would break if the agent JSON changed) but still verifies the structural pattern of name [role].

---

Positive Observations

1. Faithful MCP parity. The regex patterns, bundle definitions, class ordering, and conditional logic in permission_forecast.py are a near line-for-line translation of the TypeScript permission-forecast.ts. This reduces drift risk.

2. Defensive error handling throughout. Every HUD state read/write is wrapped in try/except with silent fallthrough. The _load_agent_eye method handles missing files, missing keys, and JSON parse errors gracefully. No new exception paths can crash the hook.

3. Clean separation of concerns. The _make_face method is a focused single-purpose helper. The prompt-signal regexes are module-level compiled constants (not inline recompilation).

4. PLAN template is concise and actionable. The new staged flow checklist gives concrete stage-by-stage guidance without bloating the 2000-char hook limit.

---

Recommendations

1. Add questionBudget to init_hud_state data dict — Impact: Medium. Prevents schema drift between initialization and defaults.

2. Add unit tests for prompt-aware generate_standalone_forecast — Impact: Medium. At minimum: ("ACT", "ACT install react-query") should yield network, ("PLAN", "PLAN ship the feature") should yield external, ("ACT", "ACT delete old files") should yield destructive.

3. Add a _DELETE_BUNDLE alongside _SHIP_BUNDLE, _TEST_BUNDLE, _INSTALL_BUNDLE — Impact: Low. Gives users a descriptive label like destructive (Delete files) instead of bare destructive.

4. Eliminate double evaluate_clarification_standalone call — Impact: Low. Either have build_instructions return a flag, or have the hook check whether the returned instructions contain the clarification marker string.

[JeremyDev87]

Code Quality Review

Files Reviewed: 5
Total Issues: 6

By Severity

• CRITICAL: 0
• HIGH: 0
• MEDIUM: 3 (consider fixing)
• LOW: 3 (optional)

---

Stage 1: Spec Compliance

All four stated objectives are implemented and match the PR description:

1. Permission forecast prompt-aware analysis -- Regex patterns in permission_forecast.py mirror MCP permission-forecast.ts exactly (SHIP/TEST/INSTALL/DELETE/REVIEW). The generate_standalone_forecast() signature gains an optional prompt kwarg with backward-compatible default.
2. PLAN template staged flow -- Template updated from "Output full plan" to "Discover -> Design -> Plan" with matching checklist items.
3. Clarification budget persistence -- questionBudget added to _EXTENDED_DEFAULTS, read via fill_defaults=True, decremented on clarification, and written back to HUD state.
4. Council scene eye glyphs -- _load_agent_eye() and _make_face() load real glyphs from agent JSON; all 35 agents have visual.eye fields. Fallback to ●‿● is preserved.

Verdict: PASS -- Implementation covers all requirements.

---

Stage 2: Code Quality

[MEDIUM] DRY violation: _load_agent_eye duplicates _load_agent_details file resolution

File: packages/claude-code-plugin/hooks/lib/mode_engine.py:356-401
Issue: Both methods resolve the same agent JSON file with nearly identical code (path construction, os.path.isfile check, open/json.load, same exception tuple). The slug normalization in _load_agent_eye (.lower().replace(" ", "-").replace("_", "-")) adds an extra transform not present in _load_agent_details, creating a subtle inconsistency -- _load_agent_details uses the raw name directly.
Fix: Extract a shared _read_agent_json(self, agent_name: str) -> Optional[dict] that handles path resolution, slug normalization, file read, and error handling. Both methods become one-liners calling the shared reader:

def _load_agent_eye(self, agent_name):
    data = self._read_agent_json(agent_name)
    return data.get("visual", {}).get("eye") if data else None

[MEDIUM] N+1 file reads in build_council_scene

File: packages/claude-code-plugin/hooks/lib/mode_engine.py:433-441
Issue: For PLAN mode with 5 cast members, _make_face() opens and parses each agent JSON independently (5 file reads). Later, _build_rules_snippet calls _load_agent_details for the primary agent, reading its JSON a second time. Total: 6 file reads where 5 would suffice with a simple cache.
Fix: Cache the parsed JSON in a dict keyed by slug during build_council_scene, or pre-load all cast agent data in a single loop. Not a runtime concern (local disk, small files), but it's a code-cleanliness win that also fixes the DRY issue above.

[MEDIUM] _SHIP_RE and _REVIEW_RE overlap on pr\b / pull\s*request

File: packages/claude-code-plugin/hooks/lib/permission_forecast.py:17-31
Issue: Both patterns match pr and pull request. In EVAL mode, a prompt like "review the PR" triggers both SHIP_BUNDLE ("Ship changes") and REVIEW_BUNDLE ("Review PR"), which is semantically incorrect -- reviewing a PR is not shipping. This overlap exists identically in the MCP TypeScript source, so this is faithful mirroring, but it remains a latent bug in both implementations.
Fix (for both TS and Python): Either exclude SHIP signals in EVAL mode, or add a priority/exclusion rule: if REVIEW matches in EVAL mode, skip SHIP. Example:

is_review_mode = _REVIEW_RE.search(prompt) and mode_upper == "EVAL"
if _SHIP_RE.search(prompt) and not is_review_mode:
    ...

[LOW] Missing prompt-aware forecast unit tests

File: packages/claude-code-plugin/tests/test_permission_forecast.py:146-173
Issue: The PR body claims "6 prompt variants verified" but TestGenerateStandaloneForecast has zero tests that pass a prompt argument. All existing tests only exercise mode-based forecasting. The new prompt-signal code paths (SHIP/TEST/INSTALL/DELETE/REVIEW pattern matching, bundle appending, class sorting, ACT fallback) have no automated regression coverage in this file.
Fix: Add test cases such as test_ship_prompt_adds_external, test_install_prompt_adds_network, test_delete_prompt_adds_destructive, test_act_fallback_when_no_prompt_signals, test_eval_review_prompt.

[LOW] init_hud_state omits questionBudget from initial data

File: packages/claude-code-plugin/hooks/lib/hud_state.py:81-99
Issue: questionBudget is defined in _EXTENDED_DEFAULTS (line 24) but NOT in the init_hud_state data dict (lines 81-99). The field only materializes via read_hud_state(fill_defaults=True). This asymmetry means a raw read without fill_defaults returns no budget, and a future developer adding a new field might forget to add it in both places.
Fix: Add "questionBudget": DEFAULT_QUESTION_BUDGET (or the literal 3) to init_hud_state's data dict, consistent with the council fields that appear in both locations.

[LOW] Long line in generate_standalone_forecast

File: packages/claude-code-plugin/hooks/lib/permission_forecast.py:193
Issue: Line is 95 characters: sorted_classes = sorted(classes, key=lambda c: _CLASS_ORDER.index(c) if c in _CLASS_ORDER else 99). While not a hard violation, the lambda with inline ternary reduces readability.
Fix: Extract a helper or use _CLASS_ORDER.index(c) if c in _CLASS_ORDER else len(_CLASS_ORDER) on a preceding line:

def _class_sort_key(c: str) -> int:
    try:
        return _CLASS_ORDER.index(c)
    except ValueError:
        return len(_CLASS_ORDER)

sorted_classes = sorted(classes, key=_class_sort_key)

---

Positive Observations

• Regex patterns are an exact mirror of the MCP TypeScript -- good parity discipline.
• Error handling is consistent: all new file I/O uses the established (OSError, json.JSONDecodeError, ValueError) exception tuple with graceful fallbacks.
• The _make_face fallback to ●‿● ensures no breakage if agent JSON lacks a visual.eye field.
• The PLAN template rewrite is clean and well-structured with clear stage progression.
• The clarification budget read/write flow correctly uses fill_defaults=True to handle the initial-state gap.
• Hook-level except Exception: pass blocks follow the established defensive pattern for hooks that must never crash.

---

Recommendation

APPROVE -- No critical or high severity issues. The three MEDIUM findings are design improvements (DRY, N+1 reads, pattern overlap) that do not affect correctness. The missing prompt-aware tests (LOW) are the most actionable item for follow-up.

[JeremyDev87]

◮‿◮ Security Review

Scope: 5 files in packages/claude-code-plugin/hooks/ — permission_forecast.py, mode_engine.py, user-prompt-submit.py, hud_state.py, test_mode_engine.py
Risk Level: LOW

Summary

• Critical Issues: 0
• High Issues: 0
• Medium Issues: 0
• Low Issues: 1 (defense-in-depth observation)

No exploitable vulnerabilities found. The changes are well-structured with appropriate error handling and safe input patterns.

---

OWASP Top 10 Evaluation

Category	Status	Notes
A01 Broken Access Control	PASS	Agent JSON file loading uses hardcoded names from COUNCIL_PRESETS dict, not user input. os.path.isfile() guard present before every open().
A02 Cryptographic Failures	N/A	No cryptographic operations in scope.
A03 Injection	PASS	User prompt is only matched via re.search() against pre-compiled patterns — never interpolated into commands, queries, or output templates. template.format() uses values from hardcoded DEFAULT_AGENTS dict, not user input. update_hud_state(questionBudget=new_budget) uses a hardcoded key with an integer value.
A04 Insecure Design	N/A	No security-sensitive design decisions in scope.
A05 Security Misconfiguration	PASS	HUD state directory created with mode=0o700. State file path under ~/.codingbuddy/.
A06 Vulnerable Components	SKIPPED	Could not run dependency audit (yarn audit / npm audit) due to tooling constraints. PR body claims all 3 workspaces clean.
A07 Authentication Failures	N/A	No authentication logic in scope.
A08 Data Integrity	PASS	JSON files loaded from local .ai-rules/agents/ with json.load() wrapped in try/except (OSError, JSONDecodeError, ValueError). HUD state uses fcntl.flock() for atomic read-modify-write.
A09 Logging Failures	PASS	Error output goes to local stderr only (print(..., file=sys.stderr)), not to remote endpoints. Silent except Exception: pass blocks are a pre-existing pattern, not introduced by this PR.
A10 SSRF	N/A	No outbound HTTP requests in scope.

---

Low Issues (Defense-in-Depth)

1. Agent name slug does not strip path separators

Severity: LOW
Category: A01 — Broken Access Control (Path Traversal)
Location: hooks/lib/mode_engine.py:392
Exploitability: Not exploitable in current code — all callers pass hardcoded agent names from COUNCIL_PRESETS dict
Blast Radius: If a future caller passed user-controlled input, arbitrary JSON files could be read from outside agents/ directory

Issue: _load_agent_eye() constructs the file path via agent_name.lower().replace(" ", "-").replace("_", "-"), which does not strip / or .. path components. A name like ../../etc/something would survive the sanitization. The pre-existing _load_agent_details() (line 370) has the same pattern. Neither method is currently reachable with user-controlled input.

Remediation: Not required for this PR since input is trusted. For future-proofing, consider adding os.path.basename() or rejecting names with /:

# Defense-in-depth (optional, not blocking)
slug = agent_name.lower().replace(" ", "-").replace("_", "-")
slug = os.path.basename(slug)  # strip any path components

---

Focused Analysis on PR Changes

ReDoS (Regex Denial of Service) — PASS:
All five new regex patterns (_SHIP_RE, _TEST_RE, _INSTALL_RE, _DELETE_RE, _REVIEW_RE) use simple word alternations with \b word boundary anchors. The only quantifiers are \s* and \s+ between fixed literal words (e.g., pull\s*request, add\s+package). No nested quantifiers or overlapping alternations that could cause catastrophic backtracking. All patterns are pre-compiled at module level.

Prompt injection via generate_standalone_forecast() — PASS:
The user prompt flows into re.search() calls only. Match results control which permission classes/bundles are added to a display string. The prompt text itself is never echoed back, interpolated into templates, or used in any command construction.

HUD state budget manipulation — PASS:
update_hud_state(questionBudget=new_budget) uses a hardcoded key name and the value is computed as max(_question_budget - 1, 0) — always an integer. The update_hud_state function merges kwargs into the state file under exclusive file lock. No arbitrary key injection possible from this call site.

Error information leakage — PASS:
The generic error handler at user-prompt-submit.py:171 prints f"CodingBuddy hook error: {e}" to stderr. This is a local CLI hook — stderr is not exposed to remote users. The exception message may contain file paths, but this is acceptable for local debugging.

---

Secrets Scan

• No hardcoded API keys, passwords, tokens, or secrets found in changed files.
• The only token reference in the hooks tree is secrets_store.py:76 (telegram_bot_token) which is a dict key lookup, not a hardcoded value, and is outside this PR's scope.

Security Checklist

• No hardcoded secrets
• All inputs validated (regex patterns safe, agent names from trusted constants)
• Injection prevention verified (no string interpolation of user input into unsafe contexts)
• Authentication/authorization verified (N/A — no auth changes)
• File operations guarded (isfile() checks, proper error handling, 0o700 permissions)
• Dependencies audited (skipped — tooling unavailable; PR claims clean)

Verdict: Approve. No blocking security issues. The single LOW finding is a defense-in-depth suggestion for future-proofing, not a current vulnerability.

[JeremyDev87]

◮‿◮ Security Re-Review (Round 2)

Scope: 6 files in packages/claude-code-plugin/hooks/ — mode_engine.py, permission_forecast.py, hud_state.py, user-prompt-submit.py, test_mode_engine.py, test_permission_forecast.py
Risk Level: LOW

Summary

• Critical Issues: 0
• High Issues: 0
• Medium Issues: 0
• Low Issues: 0

---

Round 1 Fix Verification: Path Traversal (RESOLVED)

Original issue: _load_agent_details passed agent_name directly into file path construction without stripping path separators, enabling potential directory traversal.

Fix applied in _read_agent_json (mode_engine.py:372-373):

slug = agent_name.lower().replace(" ", "-").replace("_", "-")
slug = os.path.basename(slug)  # path-traversal safety

Verdict: Correctly fixed.

• os.path.basename("../../etc/passwd") returns "passwd", stripping all directory components
• Both callers (_load_agent_details and _load_agent_eye) now route through _read_agent_json, so the fix covers all paths
• Defense-in-depth: os.path.isfile() check on the resolved path adds a second barrier
• Agent names in COUNCIL_PRESETS and DEFAULT_AGENTS are hardcoded string literals (not user-controlled), so the fix is preventive against future code changes that might introduce user-controlled input

---

New Code Analysis

1. Agent JSON cache (_agent_json_cache)

• Instance-level dict, scoped to a single ModeEngine instance
• ModeEngine is instantiated fresh per hook invocation (user-prompt-submit.py:106), so the cache is short-lived
• Cache keys are the raw agent_name strings; values are parsed JSON dicts or None
• No cache poisoning vector: single-threaded hook execution, no shared state
• No unbounded growth: entries limited to the fixed set in COUNCIL_PRESETS
• No issues found.

2. Regex patterns in permission_forecast.py (ReDoS check)

• _SHIP_RE, _TEST_RE, _INSTALL_RE, _DELETE_RE, _REVIEW_RE — all use simple alternation with at most \s* or \s+ quantifiers on character classes
• No nested quantifiers, no ambiguous alternations — all patterns are linear-time
• No ReDoS risk.

3. DELETE bundle (_DELETE_BUNDLE)

• Adds a "destructive" permission class label to the informational forecast display only
• Does NOT grant permissions or execute deletions — purely UI metadata
• No issues found.

4. Question budget persistence (user-prompt-submit.py:98-122)

• Budget read from local HUD state JSON file (not externally controlled)
• Decremented only when detected_mode in ("PLAN", "AUTO") AND output contains "CLARIFICATION REQUIRED"
• Clamped with max(budget - 1, 0) — cannot go negative
• All operations wrapped in try/except — failures silently ignored (acceptable for optional HUD metadata)
• No issues found.

5. Test file (test_permission_forecast.py)

• Standard unittest module with sys.path.insert for import resolution
• Path is constructed relative to __file__ using os.path.dirname — no injection vector
• No issues found.

---

OWASP Top 10 Checklist

Category	Status	Notes
A01 Broken Access Control	PASS	Path traversal fix verified
A02 Cryptographic Failures	N/A	No crypto in scope
A03 Injection	PASS	No eval/exec/SQL; regex is read-only matching
A04 Insecure Design	PASS	Cache is instance-scoped, short-lived
A05 Security Misconfiguration	N/A	No config changes
A06 Vulnerable Components	PASS	No new dependencies
A07 Auth Failures	N/A	No auth code changed
A08 Data Integrity	PASS	JSON deserialization has proper error handling
A09 Logging Failures	N/A	Not in scope
A10 SSRF	N/A	No network calls

Security Checklist

• No hardcoded secrets
• All inputs validated (path traversal mitigated, budget clamped)
• Injection prevention verified (no eval/exec/shell patterns)
• Authentication/authorization — N/A (no auth code)
• Dependencies audited — no new deps added

---

Verdict: APPROVE

All Round 1 findings are resolved. No new security issues identified. The path-traversal fix is correctly implemented with os.path.basename(). New code (cache, regex patterns, budget persistence, DELETE bundle) introduces no security concerns.

[JeremyDev87]

●‿● Code Quality Re-Review (Round 2)

Files Reviewed: 6
Round 1 Issues Verified: 6/6

---

Round 1 Fix Verification

#	Round 1 Finding	Status	Notes
1	[MEDIUM] DRY violation _load_agent_eye vs _load_agent_details	FIXED	Shared _read_agent_json() extracted at mode_engine.py:357. Both callers delegate to it cleanly.
2	[MEDIUM] N+1 file reads in build_council_scene	FIXED	Per-instance _agent_json_cache dict added at mode_engine.py:287. Cache hits on repeated agent lookups. Negative results cached as None to avoid re-reads.
3	[MEDIUM] SHIP/REVIEW regex overlap	Accepted	Kept as-is for MCP parity. Confirmed Python patterns are identical to permission-forecast.ts:15,23.
4	[LOW] Missing prompt-aware forecast tests	FIXED	11 tests added in test_permission_forecast.py. All pass. Coverage: backward compat (3), prompt signals (8 including combined and mode-gating).
5	[LOW] init_hud_state omits questionBudget	FIXED	Added in both _EXTENDED_DEFAULTS (line 24) and init_hud_state (line 99). Default value 3 matches MCP server.
6	[LOW] Long lambda line	Addressed	Lambda at permission_forecast.py:198 is ~98 chars — within tolerance.

New Issues Check

No new CRITICAL or HIGH issues found.

One observation (informational, not blocking):

[LOW] _DELETE_BUNDLE parity divergence
File: permission_forecast.py:40-43
The Python standalone adds a _DELETE_BUNDLE with name "Delete files" when the delete pattern matches. The MCP TypeScript counterpart (permission-forecast.ts:91-92) only adds the destructive class without a bundle. This is strictly an improvement (more informative to the user), but worth noting if strict MCP parity is a goal. No action needed — the Python side is superset-compatible.

Test Results

• test_permission_forecast.py: 11/11 passed
• test_mode_engine.py: 63/63 passed (15 directly related to changed code)

Recommendation

APPROVE — All 6 round 1 findings are properly resolved. The _read_agent_json extraction is clean with correct cache semantics (including negative caching and path-traversal safety via os.path.basename). The new permission forecast tests provide solid coverage of prompt-signal enrichment. No regressions detected.

[JeremyDev87]

Quality Review -- Purpose Alignment Re-Review (Round 2)

Summary

Overall: GOOD
Logic: pass
Error Handling: pass
Design: pass
Maintainability: pass

---

Round 1 Fix Verification

All four round-1 findings have been properly addressed:

#	Round 1 Finding	Status	Evidence
1	[MEDIUM] Missing unit tests for prompt-aware forecast	CLOSED	11 tests in hooks/tests/test_permission_forecast.py -- 3 backward-compat + 8 prompt-aware covering install, test, ship, delete, review (positive and negative), combined signals, and PLAN-with-prompt. All 11 pass.
2	[MEDIUM] init_hud_state schema inconsistency (questionBudget missing)	CLOSED	questionBudget: 3 added to both _EXTENDED_DEFAULTS (line 24) and init_hud_state data dict (line 99) in hud_state.py. Value matches DEFAULT_QUESTION_BUDGET in mode_engine.py.
3	[LOW] DELETE pattern has no bundle	CLOSED	_DELETE_BUNDLE added (permission_forecast.py:40-43) with "Delete files" label and "destructive" class. Wired into generate_standalone_forecast at line 187-188. Test test_delete_adds_destructive_bundle confirms both class and bundle name.
4	[LOW] Double evaluate_clarification_standalone call	CLOSED	user-prompt-submit.py:119 now uses string signal "CLARIFICATION REQUIRED" in instructions to detect clarification output, eliminating the duplicate function call. Comment at line 114 documents the rationale.

---

Re-check of Core Logic

Permission forecast (permission_forecast.py) -- Prompt-aware enrichment logic correctly mirrors MCP permission-forecast.ts patterns. Set-based class accumulation avoids duplicates. Canonical sort via _CLASS_ORDER ensures deterministic output. ACT-mode implicit bundle fallback (line 193-195) matches MCP TS behavior (line 102). All bundles are module-level immutable references appended to a local list -- no aliasing risk.

PLAN template (mode_engine.py:65-84) -- Staged flow (Discover -> Design -> Plan) replaces the legacy "Output full plan in every response" text. Checklist items align with the three stages. Test at line 442-444 updated to match (no longer asserts the hardcoded face glyph).

Clarification budget (user-prompt-submit.py:98-122) -- Reads budget from HUD state via read_hud_state(fill_defaults=True), passes to build_instructions, and persists the decremented budget only when clarification was triggered. The fill_defaults=True guarantees questionBudget key exists even on legacy state files.

Council scene eye glyphs (mode_engine.py:357-444) -- New _read_agent_json method with per-instance cache eliminates N+1 I/O. _make_face falls back to "●‿●" when agent JSON lacks a visual.eye field. Path-traversal safety via os.path.basename(slug) at line 373.

---

Minor Observations (LOW, non-blocking)

• permission_forecast.py:69 -- test method test_plan_with_prompt_still_empty is named as if result is empty, but the assertion confirms the result contains "network". The method name is misleading; the comment at line 72 partially acknowledges this but also states "no bundles" when _INSTALL_BUNDLE is in fact appended. Cosmetic only.
• permission_forecast.py:40-43 -- _DELETE_BUNDLE is an intentional enrichment beyond MCP TS (which adds the class but no bundle for DELETE). This is fine as standalone-specific improvement, but worth noting for future parity audits.
• permission_forecast.py:72-82 -- MODE_DEFAULT_BUNDLES["AUTO"] entries are effectively dead code: AUTO never reaches the fallback because it is gated by mode_upper == "ACT". Same behavior as MCP TS. No bug, just unused data.

Positive Observations

• _read_agent_json cache (line 357-386) is well-designed: per-instance lifetime avoids stale-cache risk across hook invocations while eliminating duplicate I/O within a single council-scene build.
• Path-traversal safety (os.path.basename) on the agent slug is a good defensive measure.
• The string-signal approach for clarification detection is cleaner than the previous double-call pattern -- single source of truth.
• Test coverage is solid: 11 new tests for permission forecast, existing mode_engine tests updated to accommodate the eye-glyph change.

---

Verdict

APPROVE -- All four round-1 gaps are fully closed. Logic is correct, error paths are handled, and the implementation achieves its stated purpose of closing standalone surface gaps for v5.4.0 features. No CRITICAL or HIGH issues remain.