[Suggestion] additionalContext is declared in UserPromptExpansionOutput but is silently dropped at every layer:

1. hookRunner.ts:applyHookOutputToInput has no case for HookEventName.UserPromptExpansion (only UserPromptSubmit and PreToolUse are handled), so sequential hooks cannot chain additionalContext into the prompt.
2. Neither consumer (slashCommandProcessor.ts nor nonInteractiveCliCommands.ts) calls output.getAdditionalContext(), so even parallel hook results lose the field.

Hook authors who return additionalContext (a field the type advertises) will find it has no effect — a type-vs-implementation mismatch.

Either add the corresponding case in applyHookOutputToInput and consume it in both paths, or remove additionalContext from UserPromptExpansionOutput if it's intentionally unsupported at this lifecycle stage.

— qwen-latest-series-invite-beta-v38 via Qwen Code /review

[Suggestion] Two observations on the hook integration here:

Model-invocable commands skip the hook. setModelInvocableCommandsExecutor (line ~456) also handles submit_prompt results from model-invoked commands (when the model calls /goal directly), but does NOT fire the UserPromptExpansion hook. If the hook's purpose is to audit/gate prompt expansion regardless of origin, model-invoked commands bypass it. Consider either firing the hook there too, or adding a comment explaining the intentional exclusion.

Duplicated blocking-check logic. The ~25-line hook-fire + blocking-check pattern is inlined here, while nonInteractiveCliCommands.ts extracts the same pattern into a helper fireUserPromptExpansionHook(). Both paths share serializeUserPromptExpansionPrompt and formatUserPromptExpansionBlockedMessage from userPromptExpansionHook.ts — consider extracting the control flow into a shared helper too, so future fixes (e.g. consuming additionalContext) only need to land in one place.

— qwen-latest-series-invite-beta-v38 via Qwen Code /review

[Critical] Mock object is missing openDiffDialog property, causing a TS2741 type error. This is related to the openDiffDialog prop added to SlashCommandProcessorActions.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[Critical] serializeUserPromptExpansionPrompt(content) is called unconditionally — before any check for whether UserPromptExpansion hooks are even configured. Every slash command returning submit_prompt now pays the cost of partToString(content, { verbose: true }) even when zero hooks match. The codebase already has a hasHooksForEvent guard pattern (see client.ts:680) to skip this work on the hot path — it should be applied here too.

Also applies at line 478 (model-invocable executor path) and nonInteractiveCliCommands.ts:196.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[Suggestion] The model-invocable executor path calls fireUserPromptExpansionEvent without passing an AbortSignal. The main user-invoked path (line 793) correctly passes abortController.signal. Without a signal, hooks spawned from model-invoked commands cannot be cancelled if the user aborts the operation.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[Suggestion] new AbortController().signal creates a fresh signal that is never aborted, making hook execution uncancellable. The parent abortController is in scope — use its signal instead.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[Suggestion] additionalContext from hook output is appended to the prompt without any length truncation. Hook stdout is capped at 1MB (MAX_OUTPUT_LENGTH), so a malicious or buggy hook can inject up to ~1MB into every expanded prompt. Consider applying a reasonable character limit (e.g., 10000 chars) before appending.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[Suggestion] UserPromptExpansionOutput interface is declared but never instantiated — createHookOutput has no case for UserPromptExpansion, so all outputs are created as generic DefaultHookOutput. Either add the case in createHookOutput or remove the unused interface and document why the default is sufficient.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[Suggestion] The MCP-based command submit_prompt test does not assert that fireUserPromptExpansionEvent was called, even though the source code does invoke it for MCP commands. Adding this assertion would provide regression protection for the MCP command hook integration path.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[Critical] When a UserPromptExpansion hook blocks a model-invocable command, this returns null. The SkillTool caller (skill.ts:384-420) interprets null as "command not found" and feeds the model Skill "<name>" not found — which is factually wrong. The command exists and executed; a security hook blocked the expansion.

The block is invisible to both the model and the user. The model may retry the command, try alternative commands, or report to the user that the skill doesn't exist.

The same issue exists in nonInteractiveCliCommands.ts:321 (if (hookResult.blockedResult) return null;).

— qwen3.7-max via Qwen Code /review

[Critical] Same issue as the interactive path: when a hook blocks, return null causes the SkillTool caller to report Skill not found instead of the actual block reason. Return the blocked message content instead so the model can surface the block reason accurately.

— qwen3.7-max via Qwen Code /review

[Suggestion] When a hook blocks a command here, hasError is never set to true (it's only set in the catch block at ~line 947). The finally block then logs SlashCommandStatus.SUCCESS for the blocked command.

This makes telemetry misleading — blocked commands appear as successful, making production debugging significantly harder. Consider setting hasError = true before returning, or introducing a dedicated SlashCommandStatus.BLOCKED.

— qwen3.7-max via Qwen Code /review

[Suggestion] Adding UserPromptExpansion to the fail-closed set means any internal error in the hook planner/runner (e.g., malformed hook config, registry crash) will block all slash command expansions in that session — not just the command that triggered the error. Unlike TodoCreated/TodoCompleted (system events where blocking is safe), UserPromptExpansion is a user-facing interaction path. A single misconfigured hook definition could deny all slash-command functionality with only a generic error message.

Consider scoping the fail-closed behavior to the specific hook definition that caused the error (skip the failing hook, let the command proceed without hook context), or catching planner/registry errors at hook registration time so misconfigured hooks are excluded before execution rather than blocking all commands at runtime.

— qwen3.7-max via Qwen Code /review

[Critical] Model-invocable executor returns block message as success string

When a UserPromptExpansion hook blocks a model-invocable command (invoked via SkillTool), this returns formatUserPromptExpansionBlockedMessage(...) — a non-null string. The SkillTool caller in skill.ts treats any non-null return as success: wraps it as llmContent: [{ text: content }], logs the launch as success, and feeds the block message to the model as if it were valid command output. The security/policy intent of the block is defeated in this path.

Same issue in nonInteractiveCliCommands.ts:322 (hookResult.blockedResult.content returned as string).

— qwen3.7-max via Qwen Code /review

[Suggestion] matchesCommandName delegates to matchesToolName, whose regex-fallback warning (line 220) says for tool "${toolName}". When called from here, the log misleadingly references "tool" for a slash command name.

Consider giving matchesCommandName its own implementation with a "for command" log message, following the matchesAgentType / matchesSessionTrigger pattern.

— qwen3.7-max via Qwen Code /review

[Critical] return null when a hook blocks causes SkillTool (skill.ts:385) to report Skill "<name>" not found to the model — hiding the actual block reason.

The R4 fix changed from returning the block message (wrong: treated as valid skill output) to returning null (wrong: treated as "command not found"). The correct behavior is to return the block reason string so the model sees it as a tool result:

— qwen3.7-max via Qwen Code /review

[Critical] Same issue as the interactive path: return null when a hook blocks causes SkillTool to report Skill not found instead of the actual block reason.

This returns the block message string (e.g. "UserPromptExpansion blocked: <reason>") so the model receives actionable information.

— qwen3.7-max via Qwen Code /review

[Suggestion] applyHookOutputToInput reads raw hookSpecificOutput['additionalContext'] and appends it directly to the prompt for the next sequential hook — bypassing both the HTML entity escaping performed by getAdditionalContext() (types.ts:355, < → <, > → >) and the 10,000-character truncation applied by appendUserPromptExpansionAdditionalContext in the CLI layer.

When multiple UserPromptExpansion hooks run sequentially, each hook's raw additionalContext accumulates in the prompt piped to the next hook's stdin — unescaped and unbounded. The final output to the model is sanitized by the CLI layer, but intermediate hooks see a different (weaker) representation.

Consider applying the same sanitization/truncation here that the parallel path uses, or document the intentional difference:

const MAX_ADDITIONAL_CONTEXT = 10_000;
// ...
const additionalContext =
  hookOutput.hookSpecificOutput['additionalContext'];
if (
  typeof additionalContext === 'string' &&
  'prompt' in modifiedInput
) {
  (
    modifiedInput as
      | UserPromptSubmitInput
      | UserPromptExpansionInput
  ).prompt += '\n\n' + additionalContext.slice(0, MAX_ADDITIONAL_CONTEXT);
}

— qwen3.7-max via Qwen Code /review

[Critical] UserPromptSubmit sequential chaining now HTML-escapes additionalContext — silent behavioral regression.

The fall-through merges UserPromptSubmit and UserPromptExpansion into a single case that routes through createHookOutput(eventName, hookOutput).getAdditionalContext(). Since createHookOutput has no case HookEventName.UserPromptSubmit, it falls through to DefaultHookOutput, whose getAdditionalContext() HTML-escapes < → < and > → >.

Before this PR, the UserPromptSubmit case directly read hookOutput.hookSpecificOutput['additionalContext'] without any escaping. Any existing sequential UserPromptSubmit hook chain that includes angle brackets in additionalContext (XML tags, HTML snippets, generic type syntax) will now see </> silently injected into the chained prompt. No test covers this path.

— qwen3.7-max via Qwen Code /review

[Critical] Prompt injection via hook block reason in model-invocable paths.

formatUserPromptExpansionBlockedMessage(reason) interpolates the reason string verbatim into the return value. In model-invocable command executors (slashCommandProcessor.ts:497, nonInteractiveCliCommands.ts:214), this return value is fed back to the LLM via SkillTool as llmContent. The reason is neither sanitized nor truncated.

A malicious project-level hook (via .qwen/settings.json in an untrusted repo) can set decision: "block" with a crafted reason containing prompt injection payloads. The model receives this as if it were the skill command's legitimate output.

— qwen3.7-max via Qwen Code /review

[Suggestion] Sequential hook chaining has no cumulative cap on additionalContext growth.

Each sequential hook can append up to 10,000 characters via getAdditionalContext(). With N sequential hooks, the prompt grows by up to N×10K with no total cap. The parallel path correctly concatenates all contexts and truncates once, but the sequential path accumulates unboundedly.

Consider adding a cumulative check before appending, or documenting the behavior for hook authors.

— qwen3.7-max via Qwen Code /review

[Suggestion] Zero debug logging at all 4 UserPromptExpansion hook call sites.

fireUserPromptExpansionHook and the two interactive paths in slashCommandProcessor.ts emit no log when hooks fire, block, or are skipped. When debugging hook-related issues, operators must enable debug-level logging for the generic hook system and correlate manually.

Add debugLogger calls at key decision points:

debugLogger.debug(`UserPromptExpansion hook firing for /${commandName}`);
// after hook execution:
debugLogger.debug(`UserPromptExpansion result: blocked=${blockingError.blocked}`);

— qwen3.7-max via Qwen Code /review

[Suggestion] Multiple test gaps in the blocking behavior:

1. shouldStopExecution() path: All blocking tests use blockingError.blocked = true with shouldStopExecution: () => false. No test exercises the shouldStopExecution() = true branch (which takes a different code path through getBlockingError() returning { blocked: false }).

2. getEffectiveReason() fallback: All blocking mocks use reason: 'Blocked by policy' (truthy), so blockingError.reason || output.getEffectiveReason() always short-circuits. The fallback is never exercised.

3. Hook-disabled guard: All tests mock getDisableAllHooks: false and hasHooksForEvent: true. No test verifies passthrough when hooks are disabled or not configured.

— qwen3.7-max via Qwen Code /review

[Suggestion] The third branch of appendUserPromptExpansionAdditionalContext (single Part object fallback at userPromptExpansionHook.ts:31) is never tested. Only the string and Array branches are covered. Add a test with a single Part object input to verify the fallback wrapping behavior.

— qwen3.7-max via Qwen Code /review

[Suggestion] formatUserPromptExpansionBlockedMessage HTML-escapes < and >, but all three callers render the output as plain text, not HTML:

1. Interactive TUI (slashCommandProcessor.ts:822): addMessage({ type: MessageType.ERROR }) — Ink renders as terminal plain text; < appears literally on screen.
2. Model-invocable (slashCommandProcessor.ts:497): returned as string to SkillTool → llmContent: [{ text }] — model receives < literally.
3. ACP non-interactive: thrown as new Error(result.content), displayed via plain-text emitter.

The existing UserPromptSubmit blocked message (Session.ts:656) does NOT HTML-escape: `🚫 **UserPromptSubmit blocked**: ${blockReason}`.

Additionally, & is not escaped before </>, so pre-encoded entities like <script> pass through unchanged.

— qwen3.7-max via Qwen Code /review

[Suggestion] getAdditionalContext() calls super.getAdditionalContext() which HTML-escapes the full string (potentially up to 1MB from hook stdout), THEN .slice(0, 10_000) truncates. Two issues:

1. Truncated mid-entity: If < appears near the 10K boundary, the escaped form < (4 chars) may be cut at &l or &lt — producing garbled fragments in the prompt.
2. Wasted work: The HTML escape runs over the full string before discarding 99% of it.

Fix: read the raw value directly, truncate first, then escape:

— qwen3.7-max via Qwen Code /review

[Suggestion] serializeUserPromptExpansionPrompt is a new exported function with zero test coverage. It wraps partToString(content, { verbose: true }) and is called at all 4 hook invocation sites, but no test verifies:

1. String input → returns the string
2. Part array → returns concatenated text with verbose formatting
3. Single Part object → returns text content
4. Empty/undefined input

This is the serialization bridge between the slash command result and the hook's prompt field. If partToString with { verbose: true } produces unexpected output, hooks receive incorrect input and no test catches it.

— qwen3.7-max via Qwen Code /review

[Suggestion] The early-return branch when additionalContext is undefined has no test. This is the hot path — every slash command without hooks configured, or whose hooks return no additional context, hits this branch. A regression that mutates the input content when additionalContext is undefined (e.g., accidentally appending \n\nundefined) would go undetected.

Suggested test:

it('returns content unchanged when additionalContext is undefined', () => {
  expect(appendUserPromptExpansionAdditionalContext('base', undefined)).toBe('base');
});

— qwen3.7-max via Qwen Code /review

[Suggestion] fireUserPromptExpansionHook has three guard/early-return branches that are all untested:

1. config.getDisableAllHooks?.() returns true
2. config.hasHooksForEvent?.('UserPromptExpansion') returns false
3. config.getHookSystem() returns null/undefined

In the test setup, all three are configured to enable hooks, so only the "hooks fire" path is exercised. If any guard condition is inverted or the null-check is removed, every non-interactive slash command would attempt to fire hooks even when disabled, potentially throwing errors.

Suggested tests: set each guard to trigger early-return, assert that fireUserPromptExpansionEvent is NOT called and the original content passes through unchanged.

— qwen3.7-max via Qwen Code /review

[Suggestion] Three new i18n keys are introduced via t() but zero locale files are updated. All 9 locale files (en.js, zh.js, fr.js, de.js, ja.js, pt.js, ru.js, zh-TW.js, ca.js) are missing translations for:

1. 'block expanded prompt submission and show stderr to user only'
2. 'When a slash command expands into a prompt'
3. 'Input to command is JSON with command_name, command_args, and expanded prompt text.'

Every other hook event (UserPromptSubmit, Notification, SessionStart, etc.) has corresponding entries in all locale files. t() falls back to the key, so English renders correctly but non-English users see untranslated strings.

— qwen3.7-max via Qwen Code /review

[Suggestion] Truncate-before-escape can exceed MAX_LENGTH in downstream consumers.

The raw input is truncated to 10,000 chars first, then </> are escaped. Each angle bracket expands from 1 to 4 chars (<), so the output can reach up to ~40,000 chars in the worst case. This affects two paths differently:

• hookRunner chaining (hookRunner.ts:507): no second truncation — the chained prompt receives the full escaped output, violating the intended 10K cap.
• CLI path (appendUserPromptExpansionAdditionalContext): applies a second .slice(0, 10000) on the already-escaped string, which can cut mid-entity (e.g., < → &l), producing garbled text appended to the user's prompt.

Adding a second .slice() after escaping guarantees the output never exceeds MAX_LENGTH:

— qwen3.7-max via Qwen Code /review

[Suggestion] No direct unit tests for this rewritten method.

The method was substantially rewritten (bypass super, direct field access, truncate-then-escape ordering), but has no direct unit tests. It's only exercised indirectly through hookRunner.test.ts chaining tests. Four type-guard branches are untested:

1. hookSpecificOutput is undefined
2. 'additionalContext' key is absent
3. Value is not a string (e.g., number)
4. Value is empty string ""

Consider adding a describe('UserPromptExpansionHookOutput.getAdditionalContext', ...) block in the core hook tests.

— qwen3.7-max via Qwen Code /review

[Suggestion] Override duplicates the parent's field-access pattern instead of calling super.

The 4-line raw-field access pattern (this.hookSpecificOutput && 'additionalContext' in ... && typeof ... === 'string') is identical to DefaultHookOutput.getAdditionalContext() at lines 344-356. If the parent adds logging, validation, or telemetry in the future, this override silently skips it.

Consider extracting a protected getRawAdditionalContext(): string | undefined helper in the parent that both methods reuse:

// In DefaultHookOutput:
protected getRawAdditionalContext(): string | undefined {
  if (this.hookSpecificOutput && 'additionalContext' in this.hookSpecificOutput) {
    const context = this.hookSpecificOutput['additionalContext'];
    return typeof context === 'string' ? context : undefined;
  }
  return undefined;
}

// In UserPromptExpansionHookOutput:
override getAdditionalContext(): string | undefined {
  const raw = this.getRawAdditionalContext();
  if (!raw) return undefined;
  return raw
    .slice(0, MAX_USER_PROMPT_EXPANSION_ADDITIONAL_CONTEXT_LENGTH)
    .replace(/</g, '&lt;')
    .replace(/>/g, '&gt;');
}

— qwen3.7-max via Qwen Code /review