release: workflow + main-scope agent fixes (v8.20.91)

[ksamaschke]

• hook: workflow-enforcement with configurable Task→Plan→Review→Execute→Review→Document sequence
• config: new enforcement.workflow block + workflow-reviewed sample preset
• fix: main_scope_has_agent_privileges no longer disables ~/.claude protection, but still bypasses project-boundary blocks
• docs/changelog/version bumped to 8.20.91

[ksamaschke]

@codex review

[Copilot]

Pull Request Overview

This PR introduces workflow enforcement (v8.20.91) with a configurable Task→Plan→Review→Execute→Review→Document sequence, fixes the main_scope_has_agent_privileges behavior to bypass project boundaries without disabling ~/.claude protection, and includes extensive test coverage and sample configurations.

Key Changes:

• New workflow-enforcement hook with stateful step validation
• Centralized agent context detection via lib/marker-detection.js
• ALL-CAPS filename detection extracted to reusable lib/allcaps-detection.js module
• Documentation write fast-path for infra protection hook

Reviewed Changes

Copilot reviewed 41 out of 41 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
src/hooks/workflow-enforcement.js	New hook enforcing configurable workflow steps with session-based state tracking
src/hooks/lib/marker-detection.js	Centralized agent context detection supporting main_scope_has_agent_privileges flag
src/hooks/lib/allcaps-detection.js	Extracted ALL-CAPS filename detection logic for reuse across hooks
src/hooks/lib/command-validation.js	Role-based command allowlist with configurable main-scope bash commands
src/hooks/agent-infrastructure-protection.js	Documentation write fast-path bypassing infra keyword checks
src/hooks/pm-constraints-enforcement.js	Refactored to use centralized marker detection and segment-based path validation
src/hooks/summary-file-enforcement.js	Uses shared allcaps-detection and marker-detection modules
src/hooks/main-scope-enforcement.js	Integrated allcaps-detection and expanded docs directory defaults
src/hooks/project-scope-enforcement.js	Conditional project boundary enforcement based on main_scope_has_agent_privileges
src/hooks/context-injection.js	MCP hints and mandatory constraint block format instructions
icc.config.default.json	Added workflow enforcement config block and main_scope_has_agent_privileges flag
tests/hooks/integration/test-workflow-enforcement.js	Integration test validating workflow state machine behavior
tests/hooks/unit/test-allcaps-detection.js	Unit tests for aggressive ALL-CAPS detection
tests/hooks/unit/test-marker-detection.js	Test for main_scope_has_agent_privileges override
tests/hooks/unit/test-pm-markdown-allowlist.js	Tests for markdown path allowlist with parent path handling
sample-configs/*	Six sample configurations covering relaxed, strict, workflow, and dev presets
Makefile, install.ps1, ansible/*	CONFIG_FILE parameter support for preset installation
VERSION, CHANGELOG.md, README.md	Version bump to 8.20.91 with comprehensive changelog entries

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

JSON structure error: "heredoc_allowed_commands" (line 197), "auto_commit_review" (line 203), and "infrastructure_protection" (line 207) are indented at the same level as the "enforcement" object, but they should be properties inside the "enforcement" object. These properties need to be indented one level deeper to be nested correctly within the enforcement configuration block.

[Copilot]

Syntax error: line 76 has a comma followed by spread operator ...configAllowed at the end of the array literal. This should be just ...configAllowed without the leading comma, or the array should be closed first and then concatenated. The correct syntax would be ].concat(configAllowed) or moving the spread inside: 'ps', 'top', 'jobs', 'bg', 'fg', ...configAllowed.

[Copilot]

Unused variable blockResponse.

[Copilot]

Unused variable getProjectRoot.

Suggested change

	const { getProjectRoot } = require('./lib/hook-helpers');
	// Removed unused getProjectRoot import

[Copilot]

Unused variable os.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Respect ICC_MAIN_SCOPE_AGENT override in project boundary check

Project boundary enforcement only consults enforcement.main_scope_has_agent_privileges from config (mainScopeAgent), ignoring the ICC_MAIN_SCOPE_AGENT=true override that marker-detection and the changelog advertise for treating the main scope like an agent. With the env var set, other hooks will skip PM limits, but project-scope-enforcement will still block writes outside the project root because mainScopeAgent remains false. This makes the new agent-privileges override inconsistent: e.g., launching a session with ICC_MAIN_SCOPE_AGENT=true and running Write to /tmp/docs/note.md is still denied by the boundary check. Consider incorporating the env override (as marker-detection does) so the bypass works when enabled intentionally.

Useful? React with 👍 / 👎.

[ksamaschke]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. You're on a roll.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull Request Overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 8 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Version mismatch: root VERSION file shows 8.20.91 but src/VERSION shows 8.20.92. These should be synchronized. Based on the PR title mentioning v8.20.91, update src/VERSION to 8.20.91 to match.

[Copilot]

Version mismatch: src/VERSION shows 8.20.92 but root VERSION file shows 8.20.91. These should be synchronized. Based on the PR title mentioning v8.20.91, this should be 8.20.91 to match the root VERSION file.

Suggested change

	8.20.92
	8.20.91

[Copilot]

Missing return statements in early exit conditions (lines 95, 99, 104). The allowOperation function calls process.exit(), so execution will stop, but the missing return makes the control flow unclear. Add return before each allowOperation() call for code clarity and to prevent linter warnings.

Suggested change

	allowOperation(log, true);
	}
	if (!hookInput || !hookInput.session_id) {
	allowOperation(log, true);
	}
	const toolName = normalizeToolName(hookInput.tool_name || hookInput.tool);
	if (!toolName) {
	allowOperation(log, true);
	return allowOperation(log, true);
	}
	if (!hookInput || !hookInput.session_id) {
	return allowOperation(log, true);
	}
	const toolName = normalizeToolName(hookInput.tool_name || hookInput.tool);
	if (!toolName) {
	return allowOperation(log, true);

[Copilot]

Missing return statement before blockOperation() call. Although blockOperation calls process.exit(), the missing return makes control flow unclear. Add return for consistency with other hooks.

Suggested change

	blockOperation(`Workflow enforcement active. Start with the ${firstStep?.name || 'first'} step before running ${toolName}.`, log);
	return blockOperation(`Workflow enforcement active. Start with the ${firstStep?.name || 'first'} step before running ${toolName}.`, log);

[Copilot]

Missing return statement before allowOperation() call. This is after saving state and should explicitly return to indicate the end of this execution path.

Suggested change

	allowOperation(log, true);
	allowOperation(log, true);
	return;

[Copilot]

Missing return statement before blockOperation() call. Add return for consistency and clarity.

[Copilot]

Missing return statement before final allowOperation() call. Add return for consistency.

Suggested change

	allowOperation(log, true);
	return allowOperation(log, true);

[Copilot]

Version mismatch in CHANGELOG: Entry shows 8.20.92 but the PR title and root VERSION file indicate this is release 8.20.91. The 8.20.92 entry should either be renamed to 8.20.91 or moved to a separate PR if these are intended to be different releases.

[ksamaschke]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Align root and source version numbers

The release metadata is inconsistent: CHANGELOG.md and src/VERSION both advertise 8.20.92, but the root VERSION file remains at 8.20.91, so any tooling that reads VERSION (many packaging/CI scripts do) will report the older version while the code and changelog indicate the newer one. Please bump the root version to match so the release number is unambiguous.

Useful? React with 👍 / 👎.

[Copilot]

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated 8 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The heredoc body extraction logic is incorrect. The regex /^.*?\n/s with the s flag makes . match newlines, causing the *? to match everything up to the last newline before the terminator, effectively removing the entire body except the last line.

The correct approach is to remove only the first line (without the s flag): trimmed.replace(/^.*?\n/, '')

For example, with input:

cat <<EOF > file
line1
line2
EOF

Current code extracts: EOF (just the terminator line)
Should extract: line1\nline2\nEOF

Suggested change

	const body = trimmed.replace(/^.*?\n/s, '');
	const body = trimmed.replace(/^.*?\n/, '');

[Copilot]

[nitpick] Missing cleanup of temporary test directory. The test creates a temporary directory in createTestProject() but never cleans it up after the tests complete. This can lead to accumulation of test directories in the system's temp folder over time.

Consider adding cleanup logic at the end of the test suite:

try {
  // ... existing test code ...
} finally {
  fs.rmSync(projectDir, { recursive: true, force: true });
}

Suggested change

	const planFirst = runHook(projectDir, 'Plan', sessionId);
	assert.strictEqual(planFirst.code, 2, 'Plan before Task should be blocked');
	const allowedSequence = ['Task', 'Plan', 'Review', 'Execute', 'Document'];
	allowedSequence.forEach((tool, index) => {
	const result = runHook(projectDir, tool, sessionId);
	assert.strictEqual(result.code, 0, `Step ${index + 1} (${tool}) should be allowed`);
	});
	const restart = runHook(projectDir, 'Task', sessionId);
	assert.strictEqual(restart.code, 0, 'Workflow should reset after Document');
	const outOfOrder = runHook(projectDir, 'Execute', sessionId);
	assert.strictEqual(outOfOrder.code, 2, 'Execute before Plan should be blocked in new cycle');
	console.log('✅ Workflow enforcement integration tests passed');
	try {
	const planFirst = runHook(projectDir, 'Plan', sessionId);
	assert.strictEqual(planFirst.code, 2, 'Plan before Task should be blocked');
	const allowedSequence = ['Task', 'Plan', 'Review', 'Execute', 'Document'];
	allowedSequence.forEach((tool, index) => {
	const result = runHook(projectDir, tool, sessionId);
	assert.strictEqual(result.code, 0, `Step ${index + 1} (${tool}) should be allowed`);
	});
	const restart = runHook(projectDir, 'Task', sessionId);
	assert.strictEqual(restart.code, 0, 'Workflow should reset after Document');
	const outOfOrder = runHook(projectDir, 'Execute', sessionId);
	assert.strictEqual(outOfOrder.code, 2, 'Execute before Plan should be blocked in new cycle');
	console.log('✅ Workflow enforcement integration tests passed');
	} finally {
	fs.rmSync(projectDir, { recursive: true, force: true });
	}

[Copilot]

Duplicated logic: The allowlist directory definitions and the parent path validation logic (lines 974-984) are nearly identical to lines 1119-1128. This creates a maintenance burden where changes need to be made in two places.

Consider extracting this logic into a helper function, e.g., checkMarkdownParentPathAllowlist(filePath, projectRoot) that can be called from both locations.

[Copilot]

Missing return statement before allowOperation. When workflow is disabled or has no steps, the function should return after allowing the operation, but it continues to execute the rest of the function instead. This will cause the workflow enforcement logic to run even when it's disabled.

Add return before allowOperation(log, true); on line 95.

Suggested change

	allowOperation(log, true);
	return allowOperation(log, true);

[Copilot]

Missing return statement before allowOperation. When hookInput or session_id is missing, the function should return after allowing the operation, but it continues to execute the rest of the function instead.

Add return before allowOperation(log, true); on line 99.

Suggested change

	allowOperation(log, true);
	return allowOperation(log, true);

[Copilot]

Missing return statement before allowOperation. When tool name is missing, the function should return after allowing the operation, but it continues to execute the rest of the function instead.

Add return before allowOperation(log, true); on line 104.

Suggested change

	allowOperation(log, true);
	return allowOperation(log, true);

[Copilot]

Missing return statement before allowOperation. After allowing the first step in the workflow, the function should return, but it continues to execute the subsequent workflow state checks instead, which will incorrectly try to match against expectedStep (which would be the second step).

Add return before allowOperation(log, true); on line 120.

Suggested change

	allowOperation(log, true);
	return allowOperation(log, true);

[Copilot]

Missing return statement before final allowOperation. After successfully completing a workflow step, the function should return, but instead execution continues and could potentially reach the catch block or other code paths.

Add return before allowOperation(log, true); on line 137.

Suggested change

	allowOperation(log, true);
	return allowOperation(log, true);