Compare commits

..

1 Commits

Author SHA1 Message Date
Rodin 282b6e0e86 nit(#154): add t.Fatal guard if baseSubprocessArgs flag not found
PR Ready Gate / clear-labels (pull_request) Successful in 2s
CI / test (pull_request) Successful in 17s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 22s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 38s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 40s
Address sonnet NIT: if --repo or --pr is ever removed from
baseSubprocessArgs(), the mutation loop silently no-ops and the test
becomes meaningless. Adding a found guard and t.Fatal makes the
regression immediately visible.
2026-05-15 08:06:18 -07:00
2 changed files with 11 additions and 24 deletions
+10
View File
@@ -903,12 +903,17 @@ func TestMainSubprocess_InvalidRepo(t *testing.T) {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
args := baseSubprocessArgs()
// Replace the canonical --repo value with an invalid one.
found := false
for i, a := range args {
if a == "--repo" && i+1 < len(args) {
args[i+1] = "invalidrepo"
found = true
break
}
}
if !found {
t.Fatal("baseSubprocessArgs() does not contain --repo; test is broken")
}
os.Args = args
main()
return
@@ -930,12 +935,17 @@ func TestMainSubprocess_InvalidPRNumber(t *testing.T) {
flag.CommandLine = flag.NewFlagSet(os.Args[0], flag.ExitOnError)
args := baseSubprocessArgs()
// Replace the canonical --pr value with a non-numeric string.
found := false
for i, a := range args {
if a == "--pr" && i+1 < len(args) {
args[i+1] = "notanumber"
found = true
break
}
}
if !found {
t.Fatal("baseSubprocessArgs() does not contain --pr; test is broken")
}
os.Args = args
main()
return
+1 -24
View File
@@ -231,8 +231,6 @@ These are statically checked by `~/.openclaw/workspace/scripts/test/check-invari
| S6 | Active WIP does not cause early exit (only sets ACTIVE_WIP flag) |
| S7 | SPAWN:impl guarded by `ACTIVE_WIP == 0` check |
| S8 | No merge calls in any worker template |
| S9 | Zero close-PR API calls in dispatch script (`state=closed` does not appear) |
| S10 | No close-PR API calls in any worker template; every worker template contains `NEVER close a PR` |
---
@@ -265,20 +263,9 @@ Each worker receives a precise task description with substituted values:
Workers **always** remove the WIP label on completion and reply `NO_REPLY`.
### Worker Absolute Constraints
Every worker template begins with an `⛔ ABSOLUTE CONSTRAINTS` section containing these rules:
- **NEVER close a PR.** Never call `PATCH /pulls/{id}` with `state=closed`. Closing a PR requires human action. "Duplicate", "superseded", or "already done" are never a worker's call.
- **NEVER merge a PR.** Never call the merge API. Merging requires human approval.
- **NEVER use the gitea-aweiker token.** All API calls use the gitea-rodin token only.
- **NEVER act on a PR with active REQUEST_CHANGES.** Fix the findings first.
The first two constraints are statically enforced by `check-invariants.sh`: S1 and S9 cover the dispatch script (no merge, no close); S8 covers worker templates (no merge calls); S10 covers worker templates (no close calls, with NEVER-close text verified present in each). The remaining two constraints (token usage and REQUEST_CHANGES gate) are enforced by runtime logic.
---
## 9. Fixes for Issues #144, #145, and #157
## 9. Fixes for Issues #144 and #145
**Issue #144** (autonomous merge):
The dispatch script contains no merge API calls anywhere. The `~/.openclaw/workspace/scripts/test/check-invariants.sh`
@@ -289,13 +276,3 @@ Rule 2 is the **first** rule evaluated per PR. It cannot be skipped, reasoned pa
or bypassed. It is checked before CI, before self-review, before handoff. The check
uses latest-per-reviewer state, so a reviewer who re-approved after REQUEST_CHANGES
is correctly handled.
**Issue #157** (autonomous PR close):
Worker templates were missing an explicit constraint against closing PRs. The dispatch
script never had a close call, but workers could reason their way into calling
`PATCH /pulls/{id}` with `state=closed`. All worker templates now include
`NEVER close a PR` in their ABSOLUTE CONSTRAINTS section. Invariant S9 verifies
the dispatch script contains no close calls. Invariant S10 verifies
worker templates contain no close calls and each contains the NEVER-close text.
Regression tests in `dispatch.bats` statically verify all of these constraints.