Compare commits

..

5 Commits

Author SHA1 Message Date
Rodin ec6fdbff42 fix(#158): address bot feedback — correct S8/S10 description, fix §9 prose break
PR Ready Gate / clear-labels (pull_request) Successful in 1s
CI / test (pull_request) Successful in 17s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 24s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 55s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m0s
2026-05-15 15:40:53 -07:00
Rodin f883f39dbf fix(#158): address NIT feedback — clarify enforcement split, clean §9 prose
PR Ready Gate / clear-labels (pull_request) Successful in 2s
CI / test (pull_request) Successful in 18s
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 24s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m7s
2026-05-15 11:06:49 -07:00
Rodin fb7d8d5e3b fix(#158): add S10 invariant to spec, fix enforcement wording in §8
PR Ready Gate / clear-labels (pull_request) Successful in 1s
CI / test (pull_request) Successful in 17s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 23s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 26s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 49s
Address MINOR and NIT findings from Sonnet and GPT review of PR #158.

MINOR (Sonnet + GPT): No static invariant for 'no close-PR in worker templates'.
- Add S10 to §6 Safety Invariants table: checks that no worker template contains
  close-PR API calls AND every template contains NEVER-close constraint text.
- Symmetric to S8 (no merge in worker templates) and S9 (no close in dispatch).

NIT (GPT): Enforcement mapping sentence in §8 was ambiguous.
- Rewrite to explicitly map: S1+S9 cover dispatch; S8+S10 cover worker templates.

NIT (Sonnet): The 'all 7 templates contain NEVER-close text' claim is now verified
by S10 (grep-based), not just prose.

Implementation: S10 added to check-invariants.sh + Bug-157-S10 regression tests
added to dispatch.bats (in rodin/workspace). All 11 invariants pass.
2026-05-15 10:26:14 -07:00
Rodin 6cefbb070e fix(#157): add S9 invariant and never-close constraint to dev-loop spec
CI / test (pull_request) Successful in 18s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 28s
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 30s
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m16s
- Add S9 to §6 Safety Invariants: zero close-PR API calls in dispatch
- Document worker ABSOLUTE CONSTRAINTS in §8 Worker Templates
- Add §9 entry for Issue #157 explaining the fix

All worker templates already contain the NEVER-close constraint from
a prior session. This commit makes the spec authoritative.

Companion changes in rodin/workspace:
- check-invariants.sh: add S9 static check
- dispatch.bats: add Bug-157-regression test
2026-05-15 14:47:54 +00:00
Rodin 838a34aa12 chore: cycle status 2026-05-15 14:42
CI / test (push) Successful in 16s
CI / review (anthropic--claude-4.6-sonnet, sonnet, SONNET_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, gpt, GPT_REVIEW_TOKEN) (push) Has been skipped
CI / review (gpt-5, security, ., rodin/security-patterns, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (push) Has been skipped
2026-05-15 14:42:27 +00:00
3 changed files with 62 additions and 54 deletions
+38
View File
@@ -0,0 +1,38 @@
# Dev-Loop Cycle Status — 2026-05-15 14:42 UTC
**Cron ID:** 5342ac81-4bbc-4e4c-a123-347a7788d50c
**Cycle:** review-bot-dev-loop (4-hour schedule)
**Status:****STEADY STATE** — All systems nominal, repo healthy
## Health Check Summary
| Check | Status | Details |
|-------|--------|---------|
| Main branch | ✅ Current | HEAD at 8ab45be (synced) |
| Working tree | ✅ Clean | No uncommitted changes |
| Test suite | ✅ All pass | 100% pass rate (go test ./...) |
| Code coverage | ✅ 76.7% | Above baseline target |
| Open issues | ✅ None | No assigned work |
| Open PRs | ✅ None | All merged |
| Remote sync | ✅ On-time | Up-to-date with origin/main |
## Actions This Cycle
- ✅ Fetched origin/main — up-to-date
- ✅ Ran full test suite — all pass
- ✅ Calculated code coverage — 76.7%
- ✅ Checked for new issues/PRs — none found
- ✅ Verified working tree clean
## Backlog Opportunities
1. **Integration tests** — cmd/review-bot coverage (53.3% → target 80%)
2. **Performance profiling** — doc-map filtering optimization
3. **Documentation** — Composite action examples
## Recommendation
**No new assignments.** Repo ready for next feature work. Standing by.
---
Generated: 2026-05-15 14:42 UTC | Cron: review-bot-dev-loop
-53
View File
@@ -1,53 +0,0 @@
# Dev-Loop Session — 2026-05-15 14:28 UTC
**Cron ID:** 5342ac81-4bbc-4e4c-a123-347a7788d50c
**Session:** review-bot-dev-loop
**Objective:** Identify high-value improvement opportunities in steady-state project
## Current State
- **Project Status:** ✅ Steady state, all tests passing
- **Code Coverage:** 76.7% overall, 53.3% for cmd/review-bot
- **Recent Work:** v0.4.0 released, 4 PRs merged
- **Last Commit:** 6fa3cb9 — cycle status checkpoint
- **Working Tree:** Clean, no uncommitted changes
## Analysis
### High-Value Opportunities
1. **Unit Test Coverage Gaps (cmd/review-bot)**
- Main function: 31.7% coverage (target for improvement)
- Subprocess testing infrastructure exists (`TestMainSubprocess_*` pattern)
- Goal: Reach 80% coverage from 53.3%
- Impact: Better regression protection, easier refactoring
2. **Integration Test Framework**
- Existing: `integration_test.go` with full review flow tested
- Opportunity: Add edge case coverage (network timeouts, malformed inputs, rate limiting)
- Tools: Already uses subprocess pattern from validation tests
3. **Performance Profiling**
- doc-map filtering currently unoptimized
- No benchmarks in place for path-scoping logic
- Opportunity: Add pprof benchmarks, document baseline metrics
4. **Documentation Gaps**
- Composite action examples in README (incomplete)
- Multi-reviewer setup: partially documented
- Specialized review types: needs examples
## Recommendation
**Unit test improvements** for cmd/review-bot are the highest-value work:
- Lower risk than new features
- Builds on existing subprocess testing infrastructure
- Delivers immediate coverage gains
- Sets foundation for future refactoring
## Status: STEADY STATE — NO NEW ASSIGNMENTS
Repo is healthy and ready for next feature work. Standing by for Aaron's direction.
---
Generated: 2026-05-15 14:28 UTC | Cron: review-bot-dev-loop
+24 -1
View File
@@ -231,6 +231,8 @@ 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` |
---
@@ -263,9 +265,20 @@ 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 and #145
## 9. Fixes for Issues #144, #145, and #157
**Issue #144** (autonomous merge):
The dispatch script contains no merge API calls anywhere. The `~/.openclaw/workspace/scripts/test/check-invariants.sh`
@@ -276,3 +289,13 @@ 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.