Skip to content

refactor(#5390): add polling-support methods to GitLab client - #5391

Merged
ggallen merged 1 commit into
mainfrom
agent/5390-gitlab-poll-methods
Jul 21, 2026
Merged

refactor(#5390): add polling-support methods to GitLab client#5391
ggallen merged 1 commit into
mainfrom
agent/5390-gitlab-poll-methods

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Summary

Add 11 polling-support methods to the GitLab forge client so the cron poller can be wired to a real API client. A PollClient wrapper type is introduced to resolve the GetIssue return-type conflict between forge.Client and poll.GitLabClient.

Related Issue

Closes #5390

Changes

  • Add internal/forge/gitlab/poll.go with PollClient type and all 11 methods:
    • ListIssuesUpdatedSince, ListMergeRequestsUpdatedSince — paginated issue/MR listing
    • ListProjectEvents — date-widened event listing with client-side timestamp filtering
    • ListIssueNotes, ListMergeRequestNotes — ascending-order note listing
    • ListResourceLabelEvents — ascending-ID label event listing
    • GetCIVariable — variable value retrieval
    • CreateNoteAwardEmoji — emoji reactions with Issue/MergeRequest routing
    • GetIssue — poll-specific issue retrieval returning *poll.Issue
    • GetMemberAccessLevel — uses /members/all/ for inherited membership
    • GetProjectPath — project path lookup by numeric ID
  • Add internal/forge/gitlab/poll_test.go with 21 tests covering all methods, pagination, error cases, client-side filtering, and inherited method delegation
  • Wire internal/cli/poll.go to construct a real PollClient instead of passing nil

Testing

  • go build ./internal/forge/gitlab/... ./internal/cli/... compiles cleanly
  • go vet ./internal/forge/gitlab/... ./internal/cli/... passes
  • go test ./internal/forge/gitlab/... — all tests pass (including 21 new poll tests)
  • Compile-time var _ poll.GitLabClient = (*PollClient)(nil) check included
  • scan-secrets passes on all changed files

Checklist

  • PR title follows Conventional Commits (correct type, ! for breaking changes)
  • Tests added/updated for new or modified logic

Closes #5390

Post-script verification

  • Branch is not main/master (agent/5390-gitlab-poll-methods)
  • Secret scan passed (gitleaks — 0a0e09d99bf9a7ca9803f21d15763d3797828b7a..HEAD)
  • PR body secret scan passed (gitleaks — no-git)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner July 20, 2026 21:25
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Triggers review agent dispatch label Jul 20, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 20, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:26 PM UTC · Completed 9:39 PM UTC
Commit: 9e7453b · View workflow run →

@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown

Site preview

Preview: https://f707b5e7-site.fullsend-ai.workers.dev

Commit: 3df5b4c7e96b124a4160827e8aa48db1c1284fb8

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.04142% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/cli/poll.go 0.00% 5 Missing ⚠️

📢 Thoughts on this report? Let us know!

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 20, 2026

Copy link
Copy Markdown

Review

Findings

Low

  • [api-contract] internal/forge/gitlab/poll.go — The poll.GitLabClient interface contract states List* methods should "follow x-next-page", but all six implementations use a short-page heuristic (len(raw) < pollPerPage) to detect the last page instead. This is the same pattern used by existing paginated methods in issue.go and mr.go, making it a pre-existing codebase convention rather than a new defect. Consider amending the interface comment to document the short-page heuristic as the accepted pagination strategy.

  • [error-handling] internal/forge/gitlab/poll.go:263 — In CreateNoteAwardEmoji, resp.Body.Close() is called without draining the response body or checking the close error. This matches the existing codebase pattern throughout ci.go and repo.go and is standard Go practice for success-path HTTP responses.


Labels: PR adds internal GitLab forge client methods for the cron poller — maintenance/infrastructure work.

Previous run

Review

Findings

Low

  • [error-message-notation] internal/forge/gitlab/poll.go:47 — Error messages in paginated methods include extra context (e.g., "list issues updated since page %d") while existing methods in the package use terser patterns (e.g., "list open issues page %d"). Minor stylistic inconsistency; the "updated since" qualifier parallels the "open" qualifier in ListOpenIssues.

  • [comment-placement] internal/forge/gitlab/poll.go:99 — Block comment explaining the date-widening logic for the GitLab Events API appears inside the function body. Existing methods in the package typically place implementation rationale in doc comments or inline. Both approaches are valid Go idioms.

  • [pagination-constant] internal/forge/gitlab/poll.go:32 — Defines pollPerPage = 100 as a package-level constant while existing paginated methods (mr.go, issue.go) inline the literal 100. ci.go uses a function-scoped const perPage = 100, providing partial precedent. The package-level constant is arguably more maintainable.

Previous run (2)

Review

Findings

High

  • [intent-authorization] — PR title uses feat prefix for internal infrastructure work. The poll command is hidden (cmd.Hidden = true in internal/cli/poll.go), making it internal infrastructure, not a user-facing feature. The linked issue describes this as "glue work that connects the two halves." Per COMMITS.md: "feat is wrong for: Adding internal packages, helpers, or abstractions that don't change user-visible behavior" and "connecting plumbing." GoReleaser uses PR titles to build release notes, so this will appear in the Features section, misleading users about new capabilities.
    Remediation: Change PR title from feat(#5390): add polling-support methods to GitLab client to refactor(#5390): add polling-support methods to GitLab client.

Low

  • [error-handling] internal/forge/gitlab/poll.go — The error message in CreateNoteAwardEmoji uses MR-specific notation !%d for the IID regardless of noteable type. When noteableType is "Issue", the message reads e.g. create award emoji on Issue !42 note 123, but GitLab convention for issue IIDs is #42. Consider using conditional formatting or simply %d without a prefix.

Labels: PR adds GitLab forge client methods for the cron poller

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the go Pull requests that update go code label Jul 20, 2026
@ggallen

ggallen commented Jul 20, 2026

Copy link
Copy Markdown
Member

/fs-fix Rebase to get the latest sources. Add tests so that the code coverage is > 80% and CI passes. Fix all the review issues, both inline and in the sticky comment.

@fullsend-ai-coder

fullsend-ai-coder Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Finished Fix · ❌ Failure · Started 9:48 PM UTC · Completed 9:56 PM UTC
Commit: 17eed93 · View workflow run →

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor Author

⚠️ Post-fix script failed — Signed-off-by rejected (exit code 1)

The fix agent completed, but the post-fix script failed before finishing.

Workflow run: https://github.com/fullsend-ai/.fullsend/actions/runs/29781524284

Details:
Agent commit contains a Signed-off-by trailer. Agents must not use 'git commit -s' or append Signed-off-by trailers.
Please check the workflow logs for full details and retry with /fs-fix if appropriate.

@ggallen ggallen changed the title feat(#5390): add polling-support methods to GitLab client refactor(#5390): add polling-support methods to GitLab client Jul 20, 2026
Signed-off-by: Claude <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
@ggallen
ggallen force-pushed the agent/5390-gitlab-poll-methods branch from 9e7453b to 3df5b4c Compare July 20, 2026 22:14
@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.

@ggallen

ggallen commented Jul 20, 2026

Copy link
Copy Markdown
Member

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 20, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:04 PM UTC · Completed 11:19 PM UTC
Commit: 17eed93 · View workflow run →

@fullsend-ai-review
fullsend-ai-review Bot dismissed their stale review July 20, 2026 23:18

Superseded by updated review

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the ready-for-merge All reviewers approved — ready to merge label Jul 20, 2026
@ggallen

ggallen commented Jul 21, 2026

Copy link
Copy Markdown
Member

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 21, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:41 AM UTC · Completed 12:55 AM UTC
Commit: 17eed93 · View workflow run →

Comment thread internal/forge/gitlab/poll.go
@fullsend-ai-review fullsend-ai-review Bot added the type/chore Maintenance and housekeeping tasks label Jul 21, 2026
@ggallen
ggallen added this pull request to the merge queue Jul 21, 2026
Merged via the queue into main with commit 4915f0e Jul 21, 2026
36 checks passed
@ggallen
ggallen deleted the agent/5390-gitlab-poll-methods branch July 21, 2026 01:07
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 21, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 1:09 AM UTC · Completed 1:26 AM UTC
Commit: 3df5b4c · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #5391refactor(#5390): add polling-support methods to GitLab client

Timeline

This was a bot-authored PR from issue #5390. The triage agent classified the issue as type/feature and the code agent opened PR #5391 with a feat prefix (~21:25 UTC). The review agent ran automatically and flagged a HIGH finding: the PR title used feat for internal infrastructure work (should be refactor). It also found a LOW issue about error message notation in CreateNoteAwardEmoji.

At 21:47 UTC, ggallen triggered /fs-fix with instructions to rebase, add tests, and fix review findings. The fix agent completed its work successfully (fixed the error messages, added 19 tests reaching 100% coverage on poll.go, attempted to change PR title via gh pr edit), but the post-fix script rejected the commit with a false positive "Signed-off-by rejected" error. The agent had NOT used git commit -s, but because it rebased onto origin/main, the SCAN_RANGE (PRE_AGENT_HEAD..HEAD) expanded to include upstream human commits containing legitimate Signed-off-by trailers.

ggallen manually applied the fixes, force-pushed at 22:14 UTC, renamed the PR title, and after two more review cycles (the first manually triggered because the synchronize auto-review was blocked by a now-fixed permission bug), the PR was approved and merged at 01:07 UTC.

What went well

  • The review agent correctly identified the feat vs refactor prefix mismatch as a HIGH finding
  • The code agent produced high-quality implementation with 97% patch coverage on its first attempt
  • The fix agent successfully addressed all review findings and achieved 100% coverage on poll.go
  • Review findings on later passes were all LOW severity and consistent with existing codebase patterns — the agent correctly recognized when conventions matched

What went wrong

  • The post-fix script's Signed-off-by check produced a false positive after the fix agent rebased, wasting the agent's work and requiring manual human intervention
  • The triage agent mislabeled internal interface-satisfaction work as type/feature, cascading to a wrong PR title prefix that the review agent then had to catch

Evidence for existing issues (not filed as new proposals)

  • agents#272: This retro found that Signed-off-by post-script failures can also be caused by the rebase SCAN_RANGE bug (not just conflicting signing instructions). The fix proposed in agents#272 (removing "signing" from convention discovery) would not prevent this class of false positive.
  • fullsend#826: The triage agent's mislabeling of feat(gitlab): add polling-support methods to GitLab LiveClient #5390 as type/feature when it was internal glue code provides additional evidence for improving type classification granularity.
  • fullsend#2418: The fix agent tried to correct the PR title via gh pr edit but was blocked by sandbox policy — supporting evidence for enabling this capability.
  • fullsend#2105: The review agent's correct detection of the prefix mismatch as a HIGH finding validates the value of this check. The mismatch originated from triage, confirming the upstream root cause matters.

Proposals filed

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

Labels

go Pull requests that update go code ready-for-merge All reviewers approved — ready to merge ready-for-review Triggers review agent dispatch type/chore Maintenance and housekeeping tasks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(gitlab): add polling-support methods to GitLab LiveClient

1 participant