Skip to content

fix(#6418): pass ORIGINATING_URL and REPO_FULL_NAME in GitLab dispatch - #6419

Merged
ggallen merged 1 commit into
mainfrom
agent/6418-gitlab-retro-dispatch-vars
Aug 20, 2026
Merged

fix(#6418): pass ORIGINATING_URL and REPO_FULL_NAME in GitLab dispatch#6419
ggallen merged 1 commit into
mainfrom
agent/6418-gitlab-retro-dispatch-vars

Conversation

@fullsend-ai-coder

@fullsend-ai-coder fullsend-ai-coder Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Pass ORIGINATING_URL, REPO_FULL_NAME, and RETRO_COMMENT to the GitLab retro agent, fixing the harness env validation failure on GitLab.

Related Issue

Changes

  • internal/poll/dispatch.go: Set ORIGINATING_URL via the existing entityURL() helper and REPO_FULL_NAME to p.projectPath. Add both to signedDispatchKeys for HMAC protection.
  • internal/scaffold/fullsend-repo-gitlab/.gitlab/ci/fullsend-agent.yml: Add ORIGINATING_URL and REPO_FULL_NAME to the HMAC verification printf format, maintaining sorted key order to match the Go signedDispatchKeys slice. Extract RETRO_COMMENT from EVENT_PAYLOAD_B64 for retro stages (parity with GitHub's reusable-retro.yml which sets it from event_payload.comment.body).
  • internal/poll/dispatch_test.go: Add tests for issue events, MR events, and subgroup project paths. Add ORIGINATING_URL and REPO_FULL_NAME with realistic values to TestComputeDispatchHMAC_MatchesPython3.
  • internal/poll/state_test.go: Set gitlabURL and projectPath on the test poller so dispatch tests produce realistic URLs.
  • docs/ADRs/0067-gitlab-cron-polling-event-dispatch.md: Add ORIGINATING_URL and REPO_FULL_NAME to the trust-boundary variable enumeration.

GITLAB_TOKEN is already handled by the agent template (export GITLAB_TOKEN="${FULLSEND_FORGE_TOKEN}") and does not need to be in the dispatch payload.

Testing

  • go test ./internal/poll/... — all tests pass
  • go vet ./internal/poll/... — clean
  • Patch coverage ≥ 80%
  • TestSignedDispatchKeys_MatchShellTemplate validates Go/shell HMAC key sync
  • TestComputeDispatchHMAC_MatchesPython3 validates cross-language HMAC consistency with all 11 keys

Checklist

  • PR title follows Conventional Commits (correct type, ! for breaking changes)
  • Commits are signed off (DCO)
  • I wrote this contribution myself and can explain all changes in it

Closes #6418

@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner August 20, 2026 16:37
@fullsend-ai-coder fullsend-ai-coder Bot added the ready-for-review Agent PR ready for human review label Aug 20, 2026
@ggallen ggallen added the fullsend-no-fix Skip bot-triggered fix agent runs label Aug 20, 2026
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 20, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 4:39 PM UTC · Completed 5:00 PM UTC

Commit: 4d9aa6d · View workflow run →

@codecov

codecov Bot commented Aug 20, 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 Aug 20, 2026

Copy link
Copy Markdown

Review

Findings

Low

  • [scope] internal/scaffold/fullsend-repo-gitlab/.gitlab/ci/fullsend-agent.yml — The RETRO_COMMENT extraction block is not mentioned in issue GitLab retro dispatch does not pass required env vars to pipeline #6418. While directly related to making the retro agent work on GitLab, this is technically beyond the issue's authorized scope. Consider expanding GitLab retro dispatch does not pass required env vars to pipeline #6418 or filing a follow-up.
  • [test helper completeness] internal/poll/state_test.go:15newTestPoller hardcodes projectPath, owner, and repo independently rather than deriving owner/repo from projectPath via splitOwnerRepo(). Consider using the production derivation to prevent future drift between these fields.
Previous run

Looks good to me

Previous run (2)

Review

Findings

Critical

  • [backward-incompatible HMAC contract change] internal/poll/dispatch.go:129 — Adding ORIGINATING_URL and REPO_FULL_NAME to signedDispatchKeys is a breaking change for the HMAC verification contract between the Go poller and deployed GitLab CI configs. The poller will sign dispatches with 11 keys, but already-deployed repos still verify with the old 9-key printf in their copy of fullsend-agent.yml. Since the poller populates these with non-empty values (originatingURL is always a constructed URL, projectPath is always set), the HMAC message will differ from the 9-key message computed by the deployed verifier, causing HMAC verification failure and pipeline abort (fail-closed) for every HMAC-enabled GitLab repo until it re-scaffolds. This affects all agent stages dispatched to those repos, not just retro.
    Remediation: Deploy as two-phase rollout — first update and re-scaffold all deployed repos with the new template, then update the poller. Alternatively, add a version or key-count field to the dispatch variables for mismatch detection. At minimum, document the required rollout order.

Medium

  • [HMAC contract versioning] internal/poll/dispatch.go:109 — The signedDispatchKeys list and its shell counterpart form a cross-repo contract with no version negotiation. Every addition requires coordinated deployment across all enrolled repos. The existing test (TestSignedDispatchKeys_MatchShellTemplate) validates the source template matches but cannot verify deployed copies. See also: [backward-incompatible HMAC contract change] finding above.
    Remediation: Consider adding a version or key-count field to the dispatch variables so the verifier can detect a mismatch and emit a clear diagnostic rather than a generic HMAC failure.

Low

  • [code duplication / missed reuse] internal/poll/dispatch.go:60 — The ORIGINATING_URL construction in dispatch() duplicates logic from the existing entityURL() helper in convert.go. Both determine the entity path segment (issues vs merge_requests) based on event type and construct the same URL pattern, but use slightly different dispatch logic (HasPrefix vs switch), which could diverge on future event types.
    Remediation: Consider reusing entityURL() instead of reimplementing the URL construction inline.

  • [test adequacy] internal/poll/dispatch_test.go:817TestComputeDispatchHMAC_MatchesPython3 does not include ORIGINATING_URL or REPO_FULL_NAME in its vars map. The test still passes (both languages compute over the same canonical message with empty values for missing keys), but it does not exercise cross-language HMAC agreement for the two new keys with realistic non-empty values.
    Remediation: Add ORIGINATING_URL and REPO_FULL_NAME with realistic values to the test vars map.

  • [stale-variable-enumeration] docs/ADRs/0067-gitlab-cron-polling-event-dispatch.md:48 — The trust-boundary analysis enumerates 9 dispatch variables but does not include ORIGINATING_URL and REPO_FULL_NAME. Since this list appears in a security trust-boundary discussion (what an attacker can POST), the omission could mislead future readers about the full attack surface.
    Remediation: Add a short annotation noting the two new signed dispatch variables.


Labels: PR modifies GitLab dispatch code in internal/poll/ and scaffold template, fixing a bug affecting the retro agent on GitLab


Next steps:

  • /fs-fix — agent addresses review findings automatically
  • /fs-fix <your instruction> — agent fixes with your specific guidance
  • Push commits directly — review re-runs automatically on push
  • /fs-fix-stop — disable automatic fix runs for this PR

fullsend-ai-review[bot]

This comment was marked as outdated.

@fullsend-ai-review fullsend-ai-review Bot added the component/dispatch Workflow dispatch and triggers label Aug 20, 2026
@ggallen

ggallen commented Aug 20, 2026

Copy link
Copy Markdown
Member

Addressing review findings

Critical: backward-incompatible HMAC contract change

This is a deployment ordering concern, not a code bug. Both sides of the contract (Go signedDispatchKeys and shell printf in fullsend-agent.yml) are updated in the same PR. The TestSignedDispatchKeys_MatchShellTemplate test validates they stay in sync at the source level.

The required rollout order is: re-scaffold enrolled repos first (to deploy the updated template), then deploy the updated poller. This is inherent to any HMAC key addition and doesn't need special code — it's the same sequence used when the original HMAC keys were added. Repos without FULLSEND_DISPATCH_SECRET configured are unaffected (HMAC verification is skipped).

Medium: HMAC contract versioning

Acknowledged as a useful future enhancement. The current approach (source-level test + coordinated deploy) works for the small number of enrolled GitLab repos. A version/key-count field would add complexity without solving the deployment ordering requirement. Filed as a follow-up consideration.

Low: code duplication / missed reuse — FIXED

Replaced the inline URL construction in dispatch() with a call to the existing entityURL() helper from convert.go. This ensures consistent entity path resolution across both code paths.

Low: test adequacy — FIXED

Added ORIGINATING_URL and REPO_FULL_NAME with realistic values to the TestComputeDispatchHMAC_MatchesPython3 test vars map so cross-language HMAC agreement is exercised with non-empty values for the new keys.

Low: stale variable enumeration in ADR 0067 — FIXED

Added ORIGINATING_URL and REPO_FULL_NAME to the trust-boundary variable list in the ADR.

Additional fix: missing RETRO_COMMENT for GitLab

The retro agent needs RETRO_COMMENT (user instructions from the /fs-retro comment). On GitHub, both reusable-retro.yml and reusable-dispatch.yml extract this from the event payload. On GitLab, the comment body is already in EVENT_PAYLOAD_B64 as note_body, but nothing set it as RETRO_COMMENT. Added extraction in fullsend-agent.yml for retro stages.

@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown

Site preview

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

Commit: 25a7009d00c9a02ca7a29cc8a9889dc7888b3445

@fullsend-ai-review

fullsend-ai-review Bot commented Aug 20, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:18 PM UTC · Completed 5:31 PM UTC

Commit: e17ac7f · View workflow run →

@fullsend-ai-review
fullsend-ai-review Bot dismissed their stale review August 20, 2026 17:31

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 Aug 20, 2026
Pass ORIGINATING_URL and REPO_FULL_NAME as pipeline variables in the
GitLab poller dispatch, fixing the retro agent harness env validation
failure on GitLab. Uses the existing entityURL() helper for URL
construction.

Also:
- Extract RETRO_COMMENT from EVENT_PAYLOAD_B64 in the GitLab agent
  template for retro stages (parity with GitHub workflow)
- Add both new keys to signedDispatchKeys and HMAC shell verification
- Update TestComputeDispatchHMAC_MatchesPython3 with realistic values
  for the new keys
- Update ADR 0067 trust-boundary variable enumeration

Closes #6418

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Greg Allen <gallen@redhat.com>
@ggallen
ggallen force-pushed the agent/6418-gitlab-retro-dispatch-vars branch from e17ac7f to 25a7009 Compare August 20, 2026 17:33
@ggallen
ggallen enabled auto-merge August 20, 2026 17:34
@fullsend-ai-review

fullsend-ai-review Bot commented Aug 20, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:35 PM UTC · Completed 5:50 PM UTC

Commit: 25a7009 · View workflow run →

@@ -13,10 +13,12 @@ func newTestPoller(client GitLabClient, opts Options) *Poller {
opts.PipelineRef = "main"
}
return &Poller{

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] test helper completeness

newTestPoller hardcodes projectPath, owner, and repo independently rather than deriving owner/repo from projectPath via splitOwnerRepo(). The production New() constructor derives owner and repo from projectPath. Using splitOwnerRepo would prevent future drift between these fields.

Suggested fix: Consider having newTestPoller call splitOwnerRepo(projectPath) to derive owner and repo, rather than hardcoding all three independently.

Merged via the queue into main with commit b294e1e Aug 20, 2026
42 checks passed
@ggallen
ggallen deleted the agent/6418-gitlab-retro-dispatch-vars branch August 20, 2026 17:53
@fullsend-ai-retro

fullsend-ai-retro Bot commented Aug 20, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 5:54 PM UTC · Completed 6:07 PM UTC

Commit: 25a7009 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #6419 — pass ORIGINATING_URL and REPO_FULL_NAME in GitLab dispatch

Workflow: Issue #6418 → triage → code → 3 review rounds → human fixes → merge. Total time: ~93 minutes.

What went well

  • Triage quality was strong. Correctly identified root cause in dispatch(), recommended using the existing entityURL() helper, and correctly noted GITLAB_TOKEN should be a pre-configured CI/CD variable rather than forwarded in the payload. Proposed a test case.
  • Review agent caught real issues. The critical HMAC backward-compatibility finding was a legitimate deployment concern — changing signedDispatchKeys from 9 to 11 keys would break already-deployed GitLab repos until they re-scaffold. The human resolved this through deployment coordination, not a code change, but the review correctly identified the risk. Two low findings (code duplication, test gaps) were directly fixed.
  • Review lifecycle worked well. The review agent properly dismissed its superseded initial review and approved the improved code.
  • Code agent got the core structure right — touched the right 4 files, added correct tests, and the primary fix was functionally sound.

What could improve

  • Code agent missed 4 refinements that the human fixed via force-push: (1) inline URL construction instead of reusing entityURL() helper, (2) missing HMAC cross-language test update, (3) missing ADR 0067 trust-boundary update, (4) missing RETRO_COMMENT extraction for GitLab retro parity. The fullsend-no-fix label was applied immediately at 16:38 UTC (1 minute after PR creation), suggesting the human anticipated needing to intervene.
  • All inline review comments returned HTTP 422 on the first review round, falling back to the review body text.
  • A DCO sign-off-only force-push triggered a third review cycle that found only 2 low informational findings and approved — token cost with no workflow impact.

Evidence for existing issues

Novel finding → Proposal below

Neither triage nor code agent performed a systematic comparison between the GitHub retro workflow (reusable-retro.yml) and the GitLab equivalent (fullsend-agent.yml). This caused both agents to miss RETRO_COMMENT, which GitHub's workflow sets from event_payload.comment.body but had no GitLab equivalent. The human discovered this gap and added the extraction logic. See proposal for details.

Proposals filed

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 fullsend-no-fix Skip bot-triggered fix agent runs ready-for-merge All reviewers approved — ready to merge ready-for-review Agent PR ready for human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GitLab retro dispatch does not pass required env vars to pipeline

1 participant