fix(#1824): add body to enrollment commit messages

[fullsend-ai-coder]

Enrollment commits created by reconcile-repos.sh used bare subject lines with no body. Repos enforcing gitlint rule B6 (body-is-missing) rejected these at the merge queue.

Update all four commit message call sites (add, update, refresh, and remove shim) to include a subject, blank line, and a brief body with lines ≤72 characters. The write_shim_to_branch_from_default function already supports multiline messages via jq --arg, so only the strings at the call sites needed changing.

Add assertions in reconcile-repos-test.sh that verify captured commit messages contain a non-trivial body with proper formatting.

Note: make lint could not run (Go toolchain download permission denied in sandbox). Shell tests passed.

---

Closes #1824

Post-script verification

• Branch is not main/master (agent/1824-enrollment-commit-body)
• Secret scan passed (gitleaks — 972ce7f6654f0b7edaf699298901b10db69e4e4c..HEAD)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[github-actions]

Site preview

Preview: https://53f7e71b-site.fullsend-ai.workers.dev

Commit: d0de400bae56d9331f6a61a6cce39c3b0ac0d712

[fullsend-ai-review]

Review

Findings

Info

• [test-adequacy] internal/scaffold/fullsend-repo/scripts/reconcile-repos-test.sh — The commit message validation asserts that exactly 4 messages are captured and checks formatting (subject length, blank separator, body presence, line width), but does not verify which specific message text corresponds to which operation. A bug that swapped ENROLL_COMMIT_MSG and UNENROLL_COMMIT_MSG at call sites would pass. Consider asserting that specific subjects appear in the expected order (e.g., grepping for "add fullsend", "update fullsend", "remove fullsend").

• [sub-agent-failure] N/A — The style-conventions sub-agent did not return findings: model not available on deployment.

• [sub-agent-failure] N/A — The intent-coherence sub-agent did not return findings: model not available on deployment.

Previous run

Review

Findings

Low

• [test-inadequate] internal/scaffold/fullsend-repo/scripts/reconcile-repos-test.sh:293 — The commit message validation loop checks that each captured message is well-formed but does not assert the expected count of messages (should be 4: one each for update, add, refresh, and remove). If a code path silently fails to produce a commit, the test would still pass as long as at least one message is captured and well-formed. Adding [ "$msg_index" -eq 4 ] after the loop would catch regressions where a commit call site is accidentally skipped.

• [code-organization] internal/scaffold/fullsend-repo/scripts/reconcile-repos.sh:367 — ENROLL_COMMIT_MSG and UNENROLL_COMMIT_MSG are defined inside their respective loop bodies (reassigned on every iteration), while UPDATE_COMMIT_MSG is defined once at the top level alongside UPDATE_PR_BODY. All three are constant strings. Hoisting the two loop-scoped variables to the top alongside the other message constants would be more consistent and slightly more efficient.

Previous run (2)

Review

Findings

Low

• [naming-convention] internal/scaffold/fullsend-repo/scripts/reconcile-repos.sh:459 — REMOVE_COMMIT_MSG breaks the established naming pattern. The existing variables for this operation use the UNENROLL prefix (UNENROLL_BRANCH, UNENROLL_PR_TITLE, UNENROLL_PR_BODY). Consider renaming to UNENROLL_COMMIT_MSG for consistency.

• [naming-convention] internal/scaffold/fullsend-repo/scripts/reconcile-repos.sh:351 — UPDATE_COMMIT_MSG and REFRESH_COMMIT_MSG have identical text. If the stale-shim-update and existing-PR-refresh operations are semantically the same, consolidate into a single variable defined alongside the other constants at the top of the file. If they are intended to diverge, differentiate the messages now.

• [test-integrity] internal/scaffold/fullsend-repo/scripts/reconcile-repos-test.sh:282 — The commit message line-length check applies the 72-character limit to all lines including the subject. Conventional commit convention limits subjects to 50 characters. Not a bug (the actual subjects are well under 50 chars), but the test would not catch a subject between 51–72 characters if one were introduced later.

Info

• [test-adequacy] internal/scaffold/fullsend-repo/scripts/reconcile-repos-test.sh — Both prior-review findings (only stale-shim path exercised; removal commit message not captured) have been addressed. The test now covers all four commit-message code paths (add, update, refresh, remove) and captures the removal message from the Contents API DELETE endpoint via field_message extraction.

Previous run (3)

Review

Findings

Low

• [test-inadequate] internal/scaffold/fullsend-repo/scripts/reconcile-repos-test.sh:180 — Only the stale-shim-update commit message path is exercised by the test. The mock config has one enabled: true repo with an existing stale shim, so only that code path produces a git/commits call. The enrollment (new repo, no existing shim), refresh (existing enrollment PR), and removal paths are not reached, so their commit messages are not validated.

• [test-inadequate] internal/scaffold/fullsend-repo/scripts/reconcile-repos-test.sh:180 — The removal commit message (REMOVE_COMMIT_MSG) is sent via the Contents API DELETE endpoint using --field message=..., not via the Git Objects API with --input stdin. The test infrastructure only captures stdin payloads for git/commits calls, so even if a removal scenario were added, the removal message would not be captured for validation.

[fullsend-ai-review]

[low] test-inadequate

Only the stale-shim-update commit message path is exercised by the test. The enrollment, refresh-existing-PR, and removal paths are not reached, so their commit messages are not validated.

Suggested fix: Add test scenarios exercising the other three commit message paths: a repo with no existing shim (enrollment), a repo with an existing enrollment PR (refresh), and a disabled repo with an existing shim (removal).

[fullsend-ai-review]

[low] test-inadequate

The removal commit message is sent via the Contents API DELETE endpoint using --field, not via the Git Objects API with --input stdin. The test infrastructure only captures stdin payloads for git/commits calls, so the removal message cannot be validated even if a removal scenario were added.

Suggested fix: Extend the mock gh script to also capture --field message= values for DELETE calls to the contents endpoint.

[ifireball]

/fs-fix

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed both review findings. The test now exercises all four commit-message paths (update, enroll, refresh, remove) and captures removal messages sent via the Contents API DELETE endpoint.

Fixed (2):

1. Only stale-shim-update commit message path exercised by test (internal/scaffold/fullsend-repo/scripts/reconcile-repos-test.sh): Added new-repo (enrollment), refresh-repo (existing PR refresh), and removed-repo (disabled/removal) to test config and mocks. All four commit-message code paths now produce captured messages validated by the existing assertions.
2. Removal commit message not captured by test infrastructure (internal/scaffold/fullsend-repo/scripts/reconcile-repos-test.sh): Extended the gh mock to parse --method and --field message=... args. For DELETE calls on contents/ endpoints, the mock now captures the commit message as a JSON payload in the same format as git/commits captures, so the shared validation loop covers removal messages too.

Tests: passed

_{Updated by fullsend fix agent}

[fullsend-ai-review]

[low] naming-convention

REMOVE_COMMIT_MSG breaks the established naming pattern. The existing variables for this operation use the UNENROLL prefix (UNENROLL_BRANCH, UNENROLL_PR_TITLE, UNENROLL_PR_BODY). Consider renaming to UNENROLL_COMMIT_MSG for consistency.

[fullsend-ai-review]

[low] naming-convention

UPDATE_COMMIT_MSG and REFRESH_COMMIT_MSG have identical text. If the stale-shim-update and existing-PR-refresh operations are semantically the same, consolidate into a single variable defined alongside the other constants at the top of the file. If they are intended to diverge, differentiate the messages now.

[fullsend-ai-review]

[low] test-integrity

The commit message line-length check applies the 72-character limit to all lines including the subject. Conventional commit convention limits subjects to 50 characters. Not a bug (the actual subjects are well under 50 chars), but the test would not catch a subject between 51-72 characters if one were introduced later.

[ifireball]

/fs-fix

[fullsend-ai-coder]

🔧 Fix agent — iteration 2 (human-triggered)

Addressed 3 of 5 review findings with code fixes (naming convention for UNENROLL_COMMIT_MSG, consolidation of duplicate UPDATE/REFRESH commit message constants, 50-char subject length test). Disagreed with 2 findings (test-inadequate claims) because the test infrastructure already covers all four commit message paths including the DELETE endpoint capture.

Fixed (3):

1. naming-convention: REMOVE_COMMIT_MSG should use UNENROLL prefix (internal/scaffold/fullsend-repo/scripts/reconcile-repos.sh): Renamed REMOVE_COMMIT_MSG to UNENROLL_COMMIT_MSG to match the established UNENROLL_BRANCH, UNENROLL_PR_TITLE, UNENROLL_PR_BODY naming pattern.
2. naming-convention: UPDATE_COMMIT_MSG and REFRESH_COMMIT_MSG are identical (internal/scaffold/fullsend-repo/scripts/reconcile-repos.sh): Consolidated into a single UPDATE_COMMIT_MSG constant defined alongside other constants at the top of the file. Both call sites now reference the shared constant.
3. test-integrity: 72-char check does not enforce 50-char subject limit (internal/scaffold/fullsend-repo/scripts/reconcile-repos-test.sh): Added a 50-character subject line length check before the existing 72-char line length check in the commit message validation loop.

Disagreed (2):

1. test-inadequate: only stale-shim-update commit message path exercised: The test already exercises all four paths. The mock config includes test-repo (stale shim update), new-repo (enrollment), refresh-repo (existing PR refresh), and removed-repo (removal). All four repos are processed by reconcile-repos.sh, and commit messages from all paths are captured in COMMIT_MSGS_LOG via the mock gh script.
2. test-inadequate: removal commit message cannot be captured via test infrastructure: The mock gh script already captures DELETE endpoint field_message values. Lines 156-161 of the mock handle the DELETE method on removed-repo/contents/* by extracting field_message and writing it as JSON to COMMIT_MSGS_LOG. The removal path is already validated.

Tests: passed

_{Updated by fullsend fix agent}

[fullsend-ai-review]

[low] test-inadequate

The commit message validation loop checks that each captured message is well-formed but does not assert the expected count of messages (should be 4: one each for update, add, refresh, and remove). If a code path silently fails to produce a commit, the test would still pass as long as at least one message is captured and well-formed.

Suggested fix: Add an assertion after the validation loop: [ "$msg_index" -eq 4 ] || { echo "FAIL: expected 4 commit messages, got $msg_index"; exit 1; }

[fullsend-ai-review]

[low] code-organization

ENROLL_COMMIT_MSG and UNENROLL_COMMIT_MSG are defined inside their respective loop bodies (reassigned on every iteration), while UPDATE_COMMIT_MSG is defined once at the top level alongside UPDATE_PR_BODY. All three are constant strings — hoisting them to the top would be more consistent.

Suggested fix: Move ENROLL_COMMIT_MSG and UNENROLL_COMMIT_MSG definitions to the top of the script alongside UPDATE_COMMIT_MSG (after line 51).

[ifireball]

/fs-fix

[fullsend-ai-coder]

🔧 Fix agent — iteration 3 (human-triggered)

Addressed both review findings: added expected commit message count assertion (4) in the test, and hoisted ENROLL_COMMIT_MSG and UNENROLL_COMMIT_MSG to top-level constants for consistency.

Fixed (2):

1. commit message count not asserted in test (internal/scaffold/fullsend-repo/scripts/reconcile-repos-test.sh): Added assertion that exactly 4 commit messages are captured (one each for update, refresh, add, and remove), so a silently skipped commit call site would cause the test to fail.
2. ENROLL_COMMIT_MSG and UNENROLL_COMMIT_MSG defined inside loop bodies (internal/scaffold/fullsend-repo/scripts/reconcile-repos.sh): Hoisted both constant definitions to the top level alongside UPDATE_COMMIT_MSG and the other message constants, matching the established pattern.

Tests: passed

_{Updated by fullsend fix agent}