Skip to content

ci: add /strands-ts command handler#2793

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

ci: add /strands-ts command handler#2793
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. Nothing is replaced — this is 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 the structure of the existing strands-command.yml (same auth gate, OIDC, secrets), and adds the strands-running label lifecycle. Read-only agent run + deferred-write finalize, per the safeguard model.
  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 would fire on a single /strands-ts comment. The auth and 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 size/m typescript Pull requests that update typescript code chore Maintenance tasks, dependency updates, CI changes, refactoring with no user-facing impact area-community Related to community and contributor health strands-running labels Jun 15, 2026

on:
issue_comment:
types: [created]

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.

Issue: No concurrency group is defined, so multiple /strands-ts comments on the same PR (or a re-trigger before the prior run finishes) spawn overlapping reviewer runs that race on the strands-running label and post duplicate output.

Suggestion: Add a per-PR concurrency group, e.g.:

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

Use cancel-in-progress: true if you'd rather the latest comment supersede an in-flight run.

runs-on: ubuntu-latest
permissions:
issues: write
pull-requests: write

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.

Issue: mark-running holds issues: write + pull-requests: write but only declares needs: [authorization-check] with no environment: gate. In the approval-env pattern, authorization-check succeeds for everyone and routes unauthorized users into a protected environment requiring manual approval — that gate is what actually blocks them. execute-readonly-agent (L50) honors it via environment: ${{ needs.authorization-check.outputs.approval-env }}, but mark-running runs as soon as authorization-check completes, so an unauthorized commenter can still drive a write (add the strands-running label) before any approval.

In the Python workflow the label write lives in setup-and-process, which is behind environment: approval-env, so this gap is new here.

Suggestion: Add the same environment gate to mark-running:

  mark-running:
    needs: [authorization-check]
    environment: ${{ needs.authorization-check.outputs.approval-env }}

Impact is limited to a label toggle, but it's worth closing so the read/write split holds consistently.

@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Comment

Clean, well-structured additive workflow that faithfully mirrors the Python /strands handler, with a correct prefix guard and a sensible read/write job split. The blocking dependency on devtools#68 is clearly documented. Two non-blocking items below; the merge-order block is the real gate.

Review Categories
  • Security: The read/write split is enforced everywhere except mark-running, which holds write perms without the environment: approval gate the rest of the workflow relies on (inline comment). Low impact (label toggle), but worth aligning with the Python workflow.
  • Robustness: No concurrency group, so repeated /strands-ts comments can spawn overlapping runs (inline comment).
  • Correctness: Job graph, conditionals, and the /strands vs /strands-ts guard all look right; workflow_dispatch path in the Python workflow is preserved. Minor: startsWith('/strands-ts') also matches strings like /strands-tsx, but that's unlikely to matter in practice.

Nicely done overall — the per-job permission scoping and the OIDC-only token on the agent job are exactly right.

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

Labels

area-community Related to community and contributor health chore Maintenance tasks, dependency updates, CI changes, refactoring with no user-facing impact size/m typescript Pull requests that update typescript code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant