Skip to content

chore: sync Claude/Codex tooling to drm-copilot v1.0.20 (routing-gate and complexity-floor fixes) - #414

Merged
drmoisan merged 2 commits into
mainfrom
chore/update-agents
Jul 31, 2026
Merged

chore: sync Claude/Codex tooling to drm-copilot v1.0.20 (routing-gate and complexity-floor fixes)#414
drmoisan merged 2 commits into
mainfrom
chore/update-agents

Conversation

@drmoisan

@drmoisan drmoisan commented Jul 31, 2026

Copy link
Copy Markdown
Owner

Summary

  • Synchronizes the .claude/ and .codex/ governance tooling in this repository to the drm-copilot v1.0.20 push-down, across two commits (Claude surface, then Codex surface).
  • Fixes a false-positive in the orchestrator completion gate: Invoke-RoutingContractValidation no longer treats any captured validator output as a failure, because the invoker captures with 2>&1 and the validator prints its success line to stdout on a clean pass. The process exit code is now the sole discriminator.
  • Corrects complexity-floor computation in ModelRouting.psm1: Get-ComplexityFloor now intersects the recorded signal names against an explicit floor-signal set instead of treating every supplied name as a triggered floor signal.
  • Extends the orchestrator-state step-status vocabulary with a per-key additive set (STEP_SPECIFIC_EXTRA_STATUS), so remediation-loop and CI-loop statuses validate only on the step keys that own them, and blocked_remediation_loop_limit now blocks PR-creation readiness on steps 5–8.
  • Adds .codex/hooks/codex-pretooluse-file-mapping.ps1, a shared, entrypoint-free transport module for Codex PreToolUse payload parsing and tool_input-to-file-edit mapping. Its payload parser deliberately performs no tool-name assertion; per its own documentation, narrowing admission at that layer was the defect that caused every Edit and Write payload to exit 2.
  • Renames the TypeScript test runner from Vitest to Jest across the atomic-executor agent tool allowlist and the two general rule files.
  • Bumps the Codex MCP server pin from @danmoisan/drm-copilot-mcp@1.0.18 to @1.0.20.

Why

The PR Intent fields in the context bundle (artifacts/pr_context.summary.txt) are empty, and no feature documents, scoping documents, or referenced issues are present in the range. The motivation below is therefore derived only from the diff and from the in-code rationale comments the push-down carries.

  • Routing-gate false positive. The prior condition was ($exitCode -ne 0) -or (-not [string]::IsNullOrWhiteSpace($outputText)). Since the default invoker captures combined output, the validator's own success line populated $outputText on a clean pass, so the gate reported a routing-contract failure whenever the Python CLI succeeded. This blocked DONE on passing checkpoints.
  • Floor-signal contract mismatch. The previous implementation assumed its caller pre-filtered SignalsPresent down to "floor": true catalog names. Callers instead pass the full checkpoint signals_present[] array, so "floor": false names and unknown names were each raising the floor to C3. The fix moves the filter inside the function and documents the parameter as the full recorded set.
  • Step-status vocabulary gap. The shared VALID_STEP_STATUS list had no room for the loop-limit and remediation statuses that step 6 and step 9 legitimately record, so valid checkpoints failed base presence validation. A per-key additive set admits each value only on its owning key and keeps it rejected everywhere else.
  • Codex hook duplication. Transport parsing and file-edit mapping were duplicated with drift across the hooks registered under the ^(apply_patch|Edit|Write)$ matcher. The two named consumers, enforce-checkpoint-monotonic.ps1 (303 lines) and enforce-completion-consistency.ps1 (300 lines), are described in the new module's header as being near the repository's 500-line file cap and therefore unable to absorb the mapping logic inline.

What Changed

Claude governance surface (commit da21214a)

File Change
.claude/hooks/validate-orchestrator-output.ps1 $hasErrors reduced to ($exitCode -ne 0); doc comment rewritten to state that ErrorText now carries the success line on a clean pass.
.claude/lib/model-routing/ModelRouting.psm1 New module-scope $script:FLOOR_SIGNAL_NAMES constant (four names); Get-ComplexityFloor filters its input through it before the empty-input guard; documentation updated to describe the full-recorded-set parameter contract.
.claude/lib/orchestrator-state/OrchestratorState.psm1 New $script:STEP_SPECIFIC_EXTRA_STATUS hashtable (step6_statusblocked_remediation_loop_limit; step9_statuspassed, failed_remediation_required, blocked_ci_loop_limit); base presence check consults the per-key extra set; PR-creation readiness rejects blocked_remediation_loop_limit alongside pending and blocked.
.claude/agents/atomic-executor.md Bash(npx vitest *)Bash(npx jest *) in the tool allowlist, and the matching change in the TypeScript quality-gate prose.
.claude/rules/general-code-change.md Unit-test stage example: Vitest → Jest.
.claude/rules/general-unit-test.md Permitted coverage exclude vitest.config.tsjest.config.cjs; fake-timer guidance vi.useFakeTimers()jest.useFakeTimers().

Codex governance surface (commit bfe83d4b)

File Change
.codex/hooks/codex-pretooluse-file-mapping.ps1 New file, 474 lines. Public surface is ConvertFrom-CodexPreToolUsePayload and ConvertTo-CodexFileEditInput; ConvertTo-CodexAddedLineText, Test-CodexGovernedPath, and Resolve-CodexUpdatedFileContent are documented as internal helpers. Carries the apply_patch file, move, and hunk regexes. Performs no policy evaluation.
.codex/hooks/enforce-epic-child-worktree-binding.ps1 Branch resolution extracted into Get-CodexChildGuardLiveBranch, which returns '' on non-zero exit or whitespace-only output and trims inside the helper.
.codex/hooks/enforce-epic-planning-only.ps1 Branch resolution extracted into Invoke-EpicPlanningGit and Get-EpicPlanningCurrentBranch; the EPIC_PLANNING_ONLY_BLOCKED throw on unresolvable branch moves into the helper.
.codex/config.toml MCP server args pin 1.0.181.0.20.

No mechanical moves or renames are present in the range. Diff shortstat: 10 files changed, 595 insertions, 54 deletions.

Architecture / How It Fits Together

  • Completion gate. .claude/hooks/validate-orchestrator-output.ps1 invokes the routing-contract validator as a child process and converts the result into a hook decision. The change alters only the failure discriminator inside Invoke-RoutingContractValidation; the returned hashtable shape (HasErrors, ErrorText) and the MODEL_ROUTING_BLOCKED: block-reason string are unchanged, so every existing caller is unaffected.
  • Model routing. Get-ComplexityFloor is a pure function consumed by the orchestrator when writing complexity_assessments[].floor. Per .claude/rules/orchestrator-state.md, the checkpoint validator requires floor == compute_complexity_floor(signals_present), so this function and the validator must agree on which names are floor signals. The constant is hard-coded rather than read from config/orchestration-routing.json because the module is pushed down to consumer repositories that do not ship that config.
  • Orchestrator state. OrchestratorState.psm1 mirrors the base checkpoint-presence checks (required keys, step-status validity, blocked_reason validity) that run before any mode-specific gate. The additive vocabulary is applied inside the existing STEP_STATUS_KEYS loop, so an absent step key still contributes no error.
  • Codex hook chain. .codex/config.toml registers a list of PreToolUse handlers under matcher = "^(apply_patch|Edit|Write)$". The new mapping module is designed to be dot-sourced by those handlers: it defines only script-scoped constants and functions, reads no stdin, and has no exit-code semantics, following the enforce-completion-helpers.ps1 precedent. Reading the payload remains each calling hook's responsibility.

Verification

Completed

  • Floor-signal parity, verified during PR authoring. The four names in $script:FLOOR_SIGNAL_NAMESclassifier_or_model_logic, auth_or_token_handling, concurrency_or_ordering, cross_module_contract_change — were compared against config/orchestration-routing.json and match exactly the entries flagged "floor": true. The three "floor": false names (single_file_localized_edit, mechanical_rename_or_move, docs_or_comment_only) are correctly absent from the constant.
  • Consumer-registration check, verified during PR authoring. enforce-checkpoint-monotonic.ps1 and enforce-completion-consistency.ps1 are both registered in .codex/config.toml under the ^(apply_patch|Edit|Write)$ matcher.

Not verified in this PR

  • No canonical verification evidence was parsed into the PR context bundle, and CI status for HEAD is reported as not available.
  • No automated test run covers this diff. This repository contains no Pester suite for the .claude/lib/** or .codex/hooks/** trees; the only *.Tests.ps1 files present are under tests/scripts/vscode/. The config-parity and transport-parity Pester suites referenced in the changed files' documentation comments reside in the upstream drm-copilot repository, not here.
  • The Python reference modules cited by the changed PowerShell modules (scripts/dev_tools/compute_complexity_floor.py, scripts/dev_tools/resolve_delegation_model.py, scripts/dev_tools/validate_orchestrator_state.py, scripts/dev_tools/_orchestrator_state_step_status.py) are not present in this repository, so cross-language parity could not be executed locally. In this consumer repository the PowerShell modules are the operative implementation.
  • No C# source, project, or build-configuration file is touched, so the CSharpier → analyzers → nullable → MSTest toolchain is not applicable to this diff.

Recommended

# PowerShell format and static analysis over the changed scripts
pwsh -NoProfile -Command "Invoke-Formatter -ScriptDefinition (Get-Content -Raw .codex/hooks/codex-pretooluse-file-mapping.ps1)"
pwsh -NoProfile -Command "Invoke-ScriptAnalyzer -Path .codex/hooks/,.claude/hooks/,.claude/lib/ -Recurse"

# Confirm the two rewritten modules import cleanly and the pure function behaves as documented
pwsh -NoProfile -Command "Import-Module ./.claude/lib/model-routing/ModelRouting.psm1 -Force; Get-ComplexityFloor -SignalsPresent @('docs_or_comment_only')"      # expect C1
pwsh -NoProfile -Command "Import-Module ./.claude/lib/model-routing/ModelRouting.psm1 -Force; Get-ComplexityFloor -SignalsPresent @('auth_or_token_handling')"    # expect C3
pwsh -NoProfile -Command "Import-Module ./.claude/lib/orchestrator-state/OrchestratorState.psm1 -Force; Get-Command -Module OrchestratorState"

# Confirm the new module is side-effect free when dot-sourced
pwsh -NoProfile -Command ". ./.codex/hooks/codex-pretooluse-file-mapping.ps1; \$LASTEXITCODE"

# Exercise an end-to-end orchestration run so the corrected completion gate is hit on a clean pass

Reviewers should additionally confirm that this repository has in fact migrated its TypeScript test runner to Jest, since the rule and agent files now assert that as policy.

Backward Compatibility / Migration Notes

  • Get-ComplexityFloor parameter semantics changed. The parameter is redefined from "names already filtered to "floor": true signals" to "the full recorded signals_present[] set." A caller that pre-filters is unaffected. A caller that previously passed non-floor or unknown names will now receive C1 where it previously received C3. This is the intended correction, but it changes recomputed floor values for any checkpoint whose signals_present[] contains only non-floor names.
  • Step-status vocabulary is additive, not replaced. VALID_STEP_STATUS is unchanged; the extra values are admitted only on step6_status and step9_status and remain rejected on all other step keys. Checkpoints that validated before continue to validate.
  • PR-creation readiness is now stricter. blocked_remediation_loop_limit on any of steps 5–8 newly fails readiness. A checkpoint carrying that status on those keys that previously passed will now be rejected.
  • Routing-gate behavior is now more permissive on the success path. A validator that exits 0 while emitting text on stderr will no longer block. Conversely, any failure signalled only through output text and not through a non-zero exit code would no longer be caught; the change's rationale comment asserts the validator always pairs errors with a non-zero exit.
  • Vitest references removed from policy text. .claude/rules/general-unit-test.md no longer lists vitest.config.ts as a permitted coverage exclude and now lists jest.config.cjs. Any repository configuration still named vitest.config.ts would fall outside the permitted exclude list.
  • No public C# API, project file, or build configuration is modified. No file is deleted or renamed.

Risks and Mitigations

Risk Assessment Mitigation
The new mapping module is not yet dot-sourced by any hook. A repository-wide search for codex-pretooluse-file-mapping returns no consumer. Confirmed by inspection. The module is dead code in this repository as merged; the duplicated, drifting per-handler logic it is meant to replace remains in effect. Low immediate risk, since an unreferenced entrypoint-free file cannot alter hook behavior. Track the consumer wiring as a follow-up; do not assume the Edit/Write exit-2 defect described in the module header is resolved by this PR alone.
Relaxing the routing gate to exit-code-only could mask a validator failure mode that reports errors without a non-zero exit. Depends on a validator invariant asserted in the rationale comment but not tested in this repository. Confirm against the validator implementation upstream. The prior behavior was unusable in practice because it blocked every clean pass, so the fix is net-positive regardless.
Hard-coded FLOOR_SIGNAL_NAMES can drift from config/orchestration-routing.json. Real but currently in parity, verified above. The pinning Pester test named in the comment does not exist in this repository. Re-verify the constant against the config whenever the signal catalog changes, or port the parity test into this repository.
Jest-versus-Vitest policy text may not match this repository's actual TypeScript tooling. Not verified; no package.json, jest.config.cjs, or vitest.config.ts was examined as part of this diff, and none appears in the changed-file set. Reviewer confirmation requested above.
Get-EpicPlanningCurrentBranch throws inside a helper rather than at the call site. Behavior-preserving by inspection: the same EPIC_PLANNING_ONLY_BLOCKED message is raised, and the caller's try block still catches it. Note one deliberate difference: whitespace-only output now returns '' instead of throwing, and the surrounding decision function receives that empty string. Review the decision function's handling of an empty LiveBranch / CurrentBranch value.

Rollback: the range is two commits on top of main at e63ddc7c with no schema, data, or build-output changes. git revert bfe83d4b da21214a, or reverting the merge commit, fully restores prior behavior. Reverting .codex/config.toml alone returns the MCP pin to 1.0.18.

Review Guide

Suggested order, highest-consequence first:

  1. .claude/hooks/validate-orchestrator-output.ps1 (20 lines) — the smallest diff with the largest behavioral consequence. Confirm the exit-code-only invariant.
  2. .claude/lib/model-routing/ModelRouting.psm1 (48 lines) — verify the new constant against config/orchestration-routing.json and confirm the filter runs before the empty-input guard.
  3. .claude/lib/orchestrator-state/OrchestratorState.psm1 (48 lines) — confirm the extra vocabulary is per-key and that the readiness tightening is intended.
  4. .codex/hooks/enforce-epic-planning-only.ps1 (28 lines) and .codex/hooks/enforce-epic-child-worktree-binding.ps1 (19 lines) — extract-method refactors; check the whitespace-only-branch behavior difference noted above.
  5. .codex/hooks/codex-pretooluse-file-mapping.ps1 (474 lines) — the largest diff but the lowest current risk, since nothing dot-sources it yet. Read the header contract first, then ConvertFrom-CodexPreToolUsePayload's four throw conditions, then the mapping function and its three internal helpers.
  6. .claude/agents/atomic-executor.md, .claude/rules/general-code-change.md, .claude/rules/general-unit-test.md, .codex/config.toml (12 lines combined) — mechanical string substitutions and a version pin.

Files 1–3 account for 116 of the 649 changed lines and carry essentially all of the behavioral change. File 5 accounts for 474 lines of purely additive, currently-unreferenced code.

Follow-ups

  • Wire enforce-checkpoint-monotonic.ps1 and enforce-completion-consistency.ps1 to dot-source codex-pretooluse-file-mapping.ps1 and remove their duplicated transport and mapping logic. Until then the module is unreferenced and the drift it addresses persists.
  • Port the config-parity Pester test that pins FLOOR_SIGNAL_NAMES to config/orchestration-routing.json, and the parity suite asserting the mapping module reads no legacy environment variable, into this repository's tests/ tree. Both are referenced in the changed files' documentation but absent here.
  • Establish a Pester suite covering .claude/lib/** and .claude/hooks/** so future push-downs of these files are gated by a local test run rather than accepted on inspection.
  • Confirm and, if needed, complete the repository's Vitest-to-Jest migration so the updated rule text matches the actual toolchain.
  • Populate the PR Intent fields in artifacts/pr_context.appendix.txt before the next push-down PR, so the "Why" section can cite stated intent rather than inferring it from the diff.

GitHub Auto-close

  • None (no verified closing issues; GitHub CLI reported unavailable during PR-context collection, and no author-asserted autoclose issues were provided)

@drmoisan drmoisan changed the title Chore/update agents chore: sync Claude/Codex tooling to drm-copilot v1.0.20 (routing-gate and complexity-floor fixes) Jul 31, 2026
@drmoisan
drmoisan merged commit 681a00b into main Jul 31, 2026
2 checks passed
@drmoisan
drmoisan deleted the chore/update-agents branch July 31, 2026 19:29
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.

1 participant