Skip to content

fix(case): enforce bindings_v2 parity before validation#2128

Merged
song-zhao-25 merged 2 commits into
mainfrom
fix/maestro-case-bindings-v2-guard
Jul 20, 2026
Merged

fix(case): enforce bindings_v2 parity before validation#2128
song-zhao-25 merged 2 commits into
mainfrom
fix/maestro-case-bindings-v2-guard

Conversation

@song-zhao-25

@song-zhao-25 song-zhao-25 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add a Step 12 check that compares bindings_v2.json.resources[] with the complete projection of caseplan.json.bindings[]
  • if they differ, regenerate the full sidecar once and re-check; halt if parity still fails
  • require the same check before every uip solution resources refresh
  • align phased-execution.md (the Phase 3 execution-order contract) with the new check: "Checks 1-6 … Never HALT" would have steered agents past Check 7 entirely

Why

The remote CM golden expense run had populated caseplan bindings but an empty bindings_v2.json.resources[]. The existing regeneration recipe was skipped, and no terminal validation prevented the run from continuing.

Since SKILL.md Rule 11 designates phased-execution.md as the full phase contract, its validator line also had to enumerate Check 7 and carve out its halt-on-persistent-mismatch — otherwise the sole enforcement point for build-only runs stays unreachable.

Validation

  • skill frontmatter and structure validation passed
  • skill-description validation passed
  • CM golden expense checker tests passed (4 tests)
  • regeneration simulated against the failed run's own caseplan.json: projected sidecar carries all 6 expected resource keys + both connection IDs, satisfying every remaining check_cm_golden_bindings.py assertion

This PR changes three documentation files: 5 insertions and 4 deletions.

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Claude finished @song-zhao-25's task in 2m 46s —— View job


PR Review: fix(case): enforce bindings_v2 parity before validation

  • Read review criteria and project conventions
  • Get full diff
  • Read all changed files in full
  • Check relative links resolve
  • Check for cross-skill references
  • Check for secrets/tokens/paths
  • Post review findings

Summary

This PR adds a mandatory semantic parity check ("Check 7") between caseplan.json.bindings[] and bindings_v2.json.resources[], wired into Rule 14, Phase 2 publish, Phase 3 exit, Phase 4 entry, Phase 5 debug, and Phase 6 publish. It also clarifies that uip solution resources refresh is a consumer (not a generator) of the sidecar, and enables TodoWrite in the CM golden expense eval task. Motivated by a remote eval run where an empty bindings_v2.json slipped through all existing checkpoints.

Change-by-Change Review

1. skills/uipath-maestro-case/SKILL.md (+6/−4)

OK

Rule 14 rewritten from a simple "always run refresh before upload/debug" to a multi-step parity guard with a clear halt condition. The Phase 2 outline adds Step 9.4 and renumbers 7→8→9. Phase 3 adds item 6 (Check 7). Phase 4 entry now gates on parity. All changes are internally consistent and reference the correct anchors in bindings-v2-sync.md.

2. skills/uipath-maestro-case/references/bindings-v2-sync.md (+32/−4)

OK

Core new section: ## Check 7 — Bindings sidecar parity (Step 12). Well-structured 5-step mechanical procedure with clear halt semantics, an explicit empty-bindings edge case, and a reporting template. The direction-is-one-way callout at the top is a good addition that directly addresses the root cause. The § prefix was removed from the ## Regenerate bindings_v2.json heading — the Check 7 anchor links to #regenerate-bindings_v2json which matches.

3. skills/uipath-maestro-case/references/brownfield.md (+2/−2)

OK

After-edit step 2 now routes through Check 7 before refresh. Completion output adds "binding-sidecar parity status when bindings changed". Both are consistent with the new Rule 14.

4. skills/uipath-maestro-case/references/case-commands.md (+3/−1)

OK

Adds a "Consumer, not generator" callout to the resources refresh section and tightens the "empty scaffold" warning to halt-and-repair instead of silent acceptance. Both directly address the failure mode described in the PR body.

5. skills/uipath-maestro-case/references/connector-trigger-common.md (+1/−1)

OK

Updates the deferred-batched callout to mention "three mutation points" and the mandatory Step 12 parity reconciliation, with a link to Check 7. Consistent with the other files.

6. skills/uipath-maestro-case/references/implementation.md (+15/−5)

OK

Step 9.6 Phase 3 todos now include item 6 (Check 7). Step 10 end-note updated to reference Check 7 as terminal reconciliation. End-of-Phase-3 validator expanded from Checks 1-6 to Checks 1-7. Check 7 described as "non-interactive and blocking" with a clear "no best-effort branch" statement. Reporting section adds the Bindings parity: PASS line template. Phase 3→4 transition now gated on Check 7. Phase 4 bridge sentence adds Check 7 as a precondition. All internally consistent.

7. skills/uipath-maestro-case/references/phased-execution.md (+13/−12)

OK

Phase summary table updated: Phase 2 output now includes bindings_v2.json, Phase 3 output includes "semantic parity". Phase 2 Publish for review gates on Rule 14 parity. Phase 3 execution order item 6 updated to Checks 1-7. Phase 4 entry gated. Phase 5 debug and Phase 6 publish both gate on Rule 14 parity before refresh. Report fields renumbered with new item 4 (binding-sidecar status). Debug and publish notes clarify the consumer-not-generator semantics. All consistent.

8. skills/uipath-maestro-case/references/plugins/variables/bindings/impl-json.md (+1/−1)

OK

Sync section updated to mention three mutation points (was two) and Step 12 Check 7. Consistent with bindings-v2-sync.md.

9. tests/tasks/uipath-maestro-case/golden_rebuild/cm_golden_expense/cm_golden_expense.yaml (+4/−3)

OK

Adds TodoWrite to allowed_tools. Comment updated to explain the reason (preserve mandatory phase work across compaction). The tool extends the experiment default's allowed_tools as documented. No other YAML structural changes.

What's Missing

Nothing identified.

Area Ratings

Area Status Notes
Frontmatter OK name matches folder, description unchanged and under 1024 chars
E2E Tests OK Existing test updated; no new untested capabilities
Skill Body OK Rule 14 rewritten cleanly; phase outlines updated consistently
References & Assets OK All 6 reference files updated in lockstep; Check 7 section well-structured; all relative links resolve
Repo Hygiene OK No cross-skill references, no secrets, no unrelated changes, CODEOWNERS already covers this path

Issues for Manual Review

  • Domain accuracy of Check 7 semantics: The review cannot verify that the 5-step comparison procedure (version check, resource-key cardinality, value/metadata shape match) is correct against the actual CLI behavior of uip solution resources refresh. The procedure is internally consistent with the existing § Regenerate section, but the semantic comparison rules (step 3) should be validated against a real build.
  • Step 9.4 timing: The PR references "Step 9.4" for Phase 2 bindings_v2 regeneration. This step exists in implementation.md and appears to be correctly placed, but the reviewer cannot verify it runs at the right point in the build sequence relative to all task-type bindings being written.

Conclusion

Clean, well-scoped PR. The change is internally consistent across all 9 files — every reference to Rule 14, Check 7, and the parity guard points to the same anchors and uses the same semantics. The new Check 7 section in bindings-v2-sync.md is well-structured with clear halt conditions and no ambiguity. The TodoWrite addition to the test task is justified by the comment. No issues found — approve.

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Claude finished @song-zhao-25's task in 2m 22s —— View job


Coder-eval task lint (advisory)

1 task YAML changed; verdicts: 0 Critical, 0 High, 0 Medium, 0 Low, 1 OK. One PR-level note on passing-run evidence.

Rubric: .claude/commands/lint-task.md. This check is advisory and never blocks merge.

Evidence of passing run

⚠️ Medium — PR body states "CM golden expense checker tests passed (4 tests)" (the 4 grading scripts validate against a known-good artifact) and "The full remote coder eval was not rerun as part of this local change." The checker-test claim covers the grading harness but not the task itself. Given the change is narrow (adding TodoWrite to allowed_tools — strictly capability-additive, no criteria or prompt change), this is low-risk; consider adding a note like: "TodoWrite-only change; prior passing run still valid." to make the claim explicit.

Per-task lint

tests/tasks/uipath-maestro-case/golden_rebuild/cm_golden_expense/cm_golden_expense.yaml — verdict: OK

The diff adds "TodoWrite" to agent.allowed_tools (line 41) and updates the comment block above it (lines 34–36). No criteria, prompt, sandbox, or structural changes.

  • Self-report anti-pattern: Not triggered — no agent-written report file in prompt or criteria.
  • Prompt over-specification: Clean — prompt states the goal and constraints without prescribing procedure.
  • Meaningful coverage: Strong — 5 run_command criteria with external Python checkers (topology, bindings, semantics, seed) plus a command_not_executed guard. Weights are well-distributed (1.0–5.0).
  • Could pass for the wrong reason: Not triggered — all criteria run external validation scripts against the generated caseplan.json; no self-grading path.
  • Near-duplicate: No overlap — nearest sibling (expense_runnable_e2e.yaml) builds a different scenario, uses different resources, and runs debug end-to-end. This task stops at validate with pinned CM-Golden IDs.
  • Validate-only flow tests: Not applicable (skill is uipath-maestro-case, not uipath-maestro-flow).
  • CLI verb reachability: Could not run scripts/check-cli-verbs.py (sandbox permission). Manual inspection: maestro case validate, maestro case debug, maestro case publish are valid catalog verbs. No issue apparent.
  • Redundant or pinned uip CLI in sandbox: No env_packages present. Clean.
  • Run-limit fields under agent: max_turns, task_timeout, turn_timeout are correctly under run_limits:, not agent:. Clean.

Within-PR duplicates

Not applicable — only one task YAML changed.

Conclusion

✅ Changed task passes the rubric. Passing-run evidence is partial (checker tests confirmed, full eval explicitly not rerun) — the change is capability-additive only, so risk is minimal. Consider a brief PR-body note to close the loop.


@song-zhao-25
song-zhao-25 force-pushed the fix/maestro-case-bindings-v2-guard branch from d90f5be to 369c050 Compare July 20, 2026 17:35
phased-execution.md still enumerated the end-of-Phase-3 validator as
Checks 1-6 with an unconditional 'Never HALT' — an agent driving Phase 3
from that list would never run the new bindings-sidecar parity check,
leaving bindings_v2.json empty exactly as in the failed CM-Golden run.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@song-zhao-25
song-zhao-25 merged commit f0c9b3e into main Jul 20, 2026
20 checks passed
@song-zhao-25
song-zhao-25 deleted the fix/maestro-case-bindings-v2-guard branch July 20, 2026 18:53
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.

2 participants