Skip to content

fix(#2193): auto-patch .gitlint to exempt bot authors during repo enrollment#2194

Closed
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/2193-gitlint-bot-exemption
Closed

fix(#2193): auto-patch .gitlint to exempt bot authors during repo enrollment#2194
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/2193-gitlint-bot-exemption

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

The code agent's GitHub noreply email produces Signed-off-by trailers that inherently exceed 72 characters, failing gitlint body-max-line-length rules in onboarded repos. This required manual .gitlint patching by human maintainers on every enrollment PR.

Add patch_gitlint_content and patch_gitlint_on_branch functions to reconcile-repos.sh that detect .gitlint in target repos during enrollment and automatically add fullsend bot usernames to the ignore-by-author-name regex. The patch is idempotent (skips repos that already have the exemption) and non-fatal (logs warnings on API failures without blocking enrollment).

Three new test cases cover: adding the section when absent, appending to an existing regex, and idempotent no-op when bots are already exempt.


Closes #2193

Post-script verification

  • Branch is not main/master (agent/2193-gitlint-bot-exemption)
  • Secret scan passed (gitleaks — a029a60d692add2778a1c13ed376d5fa1b5d8aef..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

…ollment

The code agent's GitHub noreply email produces Signed-off-by trailers
that inherently exceed 72 characters, failing gitlint body-max-line-length
rules in onboarded repos. This required manual .gitlint patching by
human maintainers on every enrollment PR.

Add patch_gitlint_content and patch_gitlint_on_branch functions to
reconcile-repos.sh that detect .gitlint in target repos during
enrollment and automatically add fullsend bot usernames to the
ignore-by-author-name regex. The patch is idempotent (skips repos
that already have the exemption) and non-fatal (logs warnings on
API failures without blocking enrollment).

Three new test cases cover: adding the section when absent, appending
to an existing regex, and idempotent no-op when bots are already
exempt.

Closes #2193
@github-actions

Copy link
Copy Markdown

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

@github-actions

Copy link
Copy Markdown

Site preview

Preview: https://6f28231b-site.fullsend-ai.workers.dev

Commit: 6c05cea7eeee84755cf34901a672e368795ba24f

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 11, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 7:39 PM UTC · Completed 7:52 PM UTC
Commit: 6c05cea · View workflow run →

@codecov

codecov Bot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

Copy link
Copy Markdown

Review

Findings

Low

  • [edge-case] internal/scaffold/fullsend-repo/scripts/reconcile-repos.sh — The idempotency check in patch_gitlint_content uses grep -qF against the entire file content rather than just the regex= line within [ignore-by-author-name]. If a bot name string appeared in a comment or unrelated section, the function would consider it already present and skip patching. Practical risk is minimal given the specificity of the bot name strings.

  • [scope-expansion-beyond-authorization] internal/scaffold/fullsend-repo/scripts/reconcile-repos.sh — This PR expands reconcile-repos.sh beyond shim-only management to also modify .gitlint files in target repos. The expansion is authorized by issue Code agent Signed-off-by trailer exceeds gitlint body-max-line-length in repos with 72-char limit #2193 and is non-fatal (always returns 0), but documenting this scope expansion in an ADR would help future contributors understand the enrollment script's responsibilities.

  • [architectural-coherence] internal/scaffold/fullsend-repo/scripts/reconcile-repos.shBOT_AUTHOR_NAMES hardcodes bot identities in the scaffold script. This is consistent with existing patterns (other constants like ENROLL_BRANCH, SHIM_PATH, PR titles are also hardcoded), but if more bots are expected in the future, consider moving to config.yaml.

  • [design-trajectory-alignment] internal/scaffold/fullsend-repo/scripts/reconcile-repos.sh — The non-fatal warning approach (returns 0 always) makes it difficult to track which repos actually have the exemption applied vs. which silently skipped it. Consider adding a counter (e.g., PATCHED_GITLINT) to the reconciliation summary for observability.

Info

  • [naming-trajectory-alignment] internal/scaffold/fullsend-repo/scripts/reconcile-repos.shpatch_gitlint_on_branch is a one-off function for .gitlint. The underlying fetch-transform-commit pattern could be generalized if other config files need similar treatment, but YAGNI applies here.

  • [scope-alignment-with-issue] internal/scaffold/fullsend-repo/scripts/reconcile-repos-test.sh — Tests 5-7 cover core behaviors (add section, append to regex, idempotent no-op) but do not cover failure modes (API errors, missing permissions). The production code handles these gracefully, so risk is low.

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Jun 11, 2026

@ralphbean ralphbean left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. One non-blocking note inline.

FAILED=$((FAILED + 1))
continue
fi
patch_gitlint_on_branch "$REPO" "$ENROLL_BRANCH"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[moderate, non-blocking] Already-enrolled repos whose shim is current hit continue above and skip this. For backfilling those repos, we could run the gitlint check independently of shim state, or just bump the shim template to force everything through this path.

@rh-hemartin rh-hemartin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't like this approach, why don't we do the same as claude code? Kind of

Signed-off-by: fullsend-ai-coder<noreply@fullsend.sh>

@ralphbean

@ralphbean

Copy link
Copy Markdown
Member

Oh, actually - I don't like this either, but for a different reason!

First, the Author field needs to be a longer string so that #1449 works. The DCO app will waive the DCO requirement if it recognizes the author as a bot and it will only recognize the author as a bot if the Author field contains the app installation id and, anyways - it is on the long-ish side.

I think this issue got filed because those Author settings were propagating to the Signed-off-by trailer, making it too long, causing gitlint to fail, but....

That's a problem! Bots should never provide Signed-off-by. Signed-off-by implies personhood and agents are not people. We should instead:

  • Add exhortation to never supply Signed-off-by to code-and-fix agents' instructions
  • Add a general linter that prohibits this in post-code and post-fix

@ralphbean ralphbean closed this Jun 12, 2026
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jun 12, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 3:49 PM UTC · Completed 3:57 PM UTC
Commit: 6c05cea · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #2194 — auto-patch .gitlint to exempt bot authors

Outcome: Closed without merging. 649 lines of wasted work.

Timeline:

  1. Issue #2193 reported gitlint body-max-line-length failures caused by bot Signed-off-by trailers.
  2. Code agent produced a 649-line fix patching .gitlint to exempt bot usernames during repo enrollment.
  3. Review agent approved with only low/info findings (edge cases, hardcoded names, test coverage).
  4. Human ralphbean initially approved with a minor inline note.
  5. Human rh-hemartin requested changes, suggesting a shorter Signed-off-by format instead.
  6. ralphbean reconsidered and closed the PR, concluding bots should never produce Signed-off-by trailers at all — the fix should be upstream in agent instructions, not downstream in linter config.
  7. ralphbean immediately filed #2236 ("Agents must never produce Signed-off-by trailers") as the correct fix.

Root cause: The code agent solved the symptom (gitlint rejects long lines) rather than the root cause (bots shouldn't use Signed-off-by). The review agent's findings were entirely implementation-level and missed the fundamental approach question.

Existing issue coverage: Several open issues already address the general patterns observed here:

  • #1472 — Review agent should validate implementation approach against issue requirements
  • #1469 — Review agent should assess feature-level design, not just implementation
  • #1891 — Code agent should prefer minimal-impact solutions matching triage scope
  • #2185 — Code agent should follow triage's recommended implementation strategy
  • #2236 — The actual root-cause fix (agents must never produce Signed-off-by)

One novel proposal below targets a specific, detectable anti-pattern not covered by existing issues.

Proposals filed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge All reviewers approved — ready to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Code agent Signed-off-by trailer exceeds gitlint body-max-line-length in repos with 72-char limit

2 participants