Skip to content

LCORE-5254: (fix) Run retro agent manually only on a /fs-retro comment - #3112

Merged
tisnik merged 1 commit into
lightspeed-core:mainfrom
anik120:fix-retro-trigger
Nov 3, 2026
Merged

tisnik merged 1 commit into
lightspeed-core:mainfrom
anik120:fix-retro-trigger

Conversation

@anik120

@anik120 anik120 commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Description

Intended behavior: when a pull request is closed or merged, the retro agent
should do nothing; it should run only when a human posts a comment whose
first line is the /fs-retro slash command.

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library [pyproject.toml + uv.lock]
  • Bump-up dependent library [requirements.*.txt for Konflux]
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement
  • Benchmarks improvement

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

  • Assisted-by: (e.g., Claude, CodeRabbit, Ollama, etc., N/A if not used)
  • Generated by: (e.g., tool name and version; N/A if not used)

Related Tickets & Documents

  • Related Issue #
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

  • Changes
    • The retro agent now runs only when a human adds a comment containing /fs-retro. Other comments no longer trigger it.

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 0b7d213 and 2865790.

Files selected for processing: 1
  • .fullsend/config.yaml

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Recent review details
Context from checks skipped due to timeout: 35
  • GitHub Check: E2E: library / ci / skills
  • GitHub Check: E2E: library / ci / shields
  • GitHub Check: E2E: server / ci / skills
  • GitHub Check: E2E: server / ci / authorized
  • GitHub Check: E2E: server / ci / tls
  • GitHub Check: E2E: library / ci / rbac
  • GitHub Check: E2E: server / ci / shields
  • GitHub Check: E2E: server / ci / default
  • GitHub Check: E2E: server / ci / other
  • GitHub Check: E2E: library / ci / other
  • GitHub Check: E2E: library / ci / mcp
  • GitHub Check: E2E: library / ci / default
  • GitHub Check: E2E: server / ci / rbac
  • GitHub Check: E2E: library / ci / authorized
  • GitHub Check: E2E: server / ci / mcp
  • GitHub Check: mypy
  • GitHub Check: spectral
  • GitHub Check: unit_tests (3.14)
  • GitHub Check: unit_tests (3.13)
  • GitHub Check: integration_tests (3.13)
  • GitHub Check: Pyright
  • GitHub Check: integration_tests (3.12)
  • GitHub Check: unit_tests (3.12)
  • GitHub Check: integration_tests (3.14)
  • GitHub Check: bandit
  • GitHub Check: Pylinter
  • GitHub Check: build-pr
  • GitHub Check: black
  • GitHub Check: check_dependencies
  • GitHub Check: Red Hat Konflux / rag-content-0-8-e2e-tests / lightspeed-stack-0-8
  • GitHub Check: Red Hat Konflux / lightspeed-core-0-8-enterprise-contract / lightspeed-stack-0-8
  • GitHub Check: Red Hat Konflux / lightspeed-stack-0-8-e2e-tests / lightspeed-stack-0-8
  • GitHub Check: dispatch / Harness dispatch
  • GitHub Check: check
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-8-on-pull-request
Additional comments: 1
.fullsend/config.yaml: 1

20-24: LGTM!

Walkthrough

The retro agent now runs only after a human adds a comment with the /fs-retro command. Its source remains harness/retro.yaml.

Changes

Retro trigger

Layer / File(s) Summary
Configure retro trigger
.fullsend/config.yaml
The trigger requires a comment_added transition, the /fs-retro command, and a human actor.

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Possibly related PRs

Suggested reviewers: tisnik, gwennie

Merge Risk: Minimal · up to 28657

The configured trigger matches the intended manual retro workflow, with no actionable merge-blocking risk established.

Pre-merge checks: Passed 6
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names a Jira-style key (LCORE-5254), a fix type, and the main change: the retro agent runs manually only on a /fs-retro comment. The raw summary confirms the retro agent in .fullsend/config.…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Performance And Algorithmic Complexity Passed PASS. The pull request changes only .fullsend/config.yaml and adds a bounded CEL trigger predicate for the retro agent. It adds no loops, API or database calls, list operations, parsers, caches, wat…
Security And Secret Handling Passed PASSED. The pull request changes only .fullsend/config.yaml. It adds a static CEL trigger that requires a /fs-retro command and a human actor. The diff adds no secrets, tokens, API endpoints, comm…
Full details: Title check

Explanation

The title names a Jira-style key (LCORE-5254), a fix type, and the main change: the retro agent runs manually only on a /fs-retro comment. The raw summary confirms the retro agent in .fullsend/config.yaml now runs only when a human adds a comment whose command is /fs-retro. The title matches the primary change and is specific, so it passes.

Full details: Performance And Algorithmic Complexity

Explanation

PASS. The pull request changes only .fullsend/config.yaml and adds a bounded CEL trigger predicate for the retro agent. It adds no loops, API or database calls, list operations, parsers, caches, watchers, or other unbounded work. No meaningful performance regression is introduced.

Full details: Security And Secret Handling

Explanation

PASSED. The pull request changes only .fullsend/config.yaml. It adds a static CEL trigger that requires a /fs-retro command and a human actor. The diff adds no secrets, tokens, API endpoints, command execution, path handling, response/logging changes, or Kubernetes Secret resources. The referenced harness uses the same trigger condition.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
Simplify code
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Intended behavior: when a pull request is closed or merged, the retro agent
should do nothing; it should run only when a human posts a comment whose
first line is the /fs-retro slash command.

Signed-off-by: Anik Bhattacharjee <anbhatta@redhat.com>

@tisnik tisnik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@tisnik
tisnik merged commit 1bdf2e0 into lightspeed-core:main Nov 3, 2026
63 checks passed
@fullsend-ai-retro

fullsend-ai-retro Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 9:27 AM UTC · Completed 9:39 AM UTC

Commit: 443aaa0 · View workflow run →

Runtime: pi · Model: opus → claude-opus-4-6 · Effort: high · Cost: $3.25

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #3112 — Fix retro agent trigger

Timeline

This PR is the third in a chain of 3 PRs over 5 days, all by anik120 under LCORE-5254, to correctly restrict the retro agent to manual /fs-retro invocation:

  1. PR LCORE-5254: Restrict Fullsend retro to explicit /fs-retro comments #3074 (Oct 29, merged same day): First attempt — placed a trigger as a YAML map under the roles: list (- retro: {trigger: ...}). This broke config parsing since roles expect plain strings. Human reviewer approved with "LGTM"; the structural error was not caught at review time.
  2. PR LCORE-5254: Fix Fullsend config parse break from retro trigger #3087 (Oct 30, merged Nov 1): Hotfix — moved retro out of roles into a separate agents: section with source: harness/retro.yaml, plus a new harness overlay file containing the trigger. CodeRabbit correctly identified (Major severity, functional correctness) that built-in routing does not use the harness trigger: — it only checks agents[].trigger on the config entry — and suggested adding the trigger to the agent entry directly. Human reviewer approved with "LGTM" without addressing CodeRabbit's finding.
  3. PR LCORE-5254: (fix) Run retro agent manually only on a /fs-retro comment #3112 (Nov 2, merged Nov 3): Final fix implementing CodeRabbit's suggestion from PR LCORE-5254: Fix Fullsend config parse break from retro trigger #3087 — inlined the trigger on the agent entry in config.yaml, removed the harness overlay file, and added runtime: pi.

On the merge of PR #3087 (run 41285662022), the retro dispatch failed with Failed to load fullsend config / line 13: cannot unmarshal !!map into string — likely because the Route step read the pre-merge config still containing PR #3074's broken YAML.

Observations

Review agent did not dispatch on any of the 3 PRs. On PR #3112's open event (run 41461132666), the Route step logged "No stage matched — skipping dispatch." The root cause is unclear — anik120 has merge permissions, so the permission check should pass. This meant the only automated review came from CodeRabbit.

Retro still dispatched on this merge. Despite PR #3112 adding agents[].trigger to gate retro, the retro agent dispatched on the PR's merge event (run 41608796745) — no /fs-retro comment exists on the PR. The most likely explanation is that the Route step read config from the pre-merge base SHA, which still had the old config without the inline trigger. This provides direct evidence for fullsend-ai/fullsend#8708 (Route step reads config from stale base SHA).

Evidence for existing issues

Review quality

CodeRabbit's review on PR #3087 was the standout: it correctly identified the functional issue (harness trigger not gating built-in routing) and provided the exact fix that became PR #3112. The human reviewer approved all 3 PRs with "LGTM" without engaging with CodeRabbit's Major finding. Had that finding been addressed during PR #3087 review, PR #3112 would not have been necessary.

No proposals are filed because the root causes are well-tracked by existing platform issues.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants