Skip to content

docs(adr): ADR 0063 — GitLab cron-polling event dispatch#2583

Open
ggallen wants to merge 1 commit into
mainfrom
worktree-gitlab-cron
Open

docs(adr): ADR 0063 — GitLab cron-polling event dispatch#2583
ggallen wants to merge 1 commit into
mainfrom
worktree-gitlab-cron

Conversation

@ggallen

@ggallen ggallen commented Jun 23, 2026

Copy link
Copy Markdown
Member

Summary

  • Add ADR 0063 documenting a two-path event dispatch model for GitLab: native CI triggers for MR events (merge_request_event) and cron-based polling for issues, comments, and label changes
  • No external infrastructure required — no webhook bridge, no Cloud Function, no webhook secrets or trigger tokens
  • Single credential per project (bot PAT via OIDC/WIF from Secret Manager)
  • Tier-adaptive: 5-min polling on Premium/Ultimate, 60-min on Free, with multi-frequency polling for slash command latency mitigation
  • Includes companion implementation plan (docs/plans/gitlab-cron-polling-implementation.md) with six phases: forge interface preparation, GitLab forge client, cron poller, CI/CD templates, CLI changes, and integration testing

Test plan

  • make lint passes (ADR status, number, frontmatter, markdown links all clean)
  • Review ADR structure against template (docs/ADRs/0000-adr-template.md)
  • Verify ADR number 0063 does not conflict with any existing or in-flight ADRs
  • Review implementation plan for completeness and consistency with ADR decisions
  • Confirm no references to PR docs: ADR 0043 — GitLab per-repo support via OIDC/WIF and webhook bridge #2042 or ADR 0043 remain in either document

🤖 Generated with Claude Code

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

ADR 0055: GitLab two-path event dispatch (native MR + cron polling)
📝 Documentation 🕐 20-40 Minutes

Grey Divider

Description

• Add ADR 0055 defining GitLab event dispatch via native MR pipelines plus scheduled polling.
• Document secretless credential flow using GitLab OIDC → GCP WIF → Secret Manager bot PAT.
• Add a phased implementation plan covering forge interface, poller, CI templates, and CLI install.
Diagram

graph TD
gl{{"GitLab project"}} --> ci["GitLab CI/CD"] --> dispatch["MR dispatch.yml"] --> agents["Agent stages"]
ci --> poll["poll.yml + fullsend poll"] --> child["Child pipelines"] --> agents
poll --> wif{{"OIDC/WIF"}} --> sm[("Secret Manager")] --> poll
poll --> gl

subgraph Legend
  direction LR
  _ext{{"External"}} ~~~ _job["Job/Template"] ~~~ _db[("Secret store")]
end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Webhook bridge Cloud Function
  • ➕ Sub-second issue/comment triggering (closer to GitHub webhook latency).
  • ➕ Webhook payload includes change metadata (reduces client-side diff/state tracking).
  • ➖ Requires externally deployed/secured inbound endpoint (operational and security burden).
  • ➖ Adds extra credentials (webhook secret + trigger token) and more setup steps.
  • ➖ Harder for self-hosted GitLab behind firewalls/VPNs.
2. Pure cron polling for all events
  • ➕ Single mechanism for every event type; no split-brain routing.
  • ➕ No inbound attack surface; fully outbound model.
  • ➖ Adds avoidable latency to MR-driven workflows (review/fix), the most frequent path.
  • ➖ More API polling and state/dedup complexity for events GitLab can natively trigger.
3. Hybrid: native MR + webhook bridge for issues/comments
  • ➕ Keeps low-latency MR dispatch while improving issue/comment latency.
  • ➕ Reduces some polling load versus fully-polled model.
  • ➖ Still requires the bridge, so most operational/security drawbacks remain.
  • ➖ Ends up maintaining two mechanisms anyway (native CI + bridge).

Recommendation: The ADR’s chosen two-path approach (native MR pipelines + cron polling for non-MR events) is the best tradeoff for GitLab: it preserves low-latency MR review dispatch while eliminating inbound webhook infrastructure and minimizing credential sprawl. The added complexity (watermarks, label-state diffing, deduplication) is contained to the poller and is a reasonable cost for the security and deployment simplicity gains.

Files changed (2) +1656 / -0

Documentation (2) +1656 / -0
0055-gitlab-cron-polling-event-dispatch.mdAdd ADR 0055 for GitLab native MR + cron-polled event dispatch +334/-0

Add ADR 0055 for GitLab native MR + cron-polled event dispatch

• Introduces an Accepted ADR specifying a two-path dispatch model: native GitLab CI triggers for merge request events and scheduled polling for issues/notes/label changes. Documents the secretless credential flow (GitLab OIDC → GCP WIF → Secret Manager bot PAT), tier-adaptive polling cadence, event routing rules, and key security constraints (protected branches, debug-trace guard, base64 payload transport, fork MR protections).

docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md

gitlab-cron-polling-implementation.mdAdd phased implementation plan for GitLab cron poller, templates, and CLI +1322/-0

Add phased implementation plan for GitLab cron poller, templates, and CLI

• Adds a detailed multi-phase plan describing required forge interface changes (new methods + ErrNotSupported + GitHubExtensions), a GitLab forge client mapping, poller architecture (watermarking, dedup, label diff state), GitLab CI/CD template structure, and admin install/uninstall flow including tier detection and pipeline schedules. Includes security-critical review checklist items and test strategy spanning unit/integration/E2E coverage.

docs/plans/gitlab-cron-polling-implementation.md

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 23, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:03 PM UTC · Completed 11:15 PM UTC
Commit: 009b93d · View workflow run →

@qodo-code-review

qodo-code-review Bot commented Jun 23, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (5)

Context used
✅ Compliance rules (platform): 51 rules

Grey Divider


Action required

1. Missing architecture.md ADR link 📜 Skill insight ⚙ Maintainability
Description
This PR adds an Accepted ADR but does not update docs/architecture.md to reflect the decided
architecture and link to the ADR. This leaves the architecture overview out of sync with the new
decision.
Code

docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md[R1-24]

+---
+title: "55. GitLab cron-polling event dispatch"
+status: Accepted
+relates_to:
+  - agent-infrastructure
+  - gitlab-implementation
+  - security-threat-model
+topics:
+  - gitlab
+  - forge
+  - ci-cd
+  - per-repo
+  - polling
+  - cron
+---
+
+# 55. GitLab cron-polling event dispatch
+
+Date: 2026-06-13
+
+## Status
+
+Accepted
+
Relevance

⭐⭐⭐ High

Team historically updates docs/architecture.md when new ADRs land (e.g., ADR additions updated
architecture.md in PRs).

PR-#250
PR-#1814

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1062100 requires an update to docs/architecture.md when an ADR is added/changed
with Accepted status. The new ADR is explicitly Accepted in both frontmatter and the Status
section, triggering the requirement.

docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md[1-24]
Skill: writing-adrs

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
An Accepted ADR was added, but `docs/architecture.md` was not updated to reference the decision.

## Issue Context
Architecture overview should include a short 'Decided:' note (or equivalent) with a link to the ADR.

## Fix Focus Areas
- docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md[1-24]
- docs/architecture.md[1-200]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. ADR adds non-template section 📜 Skill insight ⚙ Maintainability
Description
The ADR adds a ## Implementation section which is not part of the required ADR template structure.
This violates the requirement to follow docs/ADRs/0000-adr-template.md exactly.
Code

docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md[R328-334]

+## Implementation
+
+Detailed implementation guidance is in the companion document:
+[docs/plans/gitlab-cron-polling-implementation.md](../plans/gitlab-cron-polling-implementation.md).
+
+Six phases: forge interface preparation → GitLab forge client → cron poller →
+CI/CD templates → CLI changes → integration testing.
Relevance

⭐⭐ Medium

Template-exact structure enforcement unclear; history shows content tweaks but not strict “no extra
sections” policy.

PR-#161
PR-#1966

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1062099 requires ADRs to follow the template structure. The template ends after `##
Consequences and does not include an ## Implementation` section, but this new ADR adds one.

docs/ADRs/0000-adr-template.md[10-41]
docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md[328-334]
Skill: writing-adrs

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The ADR template defines the allowed sections; this ADR includes an extra `## Implementation` section.

## Issue Context
Implementation details should live in the companion plan document, while the ADR stays in the template structure.

## Fix Focus Areas
- docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md[328-334]
- docs/ADRs/0000-adr-template.md[10-41]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Context too long, unlinked 📜 Skill insight ⚙ Maintainability
Description
The ADR Context section is 4 paragraphs and does not link to the related problem docs, exceeding
the required 1–3 short paragraphs and failing to reference problem documents for background. This
makes the ADR harder to scan and duplicates context that should live in problem docs.
Code

docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md[R32-53]

+## Context
+
+Fullsend needs to detect and react to GitLab events — new issues, merge
+requests, comments, and label changes — so that agent stages (triage, code,
+review, fix, retro) can be dispatched automatically. On GitHub, native event
+triggers (`pull_request_target`, `issues`, `issue_comment`) handle this within
+GitHub Actions. GitLab has no equivalent for most event types.
+
+GitLab's CI/CD pipeline trigger sources are: `push`, `merge_request_event`,
+`schedule`, `trigger`, `web`, `api`, and `parent_pipeline`. Of these, only
+`merge_request_event` maps to an agent-relevant event. Issue creation, comment
+posting, and label changes have no native CI pipeline trigger.
+
+GitLab supports per-repo installation mode only (no per-org). The pipeline runs
+inside the enrolled project on the protected default branch. A bot project
+access token — retrieved at runtime via GitLab OIDC/GCP WIF from Secret
+Manager — serves as the single credential for all agent operations.
+
+See [ADR 0028](0028-gitlab-support.md) (deprecated) for the original GitLab
+support architecture discussion. [ADR 0045](0045-forge-portable-harness-schema.md)
+defines the forge-portable harness schema that GitLab stage templates must
+conform to.
Relevance

⭐⭐ Medium

No clear historical enforcement of strict “Context = 1–3 paragraphs + problem-doc links” ADR rule
found.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance IDs 1062090 and 1062093 require Context to be 1–3 short paragraphs and to link to
problem docs instead of restating them. In the added ADR, the Context spans four distinct
paragraphs and contains only ADR cross-references (no docs/problems/... links).

docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md[32-53]
Skill: writing-adrs

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The ADR `Context` section is longer than allowed (4 paragraphs) and does not link to relevant problem docs.

## Issue Context
ADR template requires `Context` to be 1–3 short paragraphs and to link to problem docs rather than restating background.

## Fix Focus Areas
- docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md[32-53]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. gitlab.New mismatch ✓ Resolved 🐞 Bug ≡ Correctness
Description
The plan specifies gitlab.New(token string, opts ...Option) but later examples call
gitlab.New(token, gitlabURL) / gitlab.New(token, opts.gitlabURL) as a positional argument, which
wouldn’t compile and makes base-URL configuration unclear.
Code

docs/plans/gitlab-cron-polling-implementation.md[257]

+            client, err := gitlab.New(forgeToken, gitlabURL)
Relevance

⭐⭐⭐ High

Reviewers often request fixing internal inconsistencies in plan docs (e.g., correcting wrong
referenced filenames in plans).

PR-#225

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The document defines an options-based constructor but then uses a different call signature in
multiple places, creating a compile-time inconsistency in the plan itself.

docs/plans/gitlab-cron-polling-implementation.md[119-126]
docs/plans/gitlab-cron-polling-implementation.md[247-269]
docs/plans/gitlab-cron-polling-implementation.md[1056-1066]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`docs/plans/gitlab-cron-polling-implementation.md` defines a variadic-option constructor for the GitLab client, but later code snippets call it with a second positional argument. This is internally inconsistent and would lead to compile errors or incorrect API design.

### Issue Context
The plan text says options include `WithBaseURL(url)` for self-hosted GitLab instances.

### Fix Focus Areas
- docs/plans/gitlab-cron-polling-implementation.md[119-126]
- docs/plans/gitlab-cron-polling-implementation.md[247-269]
- docs/plans/gitlab-cron-polling-implementation.md[1056-1066]

### Expected changes
- Update all examples to match the declared constructor, e.g. `gitlab.New(token, gitlab.WithBaseURL(gitlabURL))`.
- Or, if you prefer `New(token, baseURL string)`, update the constructor spec section and all mentions of `WithBaseURL(...)` accordingly.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Schedule listing omitted ✓ Resolved 🐞 Bug ⚙ Maintainability
Description
The uninstall flow requires ListPipelineSchedules(...) to discover schedules to delete, but Phase
0’s proposed forge.Client additions don’t include any schedule-listing method, leaving the plan
incomplete/inconsistent.
Code

docs/plans/gitlab-cron-polling-implementation.md[R1158-1160]

+    // 1. Delete pipeline schedules
+    schedules, _ := client.ListPipelineSchedules(ctx, owner, repo)
+    for _, s := range schedules {
Relevance

⭐⭐⭐ High

Docs/plans inconsistencies are usually addressed; prior reviews accepted fixing missing/incorrectly
referenced items in plans.

PR-#225

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The plan’s interface sketch does not include a schedule-listing method, yet the uninstall flow
sample code calls one, indicating an internal spec gap.

docs/plans/gitlab-cron-polling-implementation.md[33-42]
docs/plans/gitlab-cron-polling-implementation.md[1154-1164]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The plan’s uninstall flow depends on listing pipeline schedules, but the earlier interface changes only add create/delete schedule methods. As written, the plan can’t be implemented cleanly (or would require undocumented escape hatches).

### Issue Context
Uninstall needs to enumerate schedules to selectively delete the ones created by fullsend.

### Fix Focus Areas
- docs/plans/gitlab-cron-polling-implementation.md[33-42]
- docs/plans/gitlab-cron-polling-implementation.md[1154-1164]

### Expected changes
- Update Phase 0 to include `ListPipelineSchedules` (either on `forge.Client` or as part of a GitLab-specific extension interface) and describe expected `ErrNotSupported` behavior for GitHub.
- Alternatively, revise the uninstall flow section to match whatever interface design you intend (and ensure it’s described consistently in Phase 0/Phase 4).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Subgroup path split 🐞 Bug ≡ Correctness
Description
The plan calls splitOwnerRepo(...) on GitLab project paths (install/poll/uninstall), which would
drop nested subgroup segments and contradicts the plan’s own requirement to support
org/sub1/sub2/project namespaces.
Code

docs/plans/gitlab-cron-polling-implementation.md[R285-286]

+func (p *Poller) Run(ctx context.Context) error {
+    owner, repo := splitOwnerRepo(p.projectPath)
Relevance

⭐⭐⭐ High

Plan/spec correctness issues are typically fixed when spotted (plan docs have been corrected for
factual inconsistencies before).

PR-#225
PR-#1578

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The plan explicitly calls out nested namespace support, but the sample code still assumes a
two-segment owner/repo format via splitOwnerRepo, which is incompatible with subgroup paths.

docs/plans/gitlab-cron-polling-implementation.md[198-201]
docs/plans/gitlab-cron-polling-implementation.md[285-289]
docs/plans/gitlab-cron-polling-implementation.md[1056-1061]
docs/plans/gitlab-cron-polling-implementation.md[1154-1157]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The plan says GitLab projects can be under deeply nested namespaces and require URL-encoding the full project path or using numeric project IDs, but later examples still split project paths into `owner, repo` with `splitOwnerRepo`. If implemented as written, subgroup projects will be misaddressed.

### Issue Context
`CI_PROJECT_PATH` is typically a full namespace path (may include multiple `/`). The plan’s own subgroup section acknowledges this.

### Fix Focus Areas
- docs/plans/gitlab-cron-polling-implementation.md[198-201]
- docs/plans/gitlab-cron-polling-implementation.md[285-289]
- docs/plans/gitlab-cron-polling-implementation.md[1056-1061]
- docs/plans/gitlab-cron-polling-implementation.md[1154-1157]

### Expected changes
- Update the plan to treat GitLab identifiers as a single `projectPath` (full namespace) end-to-end, or explicitly switch to numeric `projectID` after an initial resolution call.
- Adjust method signatures shown in the plan (poller and GitLab client helper methods) to avoid assuming exactly `owner/repo`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

7. ADR exceeds 100 lines 📜 Skill insight ⚙ Maintainability
Description
The ADR content exceeds the 100-line maximum (excluding frontmatter). This indicates the ADR is
carrying too much detail that should be moved to supporting docs.
Code

docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md[R328-334]

+## Implementation
+
+Detailed implementation guidance is in the companion document:
+[docs/plans/gitlab-cron-polling-implementation.md](../plans/gitlab-cron-polling-implementation.md).
+
+Six phases: forge interface preparation → GitLab forge client → cron poller →
+CI/CD templates → CLI changes → integration testing.
Relevance

⭐ Low

Repo has many Accepted ADRs well over 100 lines; no evidence of a 100-line cap being enforced.

PR-#39
PR-#104

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1062092 caps ADR content at 100 lines (excluding frontmatter). The added ADR file
has line numbers up to 334, clearly exceeding the limit.

docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md[328-334]
Skill: writing-adrs

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
ADR content must not exceed 100 lines (excluding frontmatter), but this ADR is ~300+ lines.

## Issue Context
Keep only the minimal context, the considered options summary, the direct decision statement, and 3–5 consequence bullets; move the rest to the companion plan document.

## Fix Focus Areas
- docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md[17-334]
- docs/plans/gitlab-cron-polling-implementation.md[1-50]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. Consequences not 3-5 bullets 📜 Skill insight ⚙ Maintainability
Description
The ADR Consequences section is not formatted as 3–5 one-sentence bullet points; it contains
multiple subsections and many multi-sentence bullets. This violates the required ADR consequences
format.
Code

docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md[R266-302]

+## Consequences
+
+**What becomes easier:**
+
+- **No external infrastructure for event dispatch.** No Cloud Function, no
+  webhook bridge. Self-hosted GitLab requires only outbound HTTPS.
+- **Single credential per project.** One bot PAT in Secret Manager, retrieved
+  via OIDC/WIF. No webhook secrets, trigger tokens, or mint service changes.
+- **Stronger event authenticity.** Events read directly from the GitLab API,
+  not from potentially spoofed webhook payloads.
+- **No event loss.** Polling reads from the source of truth. Webhooks can fail
+  silently or auto-disable after 4 consecutive failures.
+- **Simpler emergency shutdown.** Disable the pipeline schedule or revoke the
+  bot PAT. No bridge to tear down.
+- **MR review latency is unaffected.** Native `merge_request_event` provides
+  sub-second triggering for the highest-frequency operation.
+- **Tier-adaptive.** Works on all GitLab tiers with graceful degradation.
+- **Reuses GCP infrastructure.** WIF and Secret Manager are already provisioned
+  for Vertex AI inference.
+
+**What becomes harder or changes:**
+
+- **Issue/comment event latency.** Up to 5 minutes on Premium, 60 minutes on
+  Free. Acceptable for asynchronous agent operations, poor for interactive use
+  on Free tier.
+- **CI minute consumption.** Polling runs continuously. At 5-minute intervals:
+  ~8,640 min/month on shared runners. Self-hosted runners are not billed.
+- **State management.** The poller must track watermarks, deduplicate events
+  across overlapping windows, and diff label state.
+- **Slash command latency.** Up to 5 minutes vs sub-second with webhooks.
+  Labels mitigate this for common operations.
+- **Quick Action stripping.** GitLab may strip `/fs-*` commands from comments.
+  Requires testing and potentially alternative syntax.
+- **Per-repo only.** No centralized config or credential management across
+  projects.
+- **`api` scope is broad.** Narrower scopes are not available in GitLab today.
+
Relevance

⭐ Low

Consequences formatting not strictly enforced; accepted ADRs have >5 bullets in Consequences (e.g.,
ADR 0003).

PR-#80

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 1062091 requires the Consequences section to be 3–5 one-sentence bullets. The
added ADR includes multiple subsections (e.g., "What becomes easier", "What becomes harder",
"Risks") and far more than 5 bullets, with several bullets spanning multiple sentences.

docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md[266-302]
Skill: writing-adrs

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The ADR `Consequences` section must be 3–5 one-sentence bullet points, but it currently includes many bullets and multi-sentence content across multiple subsections.

## Issue Context
Keep the ADR concise; move extended rationale/comparisons/risks to the companion plan document.

## Fix Focus Areas
- docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md[266-302]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md
Comment thread docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md Outdated
Comment thread docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md
@codecov

codecov Bot commented Jun 23, 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

fullsend-ai-review Bot commented Jun 23, 2026

Copy link
Copy Markdown

Looks good to me

Findings

Low

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdisProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could produce false positives for human usernames on self-hosted GitLab instances. The plan documents and accepts this — blast radius is limited to fast-poll slash command routing only (full-poll uses the authoritative Author.Bot field).

  • [error-handling] docs/plans/gitlab-cron-polling-implementation.mdpersistLabelState logs warnings but does not return errors when json.Marshal or UpdateVariable fails. Silent failure causes duplicate dispatches on next poll. Mitigated by resource_group concurrency control, but implementers should consider propagating errors.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.md — The detectNewLabels pruning loop calls isIssueClosed per tracked IID not in the current poll set — O(n) individual API calls with no batch alternative or documented cap. Consider batch-fetching issue states via ListOpenIssues during implementation.

  • [YAML-injection] docs/plans/gitlab-cron-polling-implementation.mdgenerateChildPipelineYAML uses fmt.Fprintf string concatenation to produce YAML. Currently safe because all interpolated fields (d.Stage, d.EventType) come from hardcoded constants or base64-encoded payloads. During implementation, consider using a YAML serialization library to eliminate the class of YAML injection vulnerabilities regardless of future field additions.

  • [authorization-bypass] docs/plans/gitlab-cron-polling-implementation.md — Fast-poll bot detection uses username heuristic (isProjectAccessTokenBot) instead of Author.Bot field. The Events API does not expose Author.Bot, so the heuristic is a documented tradeoff. Consider cross-checking author.id against the configured botUserID (already on the Poller struct) for more reliable bot detection.

  • [cross-reference-staleness] docs/problems/agent-architecture.md — Lines 148 and 208 reference "GitHub events" as coordination primitives and FSM transitions. With ADR 0063 establishing forge-neutral event dispatch, these could be updated to "forge events" in a follow-up PR (this is a problem doc, not an accepted ADR, so edits are permitted).

Previous run

Looks good to me

Findings

Low

  • [api-contract-violation] docs/plans/gitlab-cron-polling-implementation.md:2024 — The install flow pseudocode calls client.CommitFiles(ctx, owner, repo, project.DefaultBranch, ...) with 6 arguments including a branch parameter. The actual forge.Client.CommitFiles signature takes 5 arguments (ctx, owner, repo, message, files) with no branch parameter. The correct method for committing to a specific branch is CommitFilesToBranch(ctx, owner, repo, branch, message, files).

  • [api-contract-violation] docs/plans/gitlab-cron-polling-implementation.md:668 — The Phase 1 method mapping table lists methods under the column header forge.Client method that do not exist on the current interface: CreatePR (actual: CreateChangeProposal), ListPRs (actual: ListRepoPullRequests), GetBranch (actual: GetBranchRef). An implementer following this table would get compile errors.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.md:1102isProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could produce false positives for human usernames on self-hosted GitLab instances. The plan acknowledges and scopes blast radius to fast-poll mode only.

  • [error-handling] docs/plans/gitlab-cron-polling-implementation.md:1468persistLabelState logs warnings but does not return errors when json.Marshal or UpdateVariable fails. Silent failure causes duplicate dispatches on next poll. Mitigated by resource_group concurrency control.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.md:1456 — The detectNewLabels pruning loop calls isIssueClosed per tracked IID not in the current poll set — O(n) individual API calls with no batch alternative or documented cap.

  • [missing-authorization] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — Non-trivial change (new ADR + ~2000 line implementation plan) with no formally linked issue. The PR body mentions issues GitLab support via webhook bridge (ADR 0043) #1964 and docs: ADR 0043 — GitLab per-repo support via OIDC/WIF and webhook bridge #2042 in passing but does not formally link them via Closes or Fixes. Adding Relates to #1964 would improve automated traceability.

  • [adr-numbering-conflict] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — ADR 0063 fills an available slot between existing 0062 and 0064. Verify before merge that no other PR has claimed this number concurrently (the renumber-adr skill can check).

  • [supersession-incomplete] docs/ADRs/0028-gitlab-support.md — ADR 0028's status annotation now lists two partial supersessions (ADR 0045 for harness schema, ADR 0063 for webhook bridge). The scoping is reasonably clear but could be more explicit about which sections remain reference material.

Previous run (2)

Looks good to me

Findings

Low

  • [api-contract-violation] docs/plans/gitlab-cron-polling-implementation.md:2024 — The install flow pseudocode calls client.CommitFiles(ctx, owner, repo, project.DefaultBranch, ...) with 6 arguments including a branch parameter. The actual forge.Client.CommitFiles signature takes 5 arguments (ctx, owner, repo, message, files) with no branch parameter. The correct method is CommitFilesToBranch.

  • [api-contract-violation] docs/plans/gitlab-cron-polling-implementation.md:668 — The Phase 1 method mapping table lists methods under the column header forge.Client method that do not exist on the current interface (e.g., CreatePR instead of CreateChangeProposal, ListPRs instead of ListRepoPullRequests, GetBranch instead of GetBranchRef). An implementer would need to map these to actual method names.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.md:1102isProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could produce false positives for human usernames on self-hosted GitLab instances. The plan acknowledges and scopes blast radius to fast-poll mode only.

  • [error-handling] docs/plans/gitlab-cron-polling-implementation.md:1468persistLabelState logs warnings but does not return errors when json.Marshal or UpdateVariable fails. Silent failure causes duplicate dispatches on next poll, mitigated by resource_group concurrency control.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.md:1456 — The detectNewLabels pruning loop calls isIssueClosed per tracked IID not in the current poll set — O(n) individual API calls with no batch alternative or documented cap.

Previous run (3)

Looks good to me

Findings

Low

  • [api-contract-violation] docs/plans/gitlab-cron-polling-implementation.md:2024 — The install flow pseudocode calls client.CommitFiles(ctx, owner, repo, project.DefaultBranch, ...) with 6 arguments including a branch parameter. The actual forge.Client.CommitFiles signature takes 5 arguments with no branch parameter. The correct method is CommitFilesToBranch.

  • [api-contract-violation] docs/plans/gitlab-cron-polling-implementation.md:668 — The Phase 1 method mapping table lists methods under the column header forge.Client method that do not exist on the current interface (e.g., CreatePR instead of CreateChangeProposal, ListPRs instead of ListRepoPullRequests, GetBranch instead of GetBranchRef). An implementer would need to map these to actual method names.

  • [consumer-completeness] docs/plans/gitlab-cron-polling-implementation.md:598 — The ErrNotSupported caller handling audit says CreateOrgSecret/OrgSecretExists callers are in the "secrets layer." The actual callers are in internal/layers/dispatch.go (the dispatch layer) and internal/cli/github.go.

  • [missing-authorization] — PR references issue GitLab support via webhook bridge (ADR 0043) #1964 in the body and roadmap but lacks a formal GitHub issue link. The work is clearly authorized; adding Relates to #1964 to the PR description would improve automated traceability.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.md:1102isProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could produce false positives for human usernames on self-hosted GitLab instances. The plan acknowledges and scopes blast radius to fast-poll mode only.

  • [error-handling] docs/plans/gitlab-cron-polling-implementation.md:1468persistLabelState logs warnings but does not return errors when json.Marshal or UpdateVariable fails. Silent failure causes duplicate dispatches on next poll, mitigated by resource_group concurrency control.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.md:1456 — The detectNewLabels pruning loop calls p.isIssueClosed per tracked IID not in the current poll set — O(n) individual API calls with no batch alternative or documented cap.

  • [ADR structure] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — ADR 0063 lacks a formal ## References section. ADR 0057 includes one, establishing a convention for listing related ADRs and companion documents.

Previous run (4)

Review

Findings

Medium

  • [consumer-completeness] docs/plans/gitlab-cron-polling-implementation.md:596 — The ErrNotSupported caller handling audit references function names that do not exist in the codebase: runEnrollActions and runUnenrollActions in internal/layers/enrollment.go. The actual functions containing DispatchWorkflow calls are Install (line 89, via dispatchRepoMaintenanceWithRetry at line 141/159) and Uninstall (line 301/348). While the file reference and handling behavior are described correctly, incorrect function names could mislead implementers attempting to locate and modify these call sites during Phase 0.
    Remediation: Replace runEnrollActions with Install (or dispatchRepoMaintenanceWithRetry) and runUnenrollActions with Uninstall in the caller handling audit at line 596.

Low

  • [logic-error] docs/plans/gitlab-cron-polling-implementation.md:1129 — In discoverSlashCommands (fast-poll path), the filter uses strings.HasPrefix(evt.Note.Body, "/fs-") without trimming whitespace. In contrast, routeIssueNote and routeMRNote both call strings.TrimSpace before extracting the command token. A slash command with leading whitespace would be discovered and routed correctly by the full-poll path but silently dropped by the fast-poll path.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.md:1102isProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could produce false positives for human usernames on self-hosted GitLab instances. A false positive would cause a legitimate slash command to be silently dropped in fast-poll mode. Full-poll mode uses the Notes API Author.Bot field instead, limiting blast radius. The plan already documents and accepts this inconsistency.

  • [error-handling] docs/plans/gitlab-cron-polling-implementation.md:1468persistLabelState logs warnings but does not return errors when json.Marshal or UpdateVariable fails. If label state persistence fails silently, the next poll will re-detect the same labels as "new" and dispatch duplicate agent stages. While resource_group concurrency control mitigates conflict, the duplicates consume CI minutes.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.md:1457 — The pruning loop in detectNewLabels calls isIssueClosed for every IID in previousState that is NOT in the current poll set. For projects with many historically-tracked issues, this could generate one GitLab API call per tracked issue per full-poll cycle. The cost is self-limiting (closed issues are pruned from state after detection) but no batch API is proposed.

Previous run (5)

Review

Findings

Medium

  • [consumer-completeness] docs/plans/gitlab-cron-polling-implementation.md — The plan's ErrNotSupported caller handling audit lists "DispatchWorkflow callers (dispatch layer)" but no DispatchWorkflow call exists in internal/layers/dispatch.go or internal/layers/workflows.go. Meanwhile, internal/cli/admin.go:2606 calls DispatchWorkflow to trigger repo-maintenance.yml after enrollment config changes — this call site is not listed in the audit and will need ErrNotSupported handling for GitLab.
    Remediation: Update the caller handling section: remove the "dispatch layer" entry (no such call site exists), and add internal/cli/admin.go's DispatchWorkflow call with documented handling behavior for ErrNotSupported.

  • [logic-error] docs/plans/gitlab-cron-polling-implementation.md — In detectNewLabels, the pruning loop iterates over all IIDs in previousState and deletes entries for closed issues. However, this runs after the first loop has already updated previousState with current labels from polled issues. If a polled issue was just closed, the pruning loop will delete its entry, discarding the label state that was just computed. On the next poll, if this closed issue's updated_after timestamp keeps it in the query window, its labels will be re-detected as "new" and cause duplicate dispatches.
    Remediation: In the pruning loop, skip IIDs that were in the current poll's issues set, since their label state was just updated and should not be pruned.

Low

  • [stale-implementation-details] docs/problems/gitlab-implementation.md — The document contains extensive webhook-based implementation details that are superseded by ADR 0063's cron-polling approach. The top-level note added by this PR is adequate, but readers navigating directly to specific sections (e.g., "Webhook Payload Validation", "Trigger Token Creation") may not realize the content is obsolete. Consider adding section-level deprecation notices to obsolete sections.

  • [YAML-injection] docs/plans/gitlab-cron-polling-implementation.mdgenerateChildPipelineYAML uses %q for most fields but d.Stage is interpolated unquoted in the include: path (.gitlab/ci/fullsend-%s.yml). Currently safe because d.Stage comes from hardcoded constants in routeEvent, but the invariant should be enforced with an explicit validation gate during implementation.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdisProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could produce false positives for human usernames on self-hosted GitLab instances. A false positive silently drops the user's slash command with no diagnostic feedback in the fast-poll path. The plan already documents and accepts the inconsistency between fast-poll (username heuristic) and full-poll (Author.Bot field) bot detection.
    Remediation: Log when a note is skipped due to bot detection in the fast-poll path, providing a diagnostic signal for affected users.

  • [error-handling] docs/plans/gitlab-cron-polling-implementation.mdpersistLabelState logs warnings but does not return errors when json.Marshal or UpdateVariable fails. Persistent failures cause repeated duplicate dispatches (mitigated by resource_group concurrency control but wasteful).

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mddetectNewLabels calls isIssueClosed for every IID in previousState — one GitLab API call per tracked issue per poll cycle. O(n) API cost with no batching proposed.
    Remediation: Batch-fetch issue states using GET /projects/:id/issues?iids[]=1&iids[]=2&...&state=closed to reduce API calls to O(n/100) per poll.

  • [logic-error] docs/plans/gitlab-cron-polling-implementation.md — The MR dispatch template determines the stage by querying MR state via API after merge_request_event triggers the pipeline. A narrow race window exists where the MR state could change between trigger and API query (e.g., MR opened then immediately merged). In practice the consequence is benign — routing to the correct stage for the MR's current state.

Previous run (6)

Review

Findings

Low

  • [missing-authorization] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — PR has no formally linked issue. The roadmap references GitLab support via webhook bridge (ADR 0043) #1964 as the related GitLab work item, and the PR is authored by a member with appropriate component labels. Linking the authorizing issue would improve traceability.

  • [internal-consistency] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — The ADR security model section states slash command authorization requires Developer-level (30+) access via /fs-* commands. The companion plan's routeIssueNote allows Guest+ users to trigger triage via non-command comments on issues with the needs-info label (documented in Security-Critical Code Path docs: add agent infrastructure problem document #5). While the ADR statement is technically scoped to slash commands only, noting the needs-info exception in the ADR security model would improve completeness.

  • [ADR-section-structure] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — The Decision section includes substantial implementation guidance (credential models, event routing table, tier table, security model, forge abstraction signatures). Established ADR patterns (0057, 0062) tend to keep Decision sections more concise. However, ADR 0063 documents a complex multi-path dispatch architecture where security and credential details are integral to the decision, and the companion plan already separates the detailed pseudocode.

  • [logic-error] docs/plans/gitlab-cron-polling-implementation.md — The MR dispatch template determines the stage by querying MR state via API after merge_request_event triggers the pipeline. A narrow race window exists where the MR state could change between trigger and API query (e.g., opened MR merged before dispatch job runs). In practice the consequence is benign — routing to the correct stage for the MR's current state — and the implementation can address timing concerns.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mddetectNewLabels calls isIssueClosed for every IID in previousState — one GitLab API call per tracked issue per poll cycle. O(n) API cost with no batching proposed. Consider batching via a single ListIssues call with state=closed filter during implementation.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdisProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could match human usernames on self-hosted GitLab instances (false positive → slash commands silently skipped). Blast radius is limited to fast-poll slash command routing only; full-poll uses the authoritative Author.Bot field.

  • [error-handling] docs/plans/gitlab-cron-polling-implementation.mdpersistLabelState logs warnings but does not return errors when json.Marshal or UpdateVariable fails. Persistent failures cause duplicate dispatches (labels re-detected as "new"), mitigated by resource_group concurrency control but not duplicate pipeline creation.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdreadWatermark falls back to time.Now().Add(-1 * time.Hour) on first run. The install flow initializes watermarks to time.Now(), meaning events created before install are not detected. This is reasonable for fresh installs but worth noting for the implementation.

  • [YAML-injection] docs/plans/gitlab-cron-polling-implementation.mdgenerateChildPipelineYAML uses %q for most fields but d.Stage is interpolated unquoted in the include: path. Currently safe because d.Stage comes from hardcoded constants in routeEvent, but the invariant should be enforced with an explicit validation gate during implementation.

  • [cross-reference-completeness] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — Final paragraph references the companion plan document. Phrasing could be more explicit about what the plan contains, matching ADR 0057's pattern of listing the topics covered.

Previous run (7)

Review

Findings

Medium

  • [internal-consistency] docs/plans/gitlab-cron-polling-implementation.md — The prose description of the CI/CD variable size limit mitigation says "fall back to treating all matching labels as new" when the label state exceeds GitLab's 10,000-character limit, but the pseudocode in detectNewLabels returns a hard error from json.Unmarshal when the stored JSON is truncated. A truncated JSON string will fail to unmarshal, propagate through discoverAllEvents, and cause Run() to abort the entire poll cycle — no fallback occurs. The poller will permanently fail once label state exceeds the variable limit, rather than degrading gracefully as described.
    Remediation: Either update the pseudocode to catch the unmarshal error and fall back to an empty LabelState (treating all labels as new), or update the prose to state that exceeding the limit is a fatal error requiring manual cleanup of the FULLSEND_LABEL_STATE variable.

  • [ADR-section-structure] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — The Decision section includes substantial implementation guidance (credential model details, cron poller numbered steps, event routing table, slash command latency mitigations, GitLab tier table, security model, forge abstraction method signatures). Established ADR pattern (0045, 0057, 0062) keeps Decision focused on the architectural choice and defers implementation mechanics to companion plans. Consider moving the numbered poller steps, security comparison tables, and tier matrices to the companion plan, keeping the Decision section focused on what was decided and why.

  • [error-handling-documentation-pattern] docs/plans/gitlab-cron-polling-implementation.md — Phase 0 "Caller handling" subsection references specific line numbers (internal/layers/enrollment.go lines 159, 348). Line numbers drift as the codebase evolves, creating maintenance burden. Established plan documents reference files and function/subsystem names as more stable anchors.
    Remediation: Replace line number references with function or subsystem names (e.g., "enrollment layer repo-maintenance dispatch in internal/layers/enrollment.go").

Low

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mddetectNewLabels calls isIssueClosed for every IID in previousState — one GitLab API call per tracked issue per poll cycle. O(n) API cost with no batching proposed. Practical impact is limited since only fullsend-routable labels are tracked (small set), but consider batching via a single ListIssues call with state=closed filter during implementation.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdisProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could match human usernames on self-hosted GitLab instances (false positive → slash commands silently skipped). Blast radius is limited to fast-poll slash command routing only; full-poll uses the authoritative Author.Bot field.

  • [error-handling] docs/plans/gitlab-cron-polling-implementation.mdpersistLabelState logs warnings but does not return errors when json.Marshal or UpdateVariable fails. Persistent failures cause duplicate dispatches (labels re-detected as "new"), mitigated by resource_group concurrency control but not duplicate pipeline creation.

  • [YAML-injection] docs/plans/gitlab-cron-polling-implementation.mdgenerateChildPipelineYAML uses %q for most fields but d.Stage is interpolated unquoted in the include: path (.gitlab/ci/fullsend-%s.yml). Currently safe because d.Stage comes from hardcoded constants in routeEvent, but the invariant should be enforced with an explicit validation gate during implementation.

  • [scope-alignment] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — This ADR represents an architectural pivot from the webhook bridge approach. The roadmap references GitLab support via webhook bridge (ADR 0043) #1964 as the related GitLab work item but the issue is not formally linked to this PR. The roadmap diff and PR body both reference GitLab support via webhook bridge (ADR 0043) #1964.

  • [stale-reference-to-superseded-approach] docs/problems/gitlab-implementation.md — The document body still describes the webhook bridge architecture superseded by ADR 0063. The PR adds a prominent supersession note at the top and a "Decided" annotation to the open question, which is the standard pattern for problem docs. The remaining webhook-related sections are retained as reference material per the note.

  • [code-formatting-conventions] docs/plans/gitlab-cron-polling-implementation.md — Pseudocode blocks include inline comments explaining rationale (hybrid spec/implementation style). Established plan documents tend to keep narrative explanations in surrounding prose rather than inline code comments.

  • [Consequences-section-structure] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — Consequences section uses subsection headers ("What becomes easier", "What becomes harder", "Risks", "Comparison with GitHub") that deviate from the template's simpler format, though both approaches exist in the ADR corpus.

  • [cross-reference-completeness] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — Final line references the companion plan document. Phrasing could be more explicit about what the plan contains, matching ADR 0057's pattern of listing the topics covered.

Previous run (8)

Looks good to me

Findings

Low

  • [logic-error] docs/plans/gitlab-cron-polling-implementation.md — The fullsend-dispatch.yml template queries MR state using CI_JOB_TOKEN authentication, while other templates retrieve the bot PAT first. This is intentional: the dispatch template only needs to read the MR state of the same project (a read operation CI_JOB_TOKEN supports), while other templates need the bot PAT for write operations. The ADR credential model statement "One bot PAT per project handles all REST and GraphQL operations" refers to agent stage operations, not the dispatch routing step.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mddetectNewLabels calls isIssueClosed for every IID in previousState — one GitLab API call per tracked issue per poll cycle. O(n) API cost not documented, no batching proposed. Practical impact is limited since only fullsend-routable labels are tracked (small set), but consider batching via a single ListIssues call with state=closed filter during implementation.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdFULLSEND_LABEL_STATE CI/CD variable has a 10,000-character limit. The mitigation text says "fall back to treating all matching labels as new" but the pseudocode returns an error from json.Unmarshal when the stored value is truncated. Clarify the intended recovery behavior during implementation.

  • [scope-creep] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — This ADR represents an architectural pivot from the webhook bridge approach documented in ADR 0028. The roadmap references GitLab support via webhook bridge (ADR 0043) #1964 as the related GitLab work item but the issue is not formally linked to this PR. The roadmap diff and PR body both link GitLab support via webhook bridge (ADR 0043) #1964 to this work.

  • [architectural-coherence] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — The cron-polling model keeps all state within the enrolled project (CI/CD variables, pipeline schedules, protected branches) and requires no external orchestrator, consistent with the "repo is the coordinator" principle. The ADR's Consequences section confirms state is internal to the GitLab forge implementation.

  • [YAML-injection] docs/plans/gitlab-cron-polling-implementation.mdgenerateChildPipelineYAML uses %q for most fields but d.Stage is interpolated unquoted in the include: path (.gitlab/ci/fullsend-%s.yml). Currently safe because d.Stage comes from hardcoded constants in routeEvent, but the invariant that stage values must come from a fixed allowlist should be enforced with an explicit validation gate during implementation.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdisProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could match human usernames on self-hosted GitLab instances (false positive → slash commands silently skipped). Conversely, if GitLab changes the bot username scheme, bot comments could be misclassified. Blast radius is limited to fast-poll slash command routing only; full-poll uses the authoritative Author.Bot field.

  • [error-handling] docs/plans/gitlab-cron-polling-implementation.mdpersistLabelState logs warnings but does not return errors when json.Marshal or UpdateVariable fails. Persistent failures cause duplicate dispatches (labels re-detected as "new"), mitigated by resource_group concurrency control.

  • [internal-consistency] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — The architecture.md addition says "Bot PAT via OIDC/WIF from Secret Manager" but the ADR defines two credential modes — WIF (recommended) and protected CI/CD variable (fallback). The architecture.md summary only mentions WIF, omitting the variable mode fallback.

Previous run (9)

Review

Findings

Medium

  • [scope-creep] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — This ADR represents a fundamental architectural pivot from the webhook bridge approach documented in ADR 0028. The webhook bridge approach was the documented direction, but this ADR rejects all webhook-based options (Options 1–3) in favor of cron-polling. This is a strategic-level decision within the GitLab support problem space. The roadmap references GitLab support via webhook bridge (ADR 0043) #1964 as the related GitLab work item but the issue is not formally linked to this PR.
    Remediation: Verify that issue GitLab support via webhook bridge (ADR 0043) #1964 (or another strategic issue) authorizes exploring alternatives to the webhook bridge, and formally link it.

  • [logic-error] docs/plans/gitlab-cron-polling-implementation.md — The CI/CD template YAML files specify image: ghcr.io/fullsend-ai/fullsend:latest, but this image does not exist in the repository's container image definitions. The actual published images are ghcr.io/fullsend-ai/fullsend-sandbox:latest (base sandbox) and ghcr.io/fullsend-ai/fullsend-code:latest (code agent). No Containerfile or workflow produces ghcr.io/fullsend-ai/fullsend:latest. If implemented as written, all GitLab CI jobs would fail with an image pull error.
    Remediation: Replace ghcr.io/fullsend-ai/fullsend:latest with the appropriate existing image. For dispatch and poll jobs, use ghcr.io/fullsend-ai/fullsend-sandbox:latest. For stage templates that run agents, use ghcr.io/fullsend-ai/fullsend-code:latest.

Low

  • [internal-consistency] docs/plans/gitlab-cron-polling-implementation.md — The fullsend-dispatch.yml template queries MR state using curl with CI_JOB_TOKEN authentication, while other templates retrieve the bot PAT first. In merge_request_event pipelines, CI_JOB_TOKEN should have read access to the project's own MR API, making this correct for a same-project state query. The asymmetry is intentional (dispatch determines stage before credential retrieval happens in the stage job).

  • [YAML-injection] docs/plans/gitlab-cron-polling-implementation.mdgenerateChildPipelineYAML uses %q for most fields but d.Stage is interpolated unquoted in the include: path (.gitlab/ci/fullsend-%s.yml). Currently safe because d.Stage comes from hardcoded constants in routeEvent, but the invariant that stage values must come from a fixed allowlist should be documented explicitly.

  • [missing-authorization] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — Non-trivial PR with no formally linked issue. The PR body references GitLab support via webhook bridge (ADR 0043) #1964 and the roadmap diff links GitLab support via webhook bridge (ADR 0043) #1964 to this work, suggesting authorization exists but is not formally linked via GitHub's issue-closing keywords.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mddetectNewLabels calls isIssueClosed for every IID in previousState — one API call per tracked issue per poll cycle. The state only tracks routable labels, which bounds the set, but projects with many historically-labeled issues could see high API call volume.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdisProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could match human usernames on self-hosted GitLab instances. Blast radius is limited to fast-poll slash command routing only.

  • [error-handling] docs/plans/gitlab-cron-polling-implementation.mdpersistLabelState logs warnings but does not return errors when json.Marshal or UpdateVariable fails. Persistent failures cause duplicate dispatches (labels re-detected as "new"), mitigated by resource_group concurrency control.

  • [ADR-frontmatter-consistency] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — The ADR uses relates_to: agent-infrastructure but other GitLab-related ADRs (0028, 0045) use both agent-infrastructure and agent-architecture. Consider adding agent-architecture for consistency.

Previous run (10)

Review

Findings

Medium

  • [logic-error] docs/plans/gitlab-cron-polling-implementation.md — The fullsend-dispatch.yml template uses image: alpine:latest and calls jq -r '.state' in the script, but Alpine Linux does not include jq by default. The pipeline job would fail at runtime with jq: not found. All other stage templates in the same document use ghcr.io/fullsend-ai/fullsend:latest.
    Remediation: Change the dispatch job image to ghcr.io/fullsend-ai/fullsend:latest (consistent with other stage templates), or add apk add --no-cache curl jq as the first script line.

  • [internal-consistency] docs/plans/gitlab-cron-polling-implementation.md — The CI/CD variable size limit mitigation text states "only track issues with fullsend-relevant labels (fullsend:*)" but the detectNewLabels pseudocode tracks ALL labels on every issue (previousState[issue.IID] = issue.Labels), not just fullsend-relevant ones. The routeIssueLabel function only routes ready-to-code and ready-for-review, yet the label state stores every label. This means the 10,000-character CI/CD variable limit will be reached much sooner than the mitigation text implies.
    Remediation: Either update the pseudocode to filter labels to only fullsend-routable ones (e.g., ready-to-code, ready-for-review, needs-info) before storing in previousState, or update the mitigation text to accurately reflect that all labels are tracked.

  • [missing-authorization] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — Non-trivial PR with no linked issue. This PR adds a new ADR (421 lines), implementation plan (1,691 lines), and updates to multiple architectural documents. The roadmap changes reference GitLab support via webhook bridge (ADR 0043) #1964 as the related GitLab work item, suggesting authorization exists but is not formally linked.

Low

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mddetectNewLabels calls isIssueClosed for every IID in previousState — one API call per tracked issue per poll cycle. As the number of tracked issues grows, this becomes O(N) API calls on every full-poll cycle. Consider batching via a single ListIssues call with state=closed filter.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdisProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could match human usernames on self-hosted GitLab instances. Blast radius is limited to fast-poll slash command routing only.

  • [error-handling] docs/plans/gitlab-cron-polling-implementation.mdpersistLabelState logs warnings but does not return errors when json.Marshal or UpdateVariable fails. Persistent failures cause duplicate dispatches (labels re-detected as "new"), mitigated by resource_group concurrency control.

Previous run (11)

Looks good to me

Findings

Low

  • [architectural annotation terminology] docs/architecture.md — New entries use "(implementation pending)" annotation pattern not found elsewhere in architecture.md. Other entries describe features in present tense regardless of implementation status.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mddetectNewLabels calls isIssueClosed for every IID in previousState — one API call per tracked issue per poll cycle. As the number of tracked issues grows, this becomes O(N) API calls on every full-poll cycle. Consider batching via a single ListIssues call with state=closed filter.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdisProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could match human usernames on self-hosted GitLab instances. Blast radius is limited to fast-poll slash command routing only.

  • [error-handling] docs/plans/gitlab-cron-polling-implementation.mdpersistLabelState logs warnings but does not return errors when json.Marshal or UpdateVariable fails. Persistent failures cause duplicate dispatches (labels re-detected as "new"), mitigated by resource_group concurrency control.

Previous run (12)

Looks good to me

Findings

Low

  • [logic-error] docs/plans/gitlab-cron-polling-implementation.md — The Poller struct declares botUserID int and routeMRNote uses event.NoteAuthorID != p.botUserID to verify the changes-requested marker was authored by the enrolled bot. The Options struct includes a BotUserID int field and Run assigns p.botUserID = p.opts.BotUserID, but the newPollCmd constructor does not pass BotUserID in the Options literal. As a zero-valued int (0), the comparison always evaluates to true, so bot-authored changes-requested markers would be silently ignored. Remediation: populate BotUserID during initialization via GET /user with the bot PAT or store as a CI/CD variable during install.

  • [missing-authorization] — PR has no linked issue. This is a substantial docs-only change (new ADR + implementation plan). While no production code is changed, traceability to authorized work is recommended.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mddetectNewLabels calls isIssueClosed for every IID in previousState — one API call per tracked issue per poll cycle. The plan notes filtering to fullsend-relevant labels as mitigation but the pseudocode does not implement this filtering.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdisProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could match human usernames on self-hosted GitLab instances. Blast radius limited to fast-poll mode (full-poll uses Author.Bot field).

  • [error-handling] docs/plans/gitlab-cron-polling-implementation.mdpersistLabelState logs warnings but does not return errors when json.Marshal or UpdateVariable fails. Duplicate dispatches mitigated by resource_group concurrency control.

  • [internal-consistency] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — In the Risks section, the first two risks are both numbered 1. in the markdown source. The section header says "ordered by threat priority" but the duplicate numbering obscures whether YAML injection and prompt injection share priority rank.

  • [credential-exposure] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — In variable mode, the CI_DEBUG_TRACE guard is acknowledged as the sole defense against PAT exposure, but GitLab logs CI/CD variables at job init before any script runs. A Maintainer can re-add CI_DEBUG_TRACE post-install. The ADR recommends WIF mode for untrusted Maintainer pools, which is appropriate mitigation.

Previous run (13)

Review

Findings

Medium

  • [logic-error] docs/plans/gitlab-cron-polling-implementation.md — The Poller struct declares botUserID int and routeMRNote references p.botUserID to verify the changes-requested marker was authored by the enrolled bot, but botUserID is never assigned in the constructor or Run method. As a zero-valued int (0), the comparison event.NoteAuthorID != p.botUserID is always true (no real GitLab user has ID 0), so bot-authored changes-requested markers are silently ignored and never trigger the fix stage. Remediation: add botUserID as a constructor parameter or Options field, populated during initialization via GET /user with the bot PAT.

Low

  • [internal-consistency] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — The slash command latency mitigations section states labels have "no polling latency on the native MR CI path," but the event routing table shows label additions are transported via "Cron poll (label state diff)." Labels always have polling latency (5–60 minutes).

  • [credential-exposure] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — In variable mode, the CI_DEBUG_TRACE guard is acknowledged as the sole defense against PAT exposure, but GitLab logs CI/CD variables at job init before any script runs. A Maintainer can re-add CI_DEBUG_TRACE post-install. The ADR recommends WIF mode for untrusted Maintainer pools, which is appropriate mitigation.

  • [excessive-permission-scope] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — The bot PAT requires api scope (GitLab's broadest). The ADR correctly notes narrower scopes are unavailable in GitLab today.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mddetectNewLabels calls isIssueClosed for every IID in previousState — one API call per tracked issue per poll cycle. The plan notes filtering to fullsend-relevant labels as mitigation but the pseudocode does not implement this filtering.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdisProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could match human usernames on self-hosted GitLab instances. Blast radius limited to fast-poll mode (full-poll uses Author.Bot field).

  • [error-handling] docs/plans/gitlab-cron-polling-implementation.mdpersistLabelState logs warnings but does not return errors when json.Marshal or UpdateVariable fails. Duplicate dispatches mitigated by resource_group concurrency control.

  • [missing-authorization] — PR has no linked issue. This is a substantial docs-only change (new ADR + implementation plan). While no production code is changed, traceability to authorized work is recommended.

  • [documentation-coherence] docs/problems/gitlab-implementation.md — The problem doc retains ~624 lines of webhook-based implementation guidance now superseded by ADR 0063. The deprecation notice correctly redirects readers, but the obsolete content may cause confusion.

Previous run (14)

Review

Findings

Medium

  • [logic-error] docs/plans/gitlab-cron-polling-implementation.md — The Poller struct declares fields client, projectPath, owner, repo, opts, and accessCache but not botUserID. routeMRNote references p.botUserID which, as a zero-valued int, causes event.NoteAuthorID != p.botUserID to always be true (real GitLab user IDs are always positive). Every bot-authored changes-requested marker is rejected, so the fix stage is never triggered by the review bot's marker. Remediation: add botUserID int to the Poller struct and initialize it during construction.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.md — In discoverAllEvents, when ListMergeRequestsUpdatedSince fails, the function returns issue events with no error. However, minSkippedAt is not set for the MR list failure — only for individual MR note-fetch failures. Since the watermark is capped at minSkippedAt (step 5 of the poll loop), a persistent MR API failure will not prevent the watermark from advancing past MR events. Those MR note events (e.g., changes-requested markers) will be permanently lost. Remediation: when ListMergeRequestsUpdatedSince fails, set minSkippedAt to the since watermark.

  • [naming-convention] docs/glossary.md:42 — The Debouncing entry uses anchor text event dispatch layer but links to ADR 0002 section titled "Webhook + dispatch service" (#1-webhook--dispatch-service). The anchor text/URL mismatch could confuse readers navigating to the linked section.

  • [architectural-divergence] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — The Event Routing section frames the design goal as "event parity with GitHub," but the Consequences section acknowledges 5-minute latency on Premium and 60-minute latency on Free tier for issue/comment events (vs sub-second on GitHub). The "event parity" framing understates the latency gap — consider qualifying it as "functional event parity" or "event-type parity" to distinguish from latency parity.

Low

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mddetectNewLabels calls isIssueClosed for every IID in previousState — one API call per tracked issue per poll cycle. For projects with many tracked issues, this could hit GitLab rate limits or significantly extend poll duration.

  • [internal-consistency] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — The ADR Decision section states the poller "Reads a timestamp watermark" as a single concept, but then immediately describes two separate watermarks: FULLSEND_LAST_POLL_AT_FAST and FULLSEND_LAST_POLL_AT_FULL. Consider noting upfront that separate watermarks exist for fast-poll and full-poll modes.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdisProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could match human usernames on self-hosted GitLab instances. The plan acknowledges this and limits blast radius by restricting fast-poll to slash commands only.

  • [error-handling] docs/plans/gitlab-cron-polling-implementation.mdpersistLabelState logs warnings but does not return errors when json.Marshal or UpdateVariable fails. If persistence fails, the next poll re-detects all labels as new, causing duplicate dispatches mitigated by resource_group concurrency control.

  • [missing-authorization] — PR has no linked issue. This is a substantial docs-only change (new ADR + ~1678-line implementation plan). While no production code is changed, traceability to authorized work is recommended.

  • [interface-evolution-scope] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — ADR 0063 adds 6+ new methods to forge.Client, which ADR 0005 originally promised would not require interface changes for new forges. The ADR's Forge Abstraction subsection acknowledges this as "anticipated growth" — consider a brief cross-reference annotation on ADR 0005 to help future readers understand the evolution.

  • [naming-consistency] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — Multiple overlapping credential terms: "bot PAT," "project access token," "Developer-role project access token," and FULLSEND_FORGE_TOKEN. Each is clear in context, but a brief terminology mapping could help readers.

  • [documentation-coherence] docs/problems/gitlab-implementation.md — The problem doc retains ~624 lines of webhook-based implementation guidance now superseded by ADR 0063. The deprecation notice correctly redirects readers, but the obsolete content may still cause confusion.

  • [naming-convention] docs/plans/gitlab-cron-polling-implementation.md:1 — Title "GitLab Cron-Polling Implementation Details" doesn't follow the established "Implementation Plan: <topic>" pattern used by other plans.

  • [code-organization] docs/plans/gitlab-cron-polling-implementation.md:3 — Single-line preamble is terse compared to other plans that use a structured Context section describing the ADR relationship.

  • [naming-convention] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — "MR" abbreviation is used early without first spelling out "merge request" on introduction, though the full term does appear in the Context section.

  • [pattern-inconsistency] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — Redundant "(deprecated)" suffix on ADR 0028 references when the deprecation status is already in ADR 0028's frontmatter and Status section.

Previous run (15)

Review

Findings

Medium

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.md — In discoverAllEvents, when ListIssuesUpdatedSince succeeds but ListMergeRequestsUpdatedSince fails, the function returns an error and discards all already-discovered issue events plus the updated label state. If the MR API fails persistently (e.g., due to a permissions issue), no issue events will ever be processed because the function always bails out. Consider splitting issue and MR discovery so that a persistent MR API failure does not block issue event processing — e.g., log the MR error and continue with issue-only events, tracking minSkippedAt from the MR failure point.

Low

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.md — The detectNewLabels function calls p.isIssueClosed for every IID in previousState to prune closed issues. This is an API call per tracked issue per poll cycle. The practical impact is bounded by the 10,000-char CI/CD variable size limit (limiting tracked issues to a few hundred), and the document proposes filtering to fullsend-relevant labels, but the pseudocode does not implement this filtering.

  • [internal-consistency] docs/plans/gitlab-cron-polling-implementation.md — The ADR states the poller "Reads a timestamp watermark (FULLSEND_LAST_POLL_AT, protected CI/CD variable)" using a single variable name, but the implementation plan uses two separate watermarks: FULLSEND_LAST_POLL_AT_FAST and FULLSEND_LAST_POLL_AT_FULL. Note: the ADR's Security-Critical Code Paths section 8 does reference both variables, so only the cron poller summary section needs updating.

  • [edge-case] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — The ADR describes the CI_DEBUG_TRACE script-level guard as the "sole defense" against PAT exposure in variable mode, while also acknowledging that "GitLab logs CI/CD variables at job init, before any script runs." The guard provides damage limitation (aborting before further operations with the token), not prevention of the initial exposure. The ADR correctly recommends WIF mode as the mitigation, but the "sole defense" wording is slightly misleading given the timing.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdisProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could match human usernames on self-hosted GitLab instances where username restrictions may differ from SaaS. The plan acknowledges this and limits blast radius by restricting fast-poll to slash commands only.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdpersistLabelState logs warnings but does not return errors when json.Marshal or UpdateVariable fails. If persistence fails, the next poll re-detects all labels as new, causing duplicate dispatches mitigated by resource_group concurrency control.

  • [authorization-bypass] docs/plans/gitlab-cron-polling-implementation.md — Bot detection inconsistency between fast-poll (username pattern via isProjectAccessTokenBot) and full-poll (note.Author.Bot field). The Events API lacks a bot field, making the pattern match a necessary fallback. Blast radius is limited since fast-poll only handles slash commands, not changes-requested markers.

  • [privilege-escalation] docs/plans/gitlab-cron-polling-implementation.mdrouteMRNote allows bot-authored comments containing the changes-requested marker to trigger fix stage. Any project access token bot (not just the fullsend bot) would pass the Author.Bot check. Consider verifying the bot author ID matches the enrolled project's fullsend bot user ID.

  • [credential-scope] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — Bot PAT requires api scope (broadest available). The ADR acknowledges this as a GitLab platform limitation.

Previous run (22)

Review

Findings

Medium

  • [internal-consistency] docs/plans/gitlab-cron-polling-implementation.md:938 — Label name mismatch between GitLab routing and existing codebase. The poller's routeIssueLabel routes on fullsend:ready-to-code and fullsend:ready-for-review, and the ADR event routing table (line 232–233) uses the same prefixed names. However, the entire existing codebase — GitHub dispatch.yml, scaffold dispatch.yml, triage post-scripts, eval cases, e2e tests, and even the existing docs/problems/gitlab-implementation.md (which explicitly states "Same label names: ready-to-code, ready-for-review") — uses unprefixed names. If implemented as written, the GitLab poller will never match label-based triggers because the labels created by the triage agent use the unprefixed names.
    Remediation: Use the same label names as the existing codebase (ready-to-code, ready-for-review) in both the ADR

Previous history truncated due to comment size limits.

Previous run (16)

Review

Findings

Medium

  • [logic-error] docs/plans/gitlab-cron-polling-implementation.md — In discoverAllEvents, the error return for ListMergeRequestsUpdatedSince provides only 3 return values (nil, nil, fmt.Errorf(...)) but the function signature requires 4 ([]RoutableEvent, LabelState, time.Time, error). This pseudocode would not compile. Remediation: change to return nil, nil, time.Time{}, fmt.Errorf("list merge requests: %w", err).

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.md — In discoverAllEvents, when ListMergeRequestNotes fails for an MR, the loop logs and continues but does not track the skipped MR's UpdatedAt in minSkippedAt. The watermark can advance past the failed MR, permanently losing those note events. The issue-side equivalent was fixed (note-fetch failures update minSkippedAt and restore label state), but the MR-side path lacks this protection. Remediation: track minSkippedAt for MR note-fetch failures the same way as for issue note-fetch failures.

Low

  • [missing-authorization] — PR has no linked issue. This is a substantial docs-only change (new ADR + 1661-line implementation plan). While no production code is changed, traceability to authorized work is recommended.

  • [fail-open] docs/plans/gitlab-cron-polling-implementation.md — The CI_DEBUG_TRACE guard in CI/CD templates runs after GitLab has already logged CI/CD variables at job init. In variable mode, this guard cannot prevent PAT exposure. The ADR documents this as a known limitation and recommends WIF mode as mitigation.

  • [authorization-bypass] docs/plans/gitlab-cron-polling-implementation.md — Bot detection inconsistency between fast-poll (username pattern via isProjectAccessTokenBot) and full-poll (note.Author.Bot field). The Events API lacks a bot field, making the pattern match a necessary fallback, but the inconsistency should be documented.

  • [privilege-escalation] docs/plans/gitlab-cron-polling-implementation.mdrouteMRNote allows bot-authored changes-requested markers to trigger fix stage. In the full-poll path, bot detection uses the API's Author.Bot field (reliable). In fast-poll, username pattern matching is less reliable but fast-poll does not process changes-requested markers, limiting the actual risk.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdpersistLabelState silently discards the json.Marshal error via stateBytes, _ := json.Marshal(state). While unlikely to fail for map[int][]string, this sets a bad pattern.

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.mdisProjectAccessTokenBot heuristic (project_ prefix + _bot_ substring) could match human usernames on self-hosted GitLab instances where username restrictions may differ from SaaS.

  • [stale-terminology] docs/glossary.md — Three glossary entries use GitHub-specific webhook terminology without acknowledging GitLab's cron-polling model: "Debouncing" (line 42, references "webhook + dispatch service"), "Entry Point" (line 48, references "GitHub events (webhooks)"), and "Trigger" (line 162, references "GitHub webhook event"). Consider updating to use forge-neutral language.

  • [forge-abstraction-terminology] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — The Forge abstraction subsection opens with "This ADR adds forge-neutral methods" but then describes interface evolution (new extension interface, method refactoring). The self-qualifying paragraph acknowledges this, but the opening phrasing understates the scope.

  • [credential-scope] docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md — Bot PAT requires api scope (broadest available). The ADR acknowledges this as a GitLab platform limitation. Consider noting the intent to narrow scope when GitLab supports finer-grained scopes.

  • [github-specific-reference] docs/normative/normalized-event/v1/README.md — The "Future forges" illustrative section references "gitlab-event from GitLab webhook payload" as the input driver. ADR 0063 eliminates webhooks. The section is explicitly "illustrative only — not normative for v1" but is now outdated.

Previous run (17)

Review

Findings

Medium

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.md — In discoverAllEvents, detectNewLabels is called before the per-issue loop and updates the label state (previousState[issue.IID] = issue.Labels) for ALL issues returned by the API. When ListIssueNotes subsequently fails for an issue and the loop continues, that issue's label events are never emitted into the events slice. However, the returned updatedLabelState already marks those labels as "seen." Since those labels never enter events, they never appear in failedLabelEvents, and persistLabelState writes the state with those labels marked as seen. On the next poll, those labels will not be detected as "new" and the corresponding label-triggered stages will never fire. Remediation: track skipped issue IIDs after the per-issue loop and restore their previous label state entries in updatedLabelState, or move the previousState[issue.IID] = issue.Labels update out of detectNewLabels and into the per-issue loop so it only executes for fully-processed issues.

Low

  • [edge-case] docs/plans/gitlab-cron-polling-implementation.md — In discoverAllEvents, when ListIssueNotes fails for an issue and the issue is skipped via continue, the watermark is not prevented from advancing past the skipped issue's UpdatedAt by other successfully-processed issues with later timestamps. The minFailedAt capping logic only tracks dispatch-time failures, not discovery-time failures. If issue A (UpdatedAt=T1) has a note-fetch failure and issue B (UpdatedAt=T2, T2>T1) succeeds, maxUpdatedAt advances to T2. The 30-second overlap may not cover the gap if T1 < T2 - 30s, causing those events to be permanently missed. Consider applying the same minFailedAt capping logic for skipped issues.

  • [pseudocode-edge-case] docs/plans/gitlab-cron-polling-implementation.md — The commandToken function splits on space (strings.IndexByte(body, ' ')) but does not split on other whitespace characters (tab, newline). After strings.TrimSpace, a body like /fs-fix\nsome argument produces the token /fs-fix\nsome argument, which does not match any switch case. The command is treated as an unrecognized /fs-* command and silently dropped. This fails closed (no incorrect dispatch) but means valid slash commands followed by a newline instead of a space are silently ignored.

  • [architecture-doc-sync] docs/architecture.md — The new entry for ADR 0063 appears in the "Decided" section. While this is consistent with how other accepted ADRs are listed (the section records decisions, not shipped features), the implementation plan describes 5 phases starting from Phase 0, meaning no code exists yet. Consider adding a brief note like "(implementation pending)" to distinguish from entries whose implementations have already landed.

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added component/docs User-facing documentation component/dispatch Workflow dispatch and triggers labels Jun 23, 2026
Comment thread docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md
Comment thread docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md
Comment thread docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md Outdated
Comment thread docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md Outdated
Comment thread docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md Outdated
Comment thread docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md Outdated
Comment thread docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md
Comment thread docs/ADRs/0055-gitlab-cron-polling-event-dispatch.md Outdated
@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown

Site preview

Preview: https://94df5f9f-site.fullsend-ai.workers.dev

Commit: 7358e8954f2cfe276ed5e02fa2a9b06b12a4747b

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 26, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:30 AM UTC · Completed 2:43 AM UTC
Commit: ad4ebe1 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 26, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:58 AM UTC · Completed 4:10 AM UTC
Commit: 8441682 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 26, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 11:18 AM UTC · Ended 11:21 AM UTC
Commit: 2d8adb7 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 26, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:24 AM UTC · Completed 11:37 AM UTC
Commit: 331f938 · View workflow run →

@fullsend-ai-review fullsend-ai-review Bot added the requires-manual-review Review requires human judgment label Jun 26, 2026
@ggallen ggallen force-pushed the worktree-gitlab-cron branch from 331f938 to 7e09a94 Compare June 26, 2026 11:44
@fullsend-ai-review

fullsend-ai-review Bot commented Jun 26, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 11:47 AM UTC · Completed 12:00 PM UTC
Commit: 7e09a94 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 2, 2026

Copy link
Copy Markdown

🤖 Review · ⚠️ Cancelled · Started 9:05 PM UTC · Ended 9:14 PM UTC
Commit: 0a95cac · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 2, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:14 PM UTC · Completed 9:30 PM UTC
Commit: 450efc3 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:11 AM UTC · Completed 12:28 AM UTC
Commit: f1c1237 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:19 AM UTC · Completed 1:35 AM UTC
Commit: 7b8d1d7 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure · Started 2:28 AM UTC · Completed 2:45 AM UTC
Commit: a49e782 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure · Started 2:51 AM UTC · Completed 3:17 AM UTC
Commit: 5820596 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:04 PM UTC · Completed 12:18 PM UTC
Commit: 9b07a0c · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:27 PM UTC · Completed 12:39 PM UTC
Commit: 7dbc6c2 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 12:44 PM UTC · Completed 12:57 PM UTC
Commit: fe8ccad · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:03 PM UTC · Completed 1:17 PM UTC
Commit: c3d4ce9 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:21 PM UTC · Completed 1:36 PM UTC
Commit: 92e685a · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 1:43 PM UTC · Completed 1:59 PM UTC
Commit: de134cd · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:06 PM UTC · Completed 2:21 PM UTC
Commit: fb3bfe3 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

Comment thread docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md Outdated
Comment thread docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md
Comment thread docs/ADRs/0063-gitlab-cron-polling-event-dispatch.md
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 2:46 PM UTC · Completed 3:02 PM UTC
Commit: 124f9f2 · View workflow run →

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 3:06 PM UTC · Completed 3:23 PM UTC
Commit: 08f3566 · View workflow run →

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure · Started 3:27 PM UTC · Completed 3:42 PM UTC
Commit: 030bdb1 · View workflow run →

@ggallen

ggallen commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure · Started 8:58 PM UTC · Completed 9:11 PM UTC
Commit: 0a95cac · View workflow run →

@ggallen

ggallen commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

/fs-review

@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ❌ Failure · Started 9:32 PM UTC · Completed 9:46 PM UTC
Commit: 0a95cac · View workflow run →

Signed-off-by: Claude <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
@fullsend-ai-review

fullsend-ai-review Bot commented Jul 3, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 9:56 PM UTC · Completed 10:10 PM UTC
Commit: 7358e89 · View workflow run →

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

Labels

component/dispatch Workflow dispatch and triggers component/docs User-facing documentation component/install CLI install and app setup ready-for-merge All reviewers approved — ready to merge security Security threat model and related concerns type/feature New capability request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants