Skip to content

ci: add /strands-ts command handler#266

Open
yonib05 wants to merge 1 commit into
strands-agents:mainfrom
yonib05:ci/strands-ts-command
Open

ci: add /strands-ts command handler#266
yonib05 wants to merge 1 commit into
strands-agents:mainfrom
yonib05:ci/strands-ts-command

Conversation

@yonib05

@yonib05 yonib05 commented Jun 15, 2026

Copy link
Copy Markdown
Member

Adds a /strands-ts <command> handler that runs the new multi-agent TypeScript PR reviewer, alongside the existing /strands command. Purely additive and opt-in.

⚠️ Merge order — BLOCKED until devtools#68 merges

This workflow references composite actions (strands-ts-runner, strands-ts-finalize) that only exist on strands-agents/devtools@main after strands-agents/devtools#68 merges.

Do not merge this PR until devtools#68 is merged, or the runner/finalize steps will 404.

Related: strands-agents/devtools#68 (reviewer + actions), strands-agents/devtools#63 (supporting diff-truncation fix).

What's in this PR

  1. New strands-ts-command.yml — triggers on /strands-ts, mirrors strands-command.yml (same auth gate / roles triage,write,admin, OIDC, AWS_ROLE_ARN + AGENT_SESSIONS_BUCKET secrets), and adds the strands-running label lifecycle. Read-only agent run + deferred-write finalize.
  2. One-line guard added to strands-command.yml — the existing /strands handler triggers on the startsWith('/strands') prefix, which also matches /strands-ts; without this guard both workflows fire on one /strands-ts comment. Auth + finalize conditions now exclude /strands-ts.

After merge

Members can manually trigger /strands-ts review on any PR. We tune it on real PRs (lenses/SOPs/model selection are configurable), and once we're happy we can replace the Python reviewer for automatic execution.

@github-actions github-actions Bot added chore Maintenance tasks, dependency updates, CI changes, refactoring with no user-facing impact area-community Repo health, governance, contributor process, release process, and CI dependency bumps strands-running labels Jun 15, 2026

# No workflow-level write perms: the read/write split is enforced per job so
# the agent job never holds a write-capable token.
permissions: {}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue: There's no concurrency group. Because this workflow introduces a strands-running label lifecycle (mark-runningclear-running), two /strands-ts comments on the same PR will race: clear-running from the first run can remove the label while the second run is still executing, and you'll also get duplicate concurrent reviews.

Suggestion: Add a per-PR concurrency group so a new invocation supersedes an in-flight one, e.g.:

concurrency:
  group: strands-ts-${{ github.event.issue.number }}
  cancel-in-progress: true

aws_role_arn: ${{ secrets.AWS_ROLE_ARN }}
agents_config: ${{ vars.STRANDS_TS_AGENTS || '' }}

finalize:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: finalize (and clear-running) have no timeout-minutes. The mirrored strands-command.yml sets timeout-minutes: 30 on its finalize job. Adding a timeout here guards against a hung write-replay step holding the runner indefinitely.

pr_number: ${{ github.event.issue.number }}
pr_head_sha: ${{ steps.pr.outputs.sha }}
aws_role_arn: ${{ secrets.AWS_ROLE_ARN }}
agents_config: ${{ vars.STRANDS_TS_AGENTS || '' }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue: The PR description states this workflow uses the AGENT_SESSIONS_BUCKET secret (matching strands-command.yml, which passes sessions_bucket: ${{ secrets.AGENT_SESSIONS_BUCKET }}), but it's never referenced here.

Suggestion: Confirm whether the strands-ts-runner action needs a sessions bucket. If it does, the input is missing and session persistence will silently break; if it intentionally doesn't, please update the PR description so the secret list is accurate. Either way, worth reconciling before merge.

username: ${{ github.event.comment.user.login || 'invalid' }}
allowed-roles: 'triage,write,admin'

mark-running:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Issue: mark-running depends only on authorization-check and has no environment gate, while execute-readonly-agent does (environment: ${{ needs.authorization-check.outputs.approval-env }}). For users routed through a manual-approval environment, the strands-running label is applied before the approval is granted, so a PR can show "running" while actually pending/never-approved.

Suggestion: Consider gating mark-running on the same approval-env so the label only appears once the run is actually approved to proceed.

@github-actions

Copy link
Copy Markdown

Assessment: Comment

Clean, well-scoped additive change. The security posture is the strongest part: workflow-level permissions: {}, strict per-job least privilege, a read-only agent job, and a single write-capable finalize job that only replays vetted artifact ops. The one-line !startsWith('/strands-ts') guard on the existing /strands handler correctly prevents double-firing on the shared prefix. No tests apply (workflow YAML) and there's no public API surface, so no bar-raising review needed.

Review themes
  • Config consistency: The AGENT_SESSIONS_BUCKET secret named in the PR description isn't wired into this workflow — reconcile the description or add the missing input.
  • Concurrency: No concurrency group around the new strands-running label lifecycle, so overlapping /strands-ts comments can race on the label and run duplicate reviews.
  • Approval gating: mark-running isn't gated on the approval environment, so the label can appear before a gated run is approved.
  • Robustness: finalize/clear-running lack timeout-minutes present on the mirrored Python workflow.

None are blocking on their own; the merge-order block on devtools#68 (already documented) remains the real gate. Nice job keeping the read/write split tight.

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

Labels

area-community Repo health, governance, contributor process, release process, and CI dependency bumps chore Maintenance tasks, dependency updates, CI changes, refactoring with no user-facing impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant