-
Notifications
You must be signed in to change notification settings - Fork 8
fix: enforce worker TODO.md restriction with multi-layer guards (t173) #649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,6 +89,85 @@ is_headless() { | |
| [[ "$HEADLESS" == "true" ]] | ||
| } | ||
|
|
||
| # Install a git pre-commit hook that blocks TODO.md changes in headless mode (t173) | ||
| # This is a hard guard — even if the AI agent tries to commit TODO.md, git rejects it. | ||
| install_headless_todo_guard() { | ||
| local git_dir | ||
| git_dir=$(git rev-parse --git-dir 2>/dev/null || echo "") | ||
| if [[ -z "$git_dir" ]]; then | ||
| return 0 | ||
| fi | ||
|
|
||
| local hooks_dir="$git_dir/hooks" | ||
| local hook_file="$hooks_dir/pre-commit" | ||
| local guard_marker="# t173-headless-todo-guard" | ||
|
|
||
| mkdir -p "$hooks_dir" | ||
|
|
||
| # If a pre-commit hook already exists, append our guard (if not already present) | ||
| if [[ -f "$hook_file" ]]; then | ||
| if grep -q "$guard_marker" "$hook_file" 2>/dev/null; then | ||
| return 0 # Already installed | ||
| fi | ||
| # Append to existing hook | ||
| cat >> "$hook_file" << 'GUARD' | ||
|
|
||
| # t173-headless-todo-guard | ||
| # Block TODO.md and planning file commits in headless worker mode | ||
| if [[ "${FULL_LOOP_HEADLESS:-false}" == "true" ]]; then | ||
| if git diff --cached --name-only | grep -qE '^(TODO\.md|todo/)'; then | ||
| echo "[t173 GUARD] BLOCKED: Headless workers must not commit TODO.md or todo/ files." | ||
| echo "[t173 GUARD] The supervisor owns all TODO.md updates. Put notes in commit messages or PR body." | ||
| exit 1 | ||
| fi | ||
| fi | ||
| GUARD | ||
| else | ||
| # Create new hook | ||
| cat > "$hook_file" << 'GUARD' | ||
| #!/usr/bin/env bash | ||
| # t173-headless-todo-guard | ||
| # Block TODO.md and planning file commits in headless worker mode | ||
| if [[ "${FULL_LOOP_HEADLESS:-false}" == "true" ]]; then | ||
| if git diff --cached --name-only | grep -qE '^(TODO\.md|todo/)'; then | ||
| echo "[t173 GUARD] BLOCKED: Headless workers must not commit TODO.md or todo/ files." | ||
| echo "[t173 GUARD] The supervisor owns all TODO.md updates. Put notes in commit messages or PR body." | ||
| exit 1 | ||
| fi | ||
| fi | ||
| GUARD | ||
| chmod +x "$hook_file" | ||
| fi | ||
|
|
||
| return 0 | ||
| } | ||
|
|
||
| # Remove the t173 headless guard from pre-commit hook (cleanup) | ||
| remove_headless_todo_guard() { | ||
| local git_dir | ||
| git_dir=$(git rev-parse --git-dir 2>/dev/null || echo "") | ||
| if [[ -z "$git_dir" ]]; then | ||
| return 0 | ||
| fi | ||
|
|
||
| local hook_file="$git_dir/hooks/pre-commit" | ||
| if [[ ! -f "$hook_file" ]]; then | ||
| return 0 | ||
| fi | ||
|
|
||
| # Remove the guard block (from marker to end of guard) | ||
| if grep -q "t173-headless-todo-guard" "$hook_file" 2>/dev/null; then | ||
| # Use sed to remove the guard block | ||
| local tmp_file | ||
| tmp_file=$(mktemp) | ||
| awk '/# t173-headless-todo-guard/{skip=1} /^fi$/ && skip{skip=0; next} !skip' "$hook_file" > "$tmp_file" | ||
| mv "$tmp_file" "$hook_file" | ||
| chmod +x "$hook_file" | ||
| fi | ||
|
|
||
| return 0 | ||
| } | ||
|
Comment on lines
+146
to
+169
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The To fix this, I've suggested adding unique start/end markers in # Remove the t173 headless guard from pre-commit hook (cleanup)
remove_headless_todo_guard() {
local git_dir
git_dir=$(git rev-parse --git-dir 2>/dev/null || echo "")
if [[ -z "$git_dir" ]]; then
return 0
fi
local hook_file="$git_dir/hooks/pre-commit"
if [[ ! -f "$hook_file" ]]; then
return 0
fi
# Remove the guard block (from marker to end of guard)
if grep -q "# t173-headless-todo-guard-start" "$hook_file" 2>/dev/null; then
# Use sed to remove the guard block
local tmp_file
tmp_file=$(mktemp)
sed '/^# t173-headless-todo-guard-start$/,/^# t173-headless-todo-guard-end$/d' "$hook_file" > "$tmp_file"
mv "$tmp_file" "$hook_file"
chmod +x "$hook_file"
fi
return 0
} |
||
|
|
||
| print_phase() { | ||
| local phase="$1" | ||
| local description="$2" | ||
|
|
@@ -656,6 +735,12 @@ cmd_start() { | |
| return 1 | ||
| fi | ||
|
|
||
| # Install git pre-commit guard to block TODO.md commits in headless mode (t173) | ||
| if is_headless; then | ||
| install_headless_todo_guard | ||
| print_info "HEADLESS: Installed TODO.md commit guard (t173)" | ||
| fi | ||
|
|
||
| # Pre-flight GitHub auth check — workers spawned via nohup/cron may lack | ||
| # SSH keys or valid gh tokens. Fail fast before burning compute. | ||
| if ! gh auth status >/dev/null 2>&1; then | ||
|
|
@@ -873,6 +958,9 @@ cmd_cancel() { | |
| rm -f ".agents/loop-state/quality-loop.local.state" 2>/dev/null | ||
| rm -f ".claude/ralph-loop.local.state" 2>/dev/null | ||
| rm -f ".claude/quality-loop.local.state" 2>/dev/null | ||
|
|
||
| # Remove headless TODO.md guard if installed (t173) | ||
| remove_headless_todo_guard | ||
|
|
||
| print_success "Full loop cancelled" | ||
| return 0 | ||
|
|
@@ -929,6 +1017,9 @@ cmd_complete() { | |
| echo "" | ||
|
|
||
| clear_state | ||
|
|
||
| # Remove headless TODO.md guard if installed (t173) | ||
| remove_headless_todo_guard | ||
|
|
||
| echo "<promise>FULL_LOOP_COMPLETE</promise>" | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -335,16 +335,23 @@ loop_generate_reanchor() { | |
| local git_branch | ||
| git_branch=$(git branch --show-current 2>/dev/null || echo "unknown") | ||
|
|
||
| # Get TODO.md in-progress tasks | ||
| # Detect headless worker mode (t173: workers must not interact with TODO.md) | ||
| local is_headless="false" | ||
| if [[ "${FULL_LOOP_HEADLESS:-false}" == "true" ]]; then | ||
| is_headless="true" | ||
| fi | ||
|
|
||
| # Get TODO.md in-progress tasks (read-only context, skip in headless mode - t173) | ||
| local todo_in_progress="" | ||
| if [[ -f "TODO.md" ]]; then | ||
| if [[ "$is_headless" == "false" && -f "TODO.md" ]]; then | ||
| todo_in_progress=$(grep -A10 "## In Progress" TODO.md 2>/dev/null | head -15 || echo "No tasks in progress") | ||
| fi | ||
|
|
||
| # Extract single next task (the "pin" concept from Loom) | ||
| # Focus on ONE task per iteration to reduce context drift | ||
| # Skip in headless mode — workers work on their assigned task only (t173) | ||
| local next_task="" | ||
| if [[ -f "TODO.md" ]]; then | ||
| if [[ "$is_headless" == "false" && -f "TODO.md" ]]; then | ||
| # Get first unchecked task from In Progress section, or first from Backlog | ||
| next_task=$(awk ' | ||
| /^## In Progress/,/^##/ { if (/^- \[ \]/) { print; exit } } | ||
|
|
@@ -382,12 +389,35 @@ loop_generate_reanchor() { | |
| latest_receipt=$(cat "$latest_receipt_file") | ||
| fi | ||
|
|
||
| # Build headless worker restriction block (t173) | ||
| local headless_restriction="" | ||
| if [[ "$is_headless" == "true" ]]; then | ||
| headless_restriction=" | ||
| ## MANDATORY Worker Restrictions (t173 - Headless Mode) | ||
|
|
||
| - **Do NOT edit, commit, or push TODO.md** — the supervisor owns all TODO.md updates. | ||
| - **Do NOT edit todo/PLANS.md or todo/tasks/*** — these are supervisor-managed. | ||
| - Report status via exit code, log output, and PR creation only. | ||
| - Put task notes in commit messages or PR body, never in TODO.md. | ||
| - Work ONLY on the assigned task described above. Do not pick tasks from TODO.md. | ||
| " | ||
| fi | ||
|
Comment on lines
+392
to
+404
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This block of text defining worker restrictions is very similar to another block added in Since both scripts source |
||
|
|
||
| # Build TODO.md section (omitted in headless mode - t173) | ||
| local todo_section="" | ||
| if [[ "$is_headless" == "false" ]]; then | ||
| todo_section="### TODO.md In Progress | ||
| \`\`\` | ||
| $todo_in_progress | ||
| \`\`\`" | ||
| fi | ||
|
|
||
| # Generate re-anchor prompt with single-task focus | ||
| cat > "$LOOP_REANCHOR_FILE" << EOF | ||
| # Re-Anchor Context (MANDATORY - Read Before Any Work) | ||
|
|
||
| **Loop:** $task_id | **Iteration:** $iteration | **Branch:** $git_branch | ||
|
|
||
| ${headless_restriction} | ||
| ## Original Task | ||
|
|
||
| $prompt | ||
|
|
@@ -396,7 +426,7 @@ $prompt | |
|
|
||
| Choose the single most important next action. Do NOT try to do everything at once. | ||
|
|
||
| ${next_task:-"No specific task found in TODO.md - work on the original task above."} | ||
| ${next_task:-"Work on the original task above."} | ||
|
|
||
| ## Current State | ||
|
|
||
|
|
@@ -410,10 +440,7 @@ $git_status | |
| $git_log | ||
| \`\`\` | ||
|
|
||
| ### TODO.md In Progress | ||
| \`\`\` | ||
| $todo_in_progress | ||
| \`\`\` | ||
| ${todo_section} | ||
|
|
||
| ## Guardrails (Do Not Repeat These Mistakes) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2574,10 +2574,21 @@ build_dispatch_cmd() { | |
| # Include task description in the prompt so the worker knows what to do | ||
| # even if TODO.md doesn't have an entry for this task (t158) | ||
| # Always pass --headless for supervisor-dispatched workers (t174) | ||
| # Inject explicit TODO.md restriction into worker prompt (t173) | ||
| local prompt="/full-loop $task_id --headless" | ||
| if [[ -n "$description" ]]; then | ||
| prompt="/full-loop $task_id --headless -- $description" | ||
| fi | ||
|
|
||
| # t173: Explicit worker restriction — prevents TODO.md race condition | ||
| prompt="$prompt | ||
|
|
||
| ## MANDATORY Worker Restrictions (t173) | ||
| - Do NOT edit, commit, or push TODO.md — the supervisor owns all TODO.md updates. | ||
| - Do NOT edit todo/PLANS.md or todo/tasks/* — these are supervisor-managed. | ||
| - Report status via exit code, log output, and PR creation only. | ||
| - Put task notes in commit messages or PR body, never in TODO.md." | ||
|
|
||
|
Comment on lines
+2583
to
+2591
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This block of text defining worker restrictions is very similar to another block added in Since both scripts source |
||
| if [[ -n "$memory_context" ]]; then | ||
| prompt="$prompt | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make the removal of this git hook guard more robust, I recommend adding unique start and end markers to the heredoc. This will allow for a more reliable removal process that isn't prone to errors from nested
if/fiblocks. I'll suggest a corresponding change inremove_headless_todo_guard.