Compare commits
3 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| bacb25e029 | |||
| 92efd1af2b | |||
| 7adb296523 |
@@ -61,6 +61,13 @@ func validateDocmapPath(localPath, resolvedRoot string) error {
|
||||
return fmt.Errorf("symlinks are not allowed")
|
||||
}
|
||||
|
||||
// Reject anything that is not a regular file (directories, FIFOs, device
|
||||
// nodes, etc.) — ParseDocMapConfig expects a plain YAML file and would
|
||||
// produce a confusing error on non-regular entries.
|
||||
if !fi.Mode().IsRegular() {
|
||||
return fmt.Errorf("docmap must be a regular file")
|
||||
}
|
||||
|
||||
// Confine to resolvedRoot: use the fully-resolved path so that a directory
|
||||
// symlink inside the repo cannot carry the path outside the root.
|
||||
rel, err := filepath.Rel(resolvedRoot, resolvedPath)
|
||||
@@ -171,6 +178,9 @@ func runValidateDocmap(args []string) int {
|
||||
// Normalize Windows-style backslashes to forward slashes so that
|
||||
// changed-file paths from git on Windows match doc-map globs.
|
||||
f = strings.ReplaceAll(f, "\\", "/")
|
||||
// Strip a leading "./" emitted by non-git tools (e.g. `find`) so that
|
||||
// paths like "./cmd/foo.go" match doc-map globs written as "cmd/**".
|
||||
f = strings.TrimPrefix(f, "./")
|
||||
if !review.FileCoveredByDocMap(cfg, f) {
|
||||
uncovered = append(uncovered, f)
|
||||
}
|
||||
@@ -189,7 +199,7 @@ func runValidateDocmap(args []string) int {
|
||||
staleDocs := checkStaleDocs(cfg, resolvedRoot)
|
||||
if len(staleDocs) > 0 {
|
||||
failed = true
|
||||
fmt.Fprintln(errWriter, "ERROR: stale docmap docs: entries (paths do not exist):")
|
||||
fmt.Fprintln(errWriter, "ERROR: stale docmap entries (paths do not exist):")
|
||||
for _, d := range staleDocs {
|
||||
fmt.Fprintf(errWriter, " %s\n", d)
|
||||
}
|
||||
|
||||
@@ -599,3 +599,53 @@ func TestValidateDocmapPath_DirSymlinkBypass(t *testing.T) {
|
||||
t.Error("expected rejection of dir-symlink bypass, got nil error")
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateDocmapPath_NonRegularFile verifies that --docmap pointing at a
|
||||
// non-regular file (e.g. a directory) is rejected with a clear error before
|
||||
// ParseDocMapConfig is called.
|
||||
func TestValidateDocmapPath_NonRegularFile(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
|
||||
// Use the directory itself as the docmap path — directories pass Lstat but
|
||||
// are not regular files.
|
||||
reviewBotDir := filepath.Join(dir, ".review-bot")
|
||||
if err := os.MkdirAll(reviewBotDir, 0o755); err != nil {
|
||||
t.Fatalf("MkdirAll: %v", err)
|
||||
}
|
||||
|
||||
code, _, stderr := stdinValidateDocmap(t,
|
||||
"",
|
||||
[]string{"--docmap", reviewBotDir, "--repo-root", dir},
|
||||
)
|
||||
if code != 2 {
|
||||
t.Errorf("expected exit 2 for directory docmap, got %d; stderr: %q", code, stderr)
|
||||
}
|
||||
if !strings.Contains(stderr, "regular file") && !strings.Contains(stderr, "invalid") {
|
||||
t.Errorf("expected regular-file rejection in stderr, got %q", stderr)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRunValidateDocmap_DotSlashPrefix verifies that paths emitted with a
|
||||
// leading "./" (e.g. from `find` or `ls`) match doc-map globs correctly.
|
||||
// Without TrimPrefix, "./cmd/foo.go" would not match the pattern "cmd/**".
|
||||
func TestRunValidateDocmap_DotSlashPrefix(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
makeDocFile(t, dir, "docs/foo.md")
|
||||
|
||||
docmap := makeDocmapInDir(t, dir, `
|
||||
mappings:
|
||||
- paths:
|
||||
- "cmd/**"
|
||||
docs:
|
||||
- docs/foo.md
|
||||
`)
|
||||
|
||||
// File with a leading "./" should be treated as covered.
|
||||
code, _, stderr := stdinValidateDocmap(t,
|
||||
"./cmd/foo.go\n",
|
||||
[]string{"--docmap", docmap, "--repo-root", dir},
|
||||
)
|
||||
if code != 0 {
|
||||
t.Errorf("expected exit 0 for './' prefixed covered file, got %d; stderr: %q", code, stderr)
|
||||
}
|
||||
}
|
||||
|
||||
+1
-24
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user