chore: sync Claude/Codex tooling to drm-copilot v1.0.20 (routing-gate and complexity-floor fixes) - #414
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.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).Invoke-RoutingContractValidationno longer treats any captured validator output as a failure, because the invoker captures with2>&1and the validator prints its success line to stdout on a clean pass. The process exit code is now the sole discriminator.ModelRouting.psm1:Get-ComplexityFloornow intersects the recorded signal names against an explicit floor-signal set instead of treating every supplied name as a triggered floor signal.STEP_SPECIFIC_EXTRA_STATUS), so remediation-loop and CI-loop statuses validate only on the step keys that own them, andblocked_remediation_loop_limitnow blocks PR-creation readiness on steps 5–8..codex/hooks/codex-pretooluse-file-mapping.ps1, a shared, entrypoint-free transport module for CodexPreToolUsepayload parsing andtool_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 everyEditandWritepayload to exit 2.atomic-executoragent tool allowlist and the two general rule files.@danmoisan/drm-copilot-mcp@1.0.18to@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.($exitCode -ne 0) -or (-not [string]::IsNullOrWhiteSpace($outputText)). Since the default invoker captures combined output, the validator's own success line populated$outputTexton a clean pass, so the gate reported a routing-contract failure whenever the Python CLI succeeded. This blockedDONEon passing checkpoints.SignalsPresentdown to"floor": truecatalog names. Callers instead pass the full checkpointsignals_present[]array, so"floor": falsenames 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.VALID_STEP_STATUSlist 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.^(apply_patch|Edit|Write)$matcher. The two named consumers,enforce-checkpoint-monotonic.ps1(303 lines) andenforce-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).claude/hooks/validate-orchestrator-output.ps1$hasErrorsreduced to($exitCode -ne 0); doc comment rewritten to state thatErrorTextnow carries the success line on a clean pass..claude/lib/model-routing/ModelRouting.psm1$script:FLOOR_SIGNAL_NAMESconstant (four names);Get-ComplexityFloorfilters 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$script:STEP_SPECIFIC_EXTRA_STATUShashtable (step6_status→blocked_remediation_loop_limit;step9_status→passed,failed_remediation_required,blocked_ci_loop_limit); base presence check consults the per-key extra set; PR-creation readiness rejectsblocked_remediation_loop_limitalongsidependingandblocked..claude/agents/atomic-executor.mdBash(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.claude/rules/general-unit-test.mdvitest.config.ts→jest.config.cjs; fake-timer guidancevi.useFakeTimers()→jest.useFakeTimers().Codex governance surface (commit
bfe83d4b).codex/hooks/codex-pretooluse-file-mapping.ps1ConvertFrom-CodexPreToolUsePayloadandConvertTo-CodexFileEditInput;ConvertTo-CodexAddedLineText,Test-CodexGovernedPath, andResolve-CodexUpdatedFileContentare documented as internal helpers. Carries theapply_patchfile, move, and hunk regexes. Performs no policy evaluation..codex/hooks/enforce-epic-child-worktree-binding.ps1Get-CodexChildGuardLiveBranch, which returns''on non-zero exit or whitespace-only output and trims inside the helper..codex/hooks/enforce-epic-planning-only.ps1Invoke-EpicPlanningGitandGet-EpicPlanningCurrentBranch; theEPIC_PLANNING_ONLY_BLOCKEDthrow on unresolvable branch moves into the helper..codex/config.toml1.0.18→1.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
.claude/hooks/validate-orchestrator-output.ps1invokes the routing-contract validator as a child process and converts the result into a hook decision. The change alters only the failure discriminator insideInvoke-RoutingContractValidation; the returned hashtable shape (HasErrors,ErrorText) and theMODEL_ROUTING_BLOCKED:block-reason string are unchanged, so every existing caller is unaffected.Get-ComplexityFlooris a pure function consumed by the orchestrator when writingcomplexity_assessments[].floor. Per.claude/rules/orchestrator-state.md, the checkpoint validator requiresfloor == 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 fromconfig/orchestration-routing.jsonbecause the module is pushed down to consumer repositories that do not ship that config.OrchestratorState.psm1mirrors the base checkpoint-presence checks (required keys, step-status validity,blocked_reasonvalidity) that run before any mode-specific gate. The additive vocabulary is applied inside the existingSTEP_STATUS_KEYSloop, so an absent step key still contributes no error..codex/config.tomlregisters a list ofPreToolUsehandlers undermatcher = "^(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 theenforce-completion-helpers.ps1precedent. Reading the payload remains each calling hook's responsibility.Verification
Completed
$script:FLOOR_SIGNAL_NAMES—classifier_or_model_logic,auth_or_token_handling,concurrency_or_ordering,cross_module_contract_change— were compared againstconfig/orchestration-routing.jsonand match exactly the entries flagged"floor": true. The three"floor": falsenames (single_file_localized_edit,mechanical_rename_or_move,docs_or_comment_only) are correctly absent from the constant.enforce-checkpoint-monotonic.ps1andenforce-completion-consistency.ps1are both registered in.codex/config.tomlunder the^(apply_patch|Edit|Write)$matcher.Not verified in this PR
HEADis reported as not available..claude/lib/**or.codex/hooks/**trees; the only*.Tests.ps1files present are undertests/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.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.Recommended
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-ComplexityFloorparameter semantics changed. The parameter is redefined from "names already filtered to"floor": truesignals" to "the full recordedsignals_present[]set." A caller that pre-filters is unaffected. A caller that previously passed non-floor or unknown names will now receiveC1where it previously receivedC3. This is the intended correction, but it changes recomputed floor values for any checkpoint whosesignals_present[]contains only non-floor names.VALID_STEP_STATUSis unchanged; the extra values are admitted only onstep6_statusandstep9_statusand remain rejected on all other step keys. Checkpoints that validated before continue to validate.blocked_remediation_loop_limiton any of steps 5–8 newly fails readiness. A checkpoint carrying that status on those keys that previously passed will now be rejected..claude/rules/general-unit-test.mdno longer listsvitest.config.tsas a permitted coverage exclude and now listsjest.config.cjs. Any repository configuration still namedvitest.config.tswould fall outside the permitted exclude list.Risks and Mitigations
codex-pretooluse-file-mappingreturns no consumer.Edit/Writeexit-2 defect described in the module header is resolved by this PR alone.FLOOR_SIGNAL_NAMEScan drift fromconfig/orchestration-routing.json.package.json,jest.config.cjs, orvitest.config.tswas examined as part of this diff, and none appears in the changed-file set.Get-EpicPlanningCurrentBranchthrows inside a helper rather than at the call site.EPIC_PLANNING_ONLY_BLOCKEDmessage is raised, and the caller'stryblock still catches it. Note one deliberate difference: whitespace-only output now returns''instead of throwing, and the surrounding decision function receives that empty string.LiveBranch/CurrentBranchvalue.Rollback: the range is two commits on top of
mainate63ddc7cwith no schema, data, or build-output changes.git revert bfe83d4b da21214a, or reverting the merge commit, fully restores prior behavior. Reverting.codex/config.tomlalone returns the MCP pin to1.0.18.Review Guide
Suggested order, highest-consequence first:
.claude/hooks/validate-orchestrator-output.ps1(20 lines) — the smallest diff with the largest behavioral consequence. Confirm the exit-code-only invariant..claude/lib/model-routing/ModelRouting.psm1(48 lines) — verify the new constant againstconfig/orchestration-routing.jsonand confirm the filter runs before the empty-input guard..claude/lib/orchestrator-state/OrchestratorState.psm1(48 lines) — confirm the extra vocabulary is per-key and that the readiness tightening is intended..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..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, thenConvertFrom-CodexPreToolUsePayload's four throw conditions, then the mapping function and its three internal helpers..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
enforce-checkpoint-monotonic.ps1andenforce-completion-consistency.ps1to dot-sourcecodex-pretooluse-file-mapping.ps1and remove their duplicated transport and mapping logic. Until then the module is unreferenced and the drift it addresses persists.FLOOR_SIGNAL_NAMEStoconfig/orchestration-routing.json, and the parity suite asserting the mapping module reads no legacy environment variable, into this repository'stests/tree. Both are referenced in the changed files' documentation but absent here..claude/lib/**and.claude/hooks/**so future push-downs of these files are gated by a local test run rather than accepted on inspection.artifacts/pr_context.appendix.txtbefore the next push-down PR, so the "Why" section can cite stated intent rather than inferring it from the diff.GitHub Auto-close