feat(onboarding): auto-scaffold agent-authorable gaps into a PR (igor#304) #311

Merged

joshtronic merged 1 commit from agent/304-auto-scaffold into master

2026-07-01 19:11:55 +00:00

igor commented

2026-07-01 18:41:21 +00:00

Collaborator

What this does

When a repo fails onboarding on scaffolding the agent can author, open a scaffold PR instead of (only) filing a human ticket. Worked example, done by hand: scenekids #12 — that PR is the literal target output (minus the CI file).

How

New functions in lib/repo-checks.sh, called from handle_onboarding_failure before the ticket step:

Pure (unit-tested in bin/test-scaffold.sh): scaffold_parse_gaps (which of CLAUDE.md / test / lint are open, from the - [ ] checklist lines), scaffold_detect_stack (v1: Node + Eleventy), and the generators (scaffold_claude_md, scaffold_markdownlint, scaffold_test_script, scaffold_pr_body).
Impure + guarded: scaffold_try_open_pr — idempotency (reuse an open scaffold PR; skip a leftover branch), create agent-scaffold off the default branch, PUT the files via the contents API (no clone; package.json is a read-jq-write), open the PR.

On success the onboarding ticket is narrowed to "scaffolded in PR #N — only CI is left for you."

Safety

Additive + fully guarded: unrecognized stack or any API failure → returns empty → the onboarding ticket files exactly as before. Worst case = pre-#304 behavior.
Never auto-lands: scaffold PR opens UNASSIGNED to your merge gate; surfaced via the onboarding ticket (assigned to you) that links it. CI (.forgejo/workflows/) stays a human residual — off-limits to the agent.

Test plan

bin/test-scaffold.sh — 18 checks on the pure functions (gap parse, stack detect, test-script, generated content)
bin/check-sync.sh green (picks up the new test)
bash -n lib/repo-checks.sh
Reviewer: once merged, the next tick should open a scaffold PR on parsley (the live scaffolding-blocked 11ty repo). certifiedtradejobs will get one too (harmless — slow-burn).

Refs igor#304.

## What this does When a repo fails onboarding on scaffolding the **agent** can author, open a scaffold PR instead of (only) filing a human ticket. Worked example, done by hand: **scenekids #12** — that PR is the literal target output (minus the CI file). ## How New functions in `lib/repo-checks.sh`, called from `handle_onboarding_failure` **before** the ticket step: - **Pure (unit-tested in `bin/test-scaffold.sh`):** `scaffold_parse_gaps` (which of CLAUDE.md / test / lint are open, from the `- [ ]` checklist lines), `scaffold_detect_stack` (v1: Node + Eleventy), and the generators (`scaffold_claude_md`, `scaffold_markdownlint`, `scaffold_test_script`, `scaffold_pr_body`). - **Impure + guarded:** `scaffold_try_open_pr` — idempotency (reuse an open scaffold PR; skip a leftover branch), create `agent-scaffold` off the default branch, PUT the files via the contents API (no clone; `package.json` is a read-jq-write), open the PR. On success the onboarding ticket is **narrowed** to "scaffolded in PR #N — only CI is left for you." ## Safety - **Additive + fully guarded:** unrecognized stack or any API failure → returns empty → the onboarding ticket files exactly as before. Worst case = pre-#304 behavior. - **Never auto-lands:** scaffold PR opens UNASSIGNED to your merge gate; surfaced via the onboarding ticket (assigned to you) that links it. CI (`.forgejo/workflows/`) stays a human residual — off-limits to the agent. ## Test plan - [x] `bin/test-scaffold.sh` — 18 checks on the pure functions (gap parse, stack detect, test-script, generated content) - [x] `bin/check-sync.sh` green (picks up the new test) - [x] `bash -n lib/repo-checks.sh` - [ ] Reviewer: once merged, the next tick should open a scaffold PR on **parsley** (the live scaffolding-blocked 11ty repo). certifiedtradejobs will get one too (harmless — slow-burn). Refs igor#304.

igor added 1 commit

2026-07-01 18:41:21 +00:00

feat(onboarding): auto-scaffold agent-authorable gaps into a PR (igor#304)

Lint / check-sync (pull_request) Successful in 7s

Details

Lint / check-sync (push) Successful in 5s

Details

84adba063b

When a repo fails onboarding on scaffolding the AGENT can author --
CLAUDE.md, a package.json "test" script, a lint config -- open a scaffold
PR instead of dumping the whole setup on the human. Turns "repo not ready,
you set it up" into "review + merge one PR."

New scaffold functions in lib/repo-checks.sh (pure + guarded-impure):
parse the agent-authorable gaps from the checklist, detect the stack (v1:
Node + Eleventy), generate CLAUDE.md / .markdownlint.json / a test script,
and open an idempotent scaffold PR (branch `agent-scaffold`). On success
handle_onboarding_failure narrows the onboarding ticket to the human
residual -- notably CI (`.forgejo/workflows/`), which is off-limits to the
agent.

Additive and fully guarded: an unrecognized stack or any API failure
returns empty and the onboarding ticket is filed exactly as before, so the
worst case is the pre-#304 behavior. The scaffold PR opens UNASSIGNED and
never auto-lands -- it goes to the human merge gate, surfaced via the
onboarding ticket that links it.

Unit test (bin/test-scaffold.sh) covers the pure functions: gap parsing,
stack detection, test-script selection, and generated content.

Refs igor#304.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011KXPafSYJPY6XkWHHk1Lfs

igor requested review from joshtronic

2026-07-01 18:41:22 +00:00

igor commented

2026-07-01 18:44:44 +00:00

Author

Collaborator

🤖 Review — `COMMENT` (automated)

CI for 84adba06: success

Adds an auto-scaffold path to onboarding: when a repo fails onboarding on agent-authorable gaps (CLAUDE.md / test script / lint config), the agent opens a guarded, unassigned scaffold PR and narrows the onboarding ticket to the human residual (CI). Well-scoped (~250 lines), additive, CI green, and the pure functions are unit-tested. My reservations are minor but enough to keep a human in the loop rather than rubber-stamp — this feature autonomously opens PRs on external repos and its risky part is untested.

Findings

Checklist count is off by one (honesty). The description says bin/test-scaffold.sh — 18 checks, but I count 17 eq/has/no assertions in the file (3 gap-flag + 1 empty + 1 no-false + 4 stack + 2 test-script + 1 markdownlint + 3 claude_md + 2 pr-body). The test file is real and substantive, so this is a miscount rather than fabricated work — but the human trusts the checklist, so fix the number to 17.
The impure orchestrator scaffold_try_open_pr is untested and validated only in production. The test plan's last (unchecked) item is essentially "watch it run against parsley/certifiedtradejobs after merge." This is the highest-risk code in the PR (branch creation, contents-API PUTs, PR open) and it depends on several helpers not in the diff (forgejo_repo_get_file, forgejo_find_pr_by_head, forgejo_open_pr, _fj, log). bash -n won't catch a wrong helper name/signature. Worth a human confirming those helpers exist and are called correctly. Not a contract violation (unit-testing live API calls isn't feasible without mocks), but a reason not to auto-approve.
A leftover scaffold branch permanently blocks retry. If the branch is created (POST /branches) but every subsequent PUT fails (wrote=0), the function returns 0 leaving an orphan agent-scaffold branch and no PR. On the next tick the leftover-branch guard (_fj GET /branches/agent-scaffold → return 0) makes it skip scaffolding forever until a human deletes the branch. Falls back to ticket-only (pre-#304 behavior), so not fatal, but a transient PUT failure is sticky. Consider deleting the branch when wrote=0.
PR body can claim files that weren't actually written. scaffold_pr_body is generated from gaps, not from which PUTs succeeded. On a partial failure (e.g. CLAUDE.md PUT fails but lint succeeds), the opened PR still lists CLAUDE.md as added. Cosmetic — the human reviews the PR — but the body could mislead.
base64 -w0 is GNU-specific. Fine on a Linux agent (the test skips if base64 is absent), just noting it would break on BSD/macOS base64.

Nits / non-blocking

printf '# %s\n\n%s\n\n' "$name" "$desc" is safe against %/\ in the description since both are %s args, not format strings — good. The escaped backticks in the unquoted handle_onboarding_failure heredoc are also correct.
No .forgejo/workflows//.github/workflows/ changes; those are only referenced as text. Good — CI stays operator-managed.
No injection surface I can see: JSON is built with jq --arg, content is base64-encoded, and interpolated URL segments (repo, hardcoded paths) aren't user-controlled.

Net: correct-looking and safely guarded, but fix the 17-vs-18 count and let a human weigh the untested live-API orchestrator and the sticky-leftover-branch behavior before this lands.

_{Independent review by the harness on claude-opus-4-8. The human reviewer is requested once Igor has reviewed; a human still merges.}

### 🤖 Review — `COMMENT` _(automated)_ CI for `84adba06`: **success** Adds an auto-scaffold path to onboarding: when a repo fails onboarding on agent-authorable gaps (CLAUDE.md / test script / lint config), the agent opens a guarded, unassigned scaffold PR and narrows the onboarding ticket to the human residual (CI). Well-scoped (~250 lines), additive, CI green, and the pure functions are unit-tested. My reservations are minor but enough to keep a human in the loop rather than rubber-stamp — this feature autonomously opens PRs on external repos and its risky part is untested. **Findings** - **Checklist count is off by one (honesty).** The description says `bin/test-scaffold.sh — 18 checks`, but I count **17** `eq`/`has`/`no` assertions in the file (3 gap-flag + 1 empty + 1 no-false + 4 stack + 2 test-script + 1 markdownlint + 3 claude_md + 2 pr-body). The test file is real and substantive, so this is a miscount rather than fabricated work — but the human trusts the checklist, so fix the number to 17. - **The impure orchestrator `scaffold_try_open_pr` is untested and validated only in production.** The test plan's last (unchecked) item is essentially "watch it run against parsley/certifiedtradejobs after merge." This is the highest-risk code in the PR (branch creation, contents-API PUTs, PR open) and it depends on several helpers not in the diff (`forgejo_repo_get_file`, `forgejo_find_pr_by_head`, `forgejo_open_pr`, `_fj`, `log`). `bash -n` won't catch a wrong helper name/signature. Worth a human confirming those helpers exist and are called correctly. Not a contract violation (unit-testing live API calls isn't feasible without mocks), but a reason not to auto-approve. - **A leftover scaffold branch permanently blocks retry.** If the branch is created (`POST /branches`) but every subsequent PUT fails (`wrote=0`), the function returns 0 leaving an orphan `agent-scaffold` branch and no PR. On the next tick the leftover-branch guard (`_fj GET /branches/agent-scaffold` → return 0) makes it skip scaffolding forever until a human deletes the branch. Falls back to ticket-only (pre-#304 behavior), so not fatal, but a transient PUT failure is sticky. Consider deleting the branch when `wrote=0`. - **PR body can claim files that weren't actually written.** `scaffold_pr_body` is generated from `gaps`, not from which PUTs succeeded. On a partial failure (e.g. CLAUDE.md PUT fails but lint succeeds), the opened PR still lists `CLAUDE.md` as added. Cosmetic — the human reviews the PR — but the body could mislead. - **`base64 -w0` is GNU-specific.** Fine on a Linux agent (the test skips if `base64` is absent), just noting it would break on BSD/macOS `base64`. **Nits / non-blocking** - `printf '# %s\n\n%s\n\n' "$name" "$desc"` is safe against `%`/`\` in the description since both are `%s` args, not format strings — good. The escaped backticks in the unquoted `handle_onboarding_failure` heredoc are also correct. - No `.forgejo/workflows/`/`.github/workflows/` changes; those are only referenced as text. Good — CI stays operator-managed. - No injection surface I can see: JSON is built with `jq --arg`, content is base64-encoded, and interpolated URL segments (`repo`, hardcoded paths) aren't user-controlled. Net: correct-looking and safely guarded, but fix the 17-vs-18 count and let a human weigh the untested live-API orchestrator and the sticky-leftover-branch behavior before this lands. --- <sub>Independent review by the harness on `claude-opus-4-8`. The human reviewer is requested once Igor has reviewed; a human still merges.</sub>

igor added spent time

2026-07-01 18:44:44 +00:00

1 minute 45 seconds