Skip to content

fix #317: suppress workflow context injection on terminal COMPLETE state#318

Merged
azalio merged 1 commit into
mainfrom
claude/compassionate-cerf-jqecld
Jul 2, 2026
Merged

fix #317: suppress workflow context injection on terminal COMPLETE state#318
azalio merged 1 commit into
mainfrom
claude/compassionate-cerf-jqecld

Conversation

@azalio

@azalio azalio commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Problem

When a MAP workflow finished and step_state.json was left with current_step_id="COMPLETE" / current_step_phase="COMPLETE", the workflow-context-injector hook still injected a reminder. Because required_action_for_step() had no terminal-state awareness, it fell through to the generic fallback and produced:

REQUIRED: Complete phase COMPLETE

This misleading banner appeared on every tool call in a new session on the same branch, even though the workflow was long finished.

Fix

Added _TERMINAL_STEP_IDS: frozenset[str] = frozenset({"COMPLETE"}) constant in workflow-context-injector.py.jinja and an early-return guard at the top of format_reminder():

if step_id in _TERMINAL_STEP_IDS or step_phase in _TERMINAL_STEP_IDS:
    return None

The existing main() logic already handles None from format_reminder() by printing {} (no injection), so no further changes were needed.

The .jinja source was edited and make render-templates propagated the fix to all generated output trees (.claude/hooks/, src/mapify_cli/templates/hooks/).

Tests

Added TestTerminalStateSuppress class (9 tests) to tests/test_workflow_context_injector.py:

  • Integration tests (subprocess): Edit/Write/Bash with COMPLETE state → {}
  • Partial-terminal cases: only step_id=COMPLETE or only step_phase=COMPLETE each suppress
  • Guard against regression text: "REQUIRED" and "Complete phase COMPLETE" must not appear in output
  • Unit tests: format_reminder() returns None directly for both full-COMPLETE and phase-only-COMPLETE states
  • Sanity: active ACTOR state (step_id="2.3") still emits the [MAP] reminder

All 3234 tests pass (make check clean — ruff, mypy, pyright, pytest, check-render).

Fixes #317


Generated by Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Prevented workflow reminders from appearing when a session is already completed, avoiding stale or misleading prompts on later runs.
    • Suppressed reminder injection when either the step ID or step phase indicates a completed state.
  • Tests

    • Added regression coverage to verify completed workflows no longer emit reminders.
    • Expanded checks to confirm normal reminders still appear for non-terminal states.

When step_state.json has current_step_id or current_step_phase equal to
"COMPLETE", format_reminder() now returns None immediately via a terminal-
state guard, so the hook emits {} instead of a misleading
"REQUIRED: Complete phase COMPLETE" banner.

Add _TERMINAL_STEP_IDS frozenset constant and 9 regression tests covering
both the integration (subprocess) and unit (format_reminder) paths.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JfWuWC5LkLzZTU6xuLdvAE
@azalio azalio merged commit 74148ec into main Jul 2, 2026
1 check passed
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 399814e8-5d4a-467f-8dea-9e6b54844123

📥 Commits

Reviewing files that changed from the base of the PR and between a650894 and 615fc50.

📒 Files selected for processing (4)
  • .claude/hooks/workflow-context-injector.py
  • src/mapify_cli/templates/hooks/workflow-context-injector.py
  • src/mapify_cli/templates_src/hooks/workflow-context-injector.py.jinja
  • tests/test_workflow_context_injector.py

📝 Walkthrough

Walkthrough

Adds a _TERMINAL_STEP_IDS frozenset ({"COMPLETE"}) and a guard in format_reminder(...) that returns None when current_step_id or current_step_phase is terminal, applied identically across the installed hook, packaged template, and jinja source. A regression test suite validates the suppression behavior.

Changes

Terminal state suppression

Layer / File(s) Summary
Terminal state constant and guard
.claude/hooks/workflow-context-injector.py, src/mapify_cli/templates/hooks/workflow-context-injector.py, src/mapify_cli/templates_src/hooks/workflow-context-injector.py.jinja
Adds _TERMINAL_STEP_IDS = {"COMPLETE"} and an early-return None in format_reminder(...) when current_step_id or current_step_phase matches a terminal state, preventing stale COMPLETE state from producing misleading reminders.
Regression tests
tests/test_workflow_context_injector.py
Adds TestTerminalStateSuppress with a state-seeding helper and tests covering Edit/Write/Bash suppression, partial-field terminal matches, absence of misleading "REQUIRED"/"Complete phase COMPLETE" text, unit-level format_reminder None checks, and continued injection for non-terminal ACTOR state.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Hook as PreToolUse Hook
  participant State as step_state.json
  participant Formatter as format_reminder()

  Hook->>State: read current_step_id, current_step_phase
  Hook->>Formatter: format_reminder(state)
  alt step_id or phase is terminal (COMPLETE)
    Formatter-->>Hook: None
    Hook-->>Hook: emit {} (no context injected)
  else non-terminal state
    Formatter-->>Hook: reminder text
    Hook-->>Hook: inject additionalContext
  end
Loading

Poem

A branch once whispered "COMPLETE, COMPLETE!"
But stale old context, we won't repeat!
With a frozenset guard, so tidy and small,
No misleading reminders shall fall.
Hop, hop, hooray — tests all pass! 🐇✅

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/compassionate-cerf-jqecld

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

workflow-context-injector injects terminal completed MAP state

2 participants