Skip to content

scripts: tighten clean.sh#27

Closed
CamSoper wants to merge 2 commits intomasterfrom
test-pipeline/infra-edit
Closed

scripts: tighten clean.sh#27
CamSoper wants to merge 2 commits intomasterfrom
test-pipeline/infra-edit

Conversation

@CamSoper
Copy link
Copy Markdown
Owner

Adds 'set -euo pipefail' so yarn / hugo failures stop propagating, and cleans tmp/ and yarn-error.log on top of the existing targets.

Pipeline test: exercises the review:infra domain path and the new triage → workflow_run chained review. Opens as draft so triage fires but the chained review skips (is_draft=true); flipping to ready should fire both.

@claude claude Bot added the review:infra PR touches workflows, scripts, infra, Makefile, or build config label Apr 23, 2026
CamSoper added a commit that referenced this pull request Apr 23, 2026
gh pr view's --json field for author is 'author' (returns .author.login),
not 'user'. And headRefOid wasn't used anywhere. Both caused the field
validator to reject the whole --json argument, which exited non-zero
and dumped the full field list to the log.

Caught during the first end-to-end test of the chained review on PR #27.
@CamSoper CamSoper marked this pull request as ready for review April 23, 2026 21:01
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 23, 2026

🐿️ Review updated.

@github-actions github-actions Bot added the review:claude-working Claude is running a review right now; auto-removed when finishes label Apr 23, 2026
@claude
Copy link
Copy Markdown

claude Bot commented Apr 23, 2026

Claude Review — Last updated 2026-04-23T21:05:13Z

🚨 Outstanding ⚠️ Low-confidence 💡 Pre-existing ✅ Resolved
0 0 0 0

🚨 Outstanding in this PR

None.

⚠️ Low-confidence

None. Infra changes here are narrowly scoped, well-documented, and carry no secret-handling or trigger-surface risk:

  • .github/workflows/claude.yml — the fork-only swap from steps.esc-secrets.outputs.PULUMI_BOT_TOKEN to secrets.GITHUB_TOKEN is announced at the top of the file and in the PR body, with the downstream-workflow trade-off called out explicitly ("commits pushed via GITHUB_TOKEN do not trigger downstream workflows"). The removed Fetch secrets from ESC step and the env: block of ESC_ACTION_* configuration are no longer referenced anywhere, so the cleanup is consistent. on:, concurrency:, and paths: are untouched — no trigger-surface change. No secrets land in the diff.
  • scripts/clean.shset -euo pipefail plus the two new rm lines are exactly what the PR body describes. The stricter exit behavior means rm cleanup after a failing yarn cache clean / hugo mod clean won't run; that is the stated intent ("yarn / hugo failures stop propagating"), not an oversight.

💡 Pre-existing issues in touched files

Not emitted for infra PRs (per review-infra.md §Scope).

✅ Resolved since last review

Initial review.

📜 Review history

  • 2026-04-23T21:05:13Z — Initial review: infra-only PR, no findings (a89de5e)

Pushed a fix? Mention @claude to refresh. Think a finding is wrong? Mention @claude with your reasoning — disputes are welcome, and Claude will concede on evidence. See AGENTS.md §PR Lifecycle for the re-entrant workflow.

@claude claude Bot added the review:claude-ran Claude review has completed for this PR's current state label Apr 23, 2026
@github-actions github-actions Bot removed the review:claude-working Claude is running a review right now; auto-removed when finishes label Apr 23, 2026
@CamSoper CamSoper force-pushed the master branch 3 times, most recently from 84d8cdc to 00a0780 Compare April 23, 2026 22:03
Fork-only tweak so claude.yml works without org-side ESC setup.
@claude retains all its capabilities (re-entrant reviews, Q&A,
make-changes on PRs) -- only difference is commits pushed with
GITHUB_TOKEN don't trigger downstream workflows, which is fine for
fork testing.

This commit is NOT for upstream. Origin/master and pulumi#18680
keep the ESC design. Do not cherry-pick.
- Add 'set -euo pipefail' so transient yarn/hugo failures surface
  instead of getting swallowed.
- Also clean tmp/ and yarn-error.log.
@CamSoper CamSoper force-pushed the test-pipeline/infra-edit branch from a89de5e to 0ec277d Compare April 23, 2026 22:32
@github-actions github-actions Bot added the review:claude-stale New commits since last Claude review; refresh on next ready-transition or @claude mention label Apr 23, 2026
@CamSoper
Copy link
Copy Markdown
Owner Author

Test PR closed. The Claude PR review pipeline work is now tracked on pulumi#18680.

@CamSoper CamSoper closed this Apr 23, 2026
@CamSoper CamSoper deleted the test-pipeline/infra-edit branch April 23, 2026 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review:claude-ran Claude review has completed for this PR's current state review:claude-stale New commits since last Claude review; refresh on next ready-transition or @claude mention review:infra PR touches workflows, scripts, infra, Makefile, or build config

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant