Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ jobs:
# writeShellApplication packages that depend on pr-review-report, so the
# fileset governs their hashes too — churn leaking into the source
# rebuilds the runner as well as the binary.
OUTPUTS="pr-review-report campaign-run review-run refresh-human-queue backfill-human-queue-history review-queue"
OUTPUTS="pr-review-report campaign-run review-run refresh-human-queue design-doctor backfill-human-queue-history review-queue"

drvs() {
for o in $OUTPUTS; do
Expand Down Expand Up @@ -205,6 +205,8 @@ jobs:
date uname getent id flock timeout find sort cut tail rm mkdir tee sed grep gh pr-review-report
check refresh-human-queue refresh-human-queue \
date getent id flock mktemp mv rm git pr-review-report
check design-doctor design-doctor \
date getent id cut flock timeout gh pr-review-report
check backfill-human-queue-history backfill-human-queue-history git wc pr-review-report
check review-queue sort-review-queue pr-review-report
check pr-review-report-sh pr-review-report-sh pr-review-report
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@
# the rain-org-health dashboard fetches it at runtime from the raw URL — no site redeploy:
!/human-queue.json
!/refresh-human-queue.sh
# The design lane's FSM doctor (#241): daily pass routing every noQuestion ai:design PR back
# to ai:needs-work; a flake runner like the refresher, on its own cron line:
!/design-doctor.sh
# Append-only per-state inventory history ({ts, counts} per changed refresh) + its one-time
# backfill; the dashboard fetches the jsonl at runtime for the Theory-of-Constraints panel:
!/human-queue-history.jsonl
Expand Down
66 changes: 66 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,62 @@ order, in one call — the same act a rejection is (#219). So every ruling retir
its own row, which is why the page caps at 3 for `next_ready`'s reason with more
force: a page is stale past its head by construction.

#### `design-doctor` — the machine exit for the `noQuestion` bucket (#241)

A `noQuestion` row — the label on the PR with no trusted comment raising a
question behind it — is a state no actor consumes: `next_design` withholds it
(there is no claim to present), the producer and vetter both skip it
(`ai:design` parks the PR by design), and no other command lists it, so the row
sits for ever. The withholding is right; what was missing is any transition OUT.
`design-doctor [--dry-run]` is that transition, ruled in
[#241](https://github.com/rainlanguage/issue-pr-cron/issues/241): the daily pass
routes **every** `noQuestion` row back to `ai:needs-work`, automatically.

- **Detection is `next_design`'s own classifier** — the same search, the same
archived-repo withholding, the same author-scoped question read. One
enumeration, one classifier, no second detector: the doctor acts on exactly
the bucket `counts.noQuestion` states, so the dashboard's defect bucket and
the pass that drains it can never disagree about the population.
- **The send-back is a VERDICT, because the sender is a machine.** The doctor
writes what the vetter's own automated send-back (`draft_send_back_plan`)
writes: a `🤖 ai:vetter` `Reviewed <head>: needs-work` comment carrying the
note and a stated lens, then the one-state label edit. Two things follow that
a `Rework note` could not give it — the comment is the **currency stamp**, so
the next vetter run does not re-vet the PR and strip the `ai:needs-work` this
pass just wrote; and a machine decision does not wear the **human's marker**,
which the producer's prompt teaches as "what the human/assistant leaves".
- **The comment goes first here**, inverting the draft send-back's order by that
order's own reasoning. There the label is what removes the PR from the leak
population; here it is what **keeps** the row in this pass's population, so a
failed label edit after a posted verdict is re-planned by the next tick (the
verdict dedups), while a failed comment after a label edit would strand a
needs-work with nothing trusted behind it and outside the doctor's own search.
- **Both exits it offers are transitions the producer can perform**: push the
rework the PR was parked mid-way through, or re-raise the question with
`flag-design` — its own transition, which returns the row to the human's queue
with a claim behind it. "Proceed under the normal lanes" is deliberately not
offered: `next_action` gives a needs-work PR the rework exit, and
campaign-prompt.txt forbids the no-op push that faking one would need.
- **What it never touches**, each re-established at write time against a fresh
fetch rather than trusted from the search index: a live question (whoever
raised it), a human decision (a `👤 human` ruling at head **or a native
review**), a PR carrying a co-resident modeled state the one-state strip would
destroy (`ai:close-candidate`, `ai:blocked-on` —
`draft_send_back_strips_no_state`'s rule), a draft, a PR outside the fleet
(`$PR_ASSIGNEE` — both actors that consume `ai:needs-work` enumerate by
author, so routing a third party's PR would move it into a state nobody
reads), a PR written to inside the settling window (both writers that raise a
question label first and comment last, so a still-moving PR may have one
mid-write), archived-repo rows, and anything it could not read.
- **A withheld row is NAMED, and the ones the pass can never drain name their
consumer** — a human decision, a non-fleet PR and an anchorless PR each print
the move that does drain them, because an `Ok` line the pass repeats daily
about a stuck row is not a queue. Idempotent: a route strips `ai:design`, so a
second run finds zero rows. The routes run **serially**: they are
content-creating writes through an unretried `gh`, and
`retire_blocked_infra_mode`, the only comparable bulk-write pass, is serial
for the same reason.

### Vetting is a pure function, and `vetted_at_head` is its cache key

A verdict is the value of one function — **the PR at its current head** — and
Expand Down Expand Up @@ -3033,6 +3089,16 @@ recording it provides.
```cron
0 0 * * * PATH=$HOME/.nix-profile/bin:/usr/bin:/bin nix run git+file://<install-dir>#pr-review-report -- gc <work-dir> <install-dir> >> <install-dir>/gc.log 2>&1
```

The **design doctor** (#241) runs daily at 04:00 UTC — a run-free slot, and
just ahead of the 05:00 producer tick that consumes what it routes. It is a
flake package like the runners (its closure pins `gh` and the binary), and it
writes GitHub state, so it is deliberately NOT folded into the data-only
`refresh-human-queue` tick:

```cron
0 4 * * * PATH=$HOME/.nix-profile/bin:/usr/bin:/bin CRON_DIR=<install-dir> nix run git+file://<install-dir>#design-doctor >> <install-dir>/design-doctor.log 2>&1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🔵 Trivial

Make the cron timezone explicit.

The documentation states 04:00 UTC, but 0 4 * * * uses the cron daemon's local timezone unless the host or crontab sets UTC. Add an explicit UTC setting or verify that every deployment host uses UTC.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` at line 3101, Update the documented cron entry so its schedule
explicitly uses UTC, rather than relying on the cron daemon or deployment host
timezone; preserve the existing 04:00 schedule and command.

```
Comment thread
coderabbitai[bot] marked this conversation as resolved.
- **Pause:** `touch DISABLED` · **Resume:** `rm DISABLED`
- **Watch:** `tail -f campaign.log` · **Run now:**
`CRON_DIR=<install-dir> nix run git+file://<install-dir>#campaign-run`
Expand Down
64 changes: 64 additions & 0 deletions design-doctor.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
#!/usr/bin/env bash
# design-doctor.sh — the FSM doctor pass for the design lane (#241): route every ai:design PR
# with NO live trusted design question back to ai:needs-work, with the trusted work order
# (re-flag or proceed) posted at the current head. Detection is next_design's own classifier;
# the whole pass is ONE tested subcommand (`pr-review-report design-doctor`) and this wrapper
# only adds what a bare cron invocation cannot: the install-dir env, the org scope from
# cron.env, stamped logging, and a flock so overlapping ticks never stack.
# Installed on a daily cron; see crontab (README "Schedule & controls").
# Packaged as a flake output (`packages.design-doctor`); `gh` and the binary come from the
# flake's locked nixpkgs. errexit is turned back off — writeShellApplication forces it, but this
# script reads the subcommand's exit status as data to log before passing it on.
set +o errexit

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf 'Files matching design-doctor.sh:\n'
fd -a 'design-doctor\.sh$' . || true

printf '\nGit status/stat:\n'
git diff --stat || true

for f in $(fd 'design-doctor\.sh$' .); do
  printf '\n### %s\n' "$f"
  wc -l "$f"
  sed -n '1,120p' "$f" | cat -n
done

printf '\nSearch timeout usage and flock setup:\n'
rg -n "set \+o errexit|timeout |flock|PR_ASSIGNEE|design-doctor" .

Repository: rainlanguage/issue-pr-cron

Length of output: 19258


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
import re

path = Path("design-doctor.sh")
src = path.read_text()

print("global_errexit_disabled=", bool(re.search(r"""set\s+[\da-z]*\+o\s+errexit""", src)))
print("errexit_reenabled=", bool(re.search(r"""set\s+[\da-z]*-o\s+errexit""", src)))

lock_line = next((i,l) for i,l in enumerate(src.splitlines(),1) if 'exec 9>"$DIR/.design-doctor.lock"' in l)
print("lock_line=%d" % lock_line[0], lock_line[1])
flock_line = next((i,l) for i,l in enumerate(src.splitlines(),1) if 'flock -n 9 ||' in l)
print("flock_line=%d" % flock_line[0], flock_line[1])

# Show whether no errexit leaves lock setup non-fatal when using || and then exiting.
print("flock_or_exit_is_always_0=", flock_line[1].strip().endswith('|| { log "skipped: a previous tick still holds the lock"; exit 0; }'))
print("exec_redirect_fails_without_errexit_and_reaches_flock_", True)

# Check for any remaining setup failures protected inside critical path:
for i,l in enumerate(src.splitlines(),1):
    if l and not l.strip().startswith("#"):
        if i < lock_line[0]:
            if re.search(r"""(!.*&&|&&.*|or\(|&&|if\b|trap\b|exit\b|set\b)""", l):
                print("pre-LOCK guard at %d: %s" % (i,l.strip()))
PY

Repository: rainlanguage/issue-pr-cron

Length of output: 743


Keep strict failure handling enabled.

With global errexit disabled, lock-file creation failure can pass to flock -n 9, and the wrapper then treats flock errors as normal contention and exits 0. A failed cron.env source can also continue with partial configuration. Keep set -euo pipefail. Capture the expected timeout 124 status around the final command. Distinguish lock contention from other flock failures.

Suggested status handling
-set +o errexit
+set -euo pipefail
...
-timeout "$DOCTOR_MAXTIME" pr-review-report design-doctor "$@"
-rc=$?
+if timeout "$DOCTOR_MAXTIME" pr-review-report design-doctor "$@"; then
+  rc=0
+else
+  rc=$?
+fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@design-doctor.sh` at line 12, Remove the global errexit disablement at the
script’s strict-mode setup and preserve set -euo pipefail. Update the final
timeout/flock execution to capture and explicitly accept only timeout status
124, while treating other flock or command failures as errors; keep lock
contention distinct from lock-file creation and other flock failures.

Source: Linters/SAST tools


# --- locate the install dir + bare-cron env (mirrors refresh-human-queue.sh) ---
# $0 is a read-only nix store path, so the install dir comes from the crontab's $CRON_DIR,
# defaulting to the working directory for an interactive run from the checkout.
DIR="${CRON_DIR:-$PWD}"
: "${HOME:=$(getent passwd "$(id -un)" | cut -d: -f6)}"; export HOME
: "${USER:=$(id -un)}"; export USER
: "${LOGNAME:=$USER}"; export LOGNAME

# Every line is stamped, same format as the other cron logs, so design-doctor.log can answer
# "when did this last run / fail". Everything goes to stderr, one stream, so the crontab's
# `>> …log 2>&1` preserves the order.
log() { echo "$(date -u +%FT%TZ) design-doctor: $*" >&2; }

cd "$DIR" || { log "install dir '$DIR' is not usable — set CRON_DIR to the checkout"; exit 1; }

# --- deployment config (defaults here; override in ./cron.env) ---
# A hard cap, as both model runners carry (MAXTIME=3h, REVIEW_MAXTIME=2h): an unattended writer
# with no cap can hold its flock for ever, and every later tick then logs "a previous tick still
# holds the lock" while nothing drains. This pass is a bounded number of `gh` calls, so its cap is
# small — long enough for a large backlog on a slow API, short enough that a wedged tick is gone
# before the next one.
DOCTOR_MAXTIME="30m" # hard cap per tick

# Org scope + fleet: single source is cron.env (same as the producer/vetter/refresher). PR_ASSIGNEE
# is what the pass calls "ours" — it withholds rows outside the fleet rather than routing them into
# a state no actor enumerates.
# shellcheck disable=SC1091
[ -f cron.env ] && . ./cron.env
: "${ORGS:=rainlanguage cyclofinance S01-Issuer}"; export ORGS
export PR_ASSIGNEE
Comment on lines +37 to +43

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 'PR_ASSIGNEE|design-doctor' pr-review-report-rs/src/main.rs design-doctor.sh

Repository: rainlanguage/issue-pr-cron

Length of output: 27853


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf '--- relevant source slices ---\n'
sed -n '28149,28385p' pr-review-report-rs/src/main.rs
printf '\n--- pr_assignee usages in design doctor tests/logic ---\n'
rg -n 'pr_assignee\(|author|The fleet|fleet|NotOurFleet|not our' pr-review-report-rs/src/main.rs

printf '\n--- behavioral probe of Rust env-resolution and whitespace trim if present in helper context ---\n'
python3 - <<'PY'
# Read-only probe matching the Rust behavior described in source:
# std::env::var falls back only if the variable is absent, not if it is present and empty.
cases = {
    'absent': None,
    'empty': '',
    'zero_length_quoted_like': '',
    'single_whitespace': ' ',
    'valid': 'thedavidmeister',
}
for name, value in cases.items():
    pr_assignee = value if value is not None else 'thedavidmeister'
    print(f'{name}: PR_ASSIGNEE={value!r} -> pr_assignee()={pr_assignee!r}')
PY

Repository: rainlanguage/issue-pr-cron

Length of output: 255


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

# Check whether PR_ASSIGNEE is required/fails closed elsewhere in the shell wrapper or tests.
printf '--- PR_ASSIGNEE references ---\n'
rg -n -E 'PR_ASSIGNEE|export PR_ASSIGNEE|non-empty|require|missing|unset|cron.env' design-doctor.sh pr-review-report-rs/src/main.rs README.md crontab . 2>/dev/null | head -n 200

Repository: rainlanguage/issue-pr-cron

Length of output: 255


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo '--- relevant source slices ---'
sed -n '28149,28385p' pr-review-report-rs/src/main.rs

echo
echo '--- pr_assignee usages in design doctor tests/logic ---'
rg -n 'pr_assignee\(|author|The fleet|fleet|NotOurFleet|not our' pr-review-report-rs/src/main.rs

echo
echo '--- behavioral probe of Rust env-resolution and whitespace trim if present in helper context ---'
python3 - <<'PY'
# Read-only probe matching the Rust env-resolution semantics in source:
# std::env::var returns Err when the variable is absent and Some("") when it is present but empty.
# A ".trim().parse()" fallback then returns the default only for absent keys.
def pr_assignee(value):
    if value is None:
        return 'thedavidmeister'
    return value
for name, value in [
    ('absent', None),
    ('empty', ''),
    ('single_whitespace', ' '),
    ('valid', 'thedavidmeister'),
    ('nonexistent_assigned', ''):
        pass
]:
    print(f'{name}: PR_ASSIGNEE={value!r} -> pr_assignee()={pr_assignee(value)!r}')
PY

Repository: rainlanguage/issue-pr-cron

Length of output: 42554


Fail closed when PR_ASSIGNEE is empty.

design-doctor only uses PR_ASSIGNEE as the fleet author, so accepting an empty value gives the doctor a valid-looking author for route equality checks without defining a real fleet. Require PR_ASSIGNEE before calling pr-review-report design-doctor.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@design-doctor.sh` around lines 37 - 43, Require a non-empty PR_ASSIGNEE in
the design-doctor setup before invoking pr-review-report design-doctor. Validate
the value after loading cron.env and exporting ORGS, and fail closed with a
nonzero exit when it is unset or empty; preserve the existing behavior for valid
assignee values.


# --- kill switch ---
# The producer's own flag, not a third one. The README documents "Pause: touch DISABLED" directly
# beneath this cron's line, so an operator halting the pipeline believes everything is stopped —
# and this pass STRIPS LABELS and POSTS TRUSTED COMMENTS across every org in ORGS. A writer that
# ignores the pause is the one runner whose ticks a halted operator cannot undo.
if [ -f "$DIR/DISABLED" ]; then
log "SKIP: DISABLED flag present"
exit 0
fi

# flock so overlapping ticks never stack.
exec 9>"$DIR/.design-doctor.lock"
flock -n 9 || { log "skipped: a previous tick still holds the lock"; exit 0; }

log "tick start (cap $DOCTOR_MAXTIME)"
timeout "$DOCTOR_MAXTIME" pr-review-report design-doctor "$@"
rc=$?
[ "$rc" -eq 124 ] && log "TIMED OUT after $DOCTOR_MAXTIME — the tick was cut off; rows it had not reached are re-enumerated next tick"
log "tick end (rc=$rc)"
exit "$rc"
20 changes: 20 additions & 0 deletions flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,21 @@
];
};

# The FSM doctor for the design lane (#241): routes every ai:design PR with no live
# trusted design question back to ai:needs-work, daily. Writes labels + the trusted work
# order via gh, so gh rides in the closure like the other runners'.
design-doctor = runner {
name = "design-doctor";
file = ./design-doctor.sh;
runtimeInputs = [
pr-review-report
pkgs.gh
pkgs.coreutils
pkgs.util-linux # flock
pkgs.getent # resolves HOME when cron starts without it
];
};

# One-time history backfill; walks git history of the snapshot.
backfill-human-queue-history = runner {
name = "backfill-human-queue-history";
Expand Down Expand Up @@ -221,6 +236,7 @@
campaign-run
review-run
refresh-human-queue
design-doctor
backfill-human-queue-history
review-queue
;
Expand All @@ -242,6 +258,10 @@
type = "app";
program = "${refresh-human-queue}/bin/refresh-human-queue";
};
design-doctor = {
type = "app";
program = "${design-doctor}/bin/design-doctor";
};
};

devShells.cron = cron-shell;
Expand Down
Loading
Loading