fix(milestones): tighten verify_fix_deployed role guard to DEPLOYER-only - #2147
Conversation
- Rename _assert_deployer_or_vendor_role → _assert_deployer_role; require only CVDRole.DEPLOYER (drop VENDOR fallback per CSB-15-002) - Update verify_fix_deployed docstring to reflect DEPLOYER-only requirement - Update DEMOMA-15-001: drop VENDOR-or-DEPLOYER language, state DEPLOYER-only - Update DEMOMA-19-004 rationale to remove 'requires VENDOR or DEPLOYER' - Update test_milestones_vendor_guard: VENDOR-only now expects AssertionError; class renamed TestVerifyFixDeployedDeployerGuard; assertions check DEPLOYER Closes #1785 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rify it applies to DEPLOYER actors only Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
sei-ahouseholder
left a comment
sei-ahouseholder
left a comment
There was a problem hiding this comment.
PR Triage: #2147 — fix(milestones): tighten verify_fix_deployed role guard to DEPLOYER-only
Linked issues: #1785 (Rename verify_fix_deployed() to clarify it applies to DEPLOYER actors only), #2148 (fix(milestones): tighten verify_fix_deployed role guard to DEPLOYER-only)
Changed files: 4 files — demo, specs
CI status: ❌ failing (pre-existing — see notes below)
Merge state: ✅ MERGEABLE (UNSTABLE)
Base branch: main
Needs integration tests: yes (modifies vultron/demo/)
Findings
| # | Phase | Severity | Description | Outcome |
|---|---|---|---|---|
| phase2-missing-closes-2148-0 | issue-linkage | All 5 ACs from #2148 are satisfied in this diff; Closes #2148 absent from PR body |
fix-now | |
| phase3-specs-notes-label-and-verification-0 | pr-body-format | PR labeled specs-notes (docs-only) but modifies .py files; missing Verification section |
fix-now |
Total: 0 FAIL · 2 IMPROVE · 0 NEW-ISSUE
CI Note
All 5 failing demo jobs (fcvcv Demo Integration, fvcv-handoff Demo Integration, fcvcv Invariant Harness, fvcv-handoff Invariant Harness, fcv-reject Invariant Harness) are pre-existing failures tracked under #2233 — not caused by this PR. Core Python tests, all linters (black/flake8/mypy/pyright), and Spec Lint all pass ✅.
Code Review Note
Logic change is correct and well-scoped. The _assert_deployer_role check (CVDRole.DEPLOYER not in (participant.case_roles or [])) is simpler and more precise than the previous set-intersection. The sole verify_fix_deployed callsite in fcvcv_demo.py:807 passes v2.id_ (V2 = VENDOR+DEPLOYER), so no callsite breakage.
Triage artifact: .claude/pr-2147-triage.json
Next step: run /pr-execute or /pr-ship to apply fixes.
…fy-fix-deployed-deployer
sei-ahouseholder
left a comment
sei-ahouseholder
left a comment
There was a problem hiding this comment.
PR Execute: #2147
Branch synced: merged origin/main (227 commits) cleanly — no conflicts
Tests: 7015 passed · 2 pre-existing failures (#2274) · 5 xfailed (all issues open) · integration 1127 passed ✅
Merge state: ✅ MERGEABLE (UNSTABLE — pre-existing CI only)
Findings Applied
| Finding | Action | Result |
|---|---|---|
| phase2-missing-closes-2148-0 | Added - Closes #2148 to PR body |
✅ fixed |
| phase3-specs-notes-label-and-verification-0 | Removed specs-notes label; added Verification section with test counts and AC tick-offs |
✅ fixed |
No code changes required
Both findings were PR metadata corrections (body + label). No files were edited, no commit was needed.
Pre-existing failures (not caused by this PR)
- Unit:
test_invariant_5_expected_event_types_present[validate_report]and[engage_case]→ #2274 - Demo CI: fcvcv, fvcv-handoff, fcv-reject jobs → #2233
Execute artifact: .claude/pr-2147-execute.json
Next step: run /pr-verify or /pr-ship to complete the pipeline.
sei-ahouseholder
left a comment
sei-ahouseholder
left a comment
There was a problem hiding this comment.
PR Verify: #2147 — fix(milestones): tighten verify_fix_deployed role guard to DEPLOYER-only
Overall verdict: ⏳ PENDING-CI
Merge state: ✅ MERGEABLE (UNSTABLE → improving)
Base branch: main
Per-Finding Verdicts
| Finding | Severity | Outcome | Verdict |
|---|---|---|---|
| phase2-missing-closes-2148-0 | fixed | ✅ CONFIRMED — - Closes #2148 present at top of PR body |
|
| phase3-specs-notes-label-and-verification-0 | fixed | ✅ CONFIRMED — specs-notes label removed; Verification section with test counts and 5 AC tick-offs present |
All findings confirmed. No gaps.
CI Status
| Job | Status |
|---|---|
| Tests (pytest) | ✅ pass |
| Lint (black / flake8 / mypy / pyright) | ✅ pass |
| Spec Lint | ✅ pass |
| Build | ✅ pass |
| CodeQL / Analyze | ✅ pass |
| fv Demo Integration | ✅ pass |
| fcvcv Demo Integration | ✅ pass (was failing before sync) |
| fvcv-handoff Demo Integration | ✅ pass (was failing before sync) |
| fcv-reject Demo Integration | ✅ pass (was failing before sync) |
| fv Invariant Harness | ⏳ pending |
| fcvcv Invariant Harness | ⏳ pending |
| fvcv-handoff Invariant Harness | ⏳ pending |
| fcv-reject Invariant Harness | ⏳ pending |
The merge with origin/main resolved the previously failing demo CI jobs — all four Demo Integration scenarios that were broken under #2233 are now passing. The 4 Invariant Harness jobs are running downstream and expected to pass (their Demo Integration prerequisites are green).
Pre-existing test failures (not caused by this PR)
test_invariant_5_expected_event_types_present[validate_report]and[engage_case]→ #2274 (tracked, open)
Verdict: PENDING-CI — re-run /pr-verify after the 4 Invariant Harness jobs complete to get the final verdict and clean up artifacts.
2 participants
Summary
Tightens the
verify_fix_deployedrole guard to require onlyCVDRole.DEPLOYER, removing the misleadingCVDRole.VENDORfallback.Updates
DEMOMA-15-001andDEMOMA-19-004to reflect the correctedprecondition. Updates tests so a VENDOR-only caller now correctly
raises
AssertionError.Changes
vultron/demo/helpers/milestones.py: Rename_assert_deployer_or_vendor_role→_assert_deployer_role; requireonly
CVDRole.DEPLOYER. Updateverify_fix_deployeddocstring.specs/multi-actor-demo.yaml: UpdateDEMOMA-15-001to stateDEPLOYER-only requirement; update
DEMOMA-19-004rationale to drop"requires VENDOR or DEPLOYER" language.
test/demo/test_milestones_vendor_guard.py: Rename class toTestVerifyFixDeployedDeployerGuard; update docstring; addtest_vendor_only_raises_assertionerrortest; fix assertions tocheck
CVDRole.DEPLOYERin error messages.Verification
test_vendor_only_raises_assertionerror)verify_fix_deployedguard accepts ONLYCVDRole.DEPLOYER; VENDOR-only raisesAssertionError_assert_deployer_or_vendor_rolerenamed to_assert_deployer_role; checks onlyCVDRole.DEPLOYERtest_milestones_vendor_guard.pyupdated; VENDOR-only expectsAssertionError; DEPLOYER-only passesDEMOMA-15-001states DEPLOYER-only requirementDEMOMA-19-004rationale updated to drop "requires VENDOR or DEPLOYER"