Skip to content

programs: pipeline-test bucket program#28

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

programs: pipeline-test bucket program#28
CamSoper wants to merge 2 commits intomasterfrom
test-pipeline/programs-edit

Conversation

@CamSoper
Copy link
Copy Markdown
Owner

Adds a minimal S3 bucket example at static/programs/pipeline-test-bucket-typescript/.

Pipeline test: exercises the review:programs domain (heightened scrutiny) and the chained triage → review path. The program contains deliberate issues (wrong package, wrong casing, deprecated resource, wrong versioning type) so the reviewer has material to flag. Added to scripts/programs/ignore.txt so the real program test suite skips it.

@CamSoper CamSoper marked this pull request as ready for review April 23, 2026 21:11
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 23, 2026

🐿️ Review errored. Flip to draft and back to ready, or mention @claude, to retry.

@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 claude Bot added review:programs PR touches static/programs/ review:infra PR touches workflows, scripts, infra, Makefile, or build config review:mixed PR touches more than one domain fact-check:needed PR introduces factual claims; fact-check runs labels 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
@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
CamSoper added a commit that referenced this pull request Apr 23, 2026
The domain-selection tables in triage.md, docs-review.md,
docs-review-ci.md, and docs-review-core.md all treated paths as an
unordered set of globs. A PR touching static/programs/<name>/package.json
matched BOTH static/programs/ (programs) and package.json (infra), so
triage applied review:programs AND review:infra AND review:mixed --
which was noisy and semantically wrong (per-program package.json is
programs territory, not site infra). Same issue for
scripts/programs/ignore.txt (programs tooling, not site infra).

Switch to explicit path-precedence order. A file matches the first
rule and does not get classified again:

1. static/programs/** → programs (covers every nested file in a
   program directory: Pulumi.yaml, package.json, requirements.txt,
   source files, etc.)
2. content/blog/**, content/customers/** → blog
3. content/docs/**, content/learn/**, content/tutorials/**,
   content/what-is/** → docs
4. .github/workflows/**, scripts/** except scripts/programs/**,
   infrastructure/**, Makefile (repo root), package.json (repo root
   only), webpack.config.js, webpack.*.js → infra
5. Everything else → review-shared only

Caught during fork-based end-to-end testing of PR #28 (a programs-only
PR that was mislabeled review:infra + review:programs + review:mixed).
@claude claude Bot added the review:claude-ran Claude review has completed for this PR's current state label Apr 23, 2026
@claude
Copy link
Copy Markdown

claude Bot commented Apr 23, 2026

Claude Review — Last updated 2026-04-23T22:36:44Z

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

🚨 Outstanding in this PR

static/programs/pipeline-test-bucket-typescript/index.ts:2 — Wrong package name. @pulumi/pulumi-aws is not a published npm package; the Pulumi AWS provider SDK is @pulumi/aws. Confirmed against pulumi/pulumi-aws sdk/nodejs/ (the generated SDK publishes as @pulumi/aws). The program will fail npm install / tsc before any resource is created.

import * as aws from "@pulumi/aws";

static/programs/pipeline-test-bucket-typescript/package.json:10 — Same wrong package. The dependency key @pulumi/pulumi-aws will 404 on the npm registry. Use @pulumi/aws. (Sibling programs such as aws-s3bucket-bucketobject-interpolate-typescript pin @pulumi/aws to ^7.0.0 — see ⚠️ Low-confidence below for the version note.)

        "@pulumi/aws": "^6.0.0"

static/programs/pipeline-test-bucket-typescript/index.ts:4aws.s3.Bucket is the deprecated legacy form in the current Pulumi AWS provider; aws.s3.BucketV2 exists in pulumi/pulumi-aws sdk/nodejs/s3/bucketV2.ts and is the recommended form (per review-programs.md §Provider API currency). New example programs should use aws.s3.BucketV2 and, for versioning, a separate aws.s3.BucketVersioningV2 resource per the BucketV2 migration guide.

static/programs/pipeline-test-bucket-typescript/index.ts:5bucket_name is not a valid property on aws.s3.Bucket. The Pulumi TypeScript SDK is camelCase, and the property for the bucket's AWS name is bucket — there is no bucketName / bucket_name on BucketArgs (verified against pulumi/pulumi-aws sdk/nodejs/s3/bucket.ts). bucket_name is a snake_case Python-ism and will fail TypeScript typecheck.

    bucket: "pipeline-test-demo",

static/programs/pipeline-test-bucket-typescript/index.ts:6versioning: true is the wrong type. On aws.s3.Bucket the versioning input is typed pulumi.Input<inputs.s3.BucketVersioning> — an object, not a boolean (pulumi/pulumi-aws sdk/nodejs/s3/bucket.ts: declare public readonly versioning: pulumi.Output<outputs.s3.BucketVersioning>). A boolean will fail typecheck. If you keep the deprecated Bucket resource, the minimal fix is:

    versioning: {
        enabled: true,
    },

If you migrate to BucketV2 (recommended — see finding above), move versioning to a separate aws.s3.BucketVersioningV2 resource.

⚠️ Low-confidence

static/programs/pipeline-test-bucket-typescript/package.json:9@pulumi/aws pinned to ^6.0.0, but sibling TypeScript programs in this repo (e.g., static/programs/aws-s3bucket-bucketobject-interpolate-typescript/package.json) pin ^7.0.0. If the intent is consistency with the current stable of sibling programs, bump to ^7.0.0. Low-confidence because review-programs.md §Do-not-flag accepts dependency pins that match siblings — but here the siblings don't match this pin.

static/programs/pipeline-test-bucket-typescript/ — No package-lock.json or yarn.lock alongside package.json. review-programs.md §Project structure expects a lock file for TypeScript programs. Not a blocker here — the program is in scripts/programs/ignore.txt, so the test harness will skip it — but worth noting if this directory is ever removed from the ignore list.

💡 Pre-existing issues in touched files

None. All program files are new in this PR; no unchanged context to extract from.

✅ Resolved since last review

Initial review.

📜 Review history

  • 2026-04-23T22:07:46Z — Initial review; 5 code-correctness findings on the pipeline-test program (bfde16b)
  • 2026-04-23T22:36:44Z — Refreshed after new commits; dropped spurious .github/workflows/claude.yml finding (file not in PR diff); 5 outstanding program findings preserved (7c9a110)

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.

@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 marked this pull request as draft April 23, 2026 21:34
@CamSoper CamSoper marked this pull request as ready for review April 23, 2026 21:34
@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
CamSoper added a commit that referenced this pull request Apr 23, 2026
The previous procedure said 'compute the target label set (existing
minus removed, plus added)' which Sonnet glossed over: on PR #28, after
the domain rules changed to path-precedence, triage saw review:infra
+ review:mixed as stale and should have removed them, but ran as a
no-op.

Rewrite the procedure in explicit TARGET / ADD / REMOVE steps:

- TARGET is built from scratch per the classification rules.
- EXISTING_TRIAGE excludes state labels (review:claude-ran, -stale,
  -working, needs-author-response) so those stay untouched.
- ADD = TARGET − EXISTING_TRIAGE.
- REMOVE = EXISTING_TRIAGE − TARGET. Any previously-applied review:*
  or fact-check:needed label that no longer applies under current
  rules is stale and must be dropped.
- The gh pr edit call fires only when ADD or REMOVE is non-empty.
- Summary log line now includes added/removed lists for traceability.

Caught during fork-based end-to-end testing; Sonnet followed the
re-written procedure correctly on the next run.
@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 marked this pull request as draft April 23, 2026 22:00
@CamSoper CamSoper marked this pull request as ready for review April 23, 2026 22:00
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 23, 2026

🐿️ Review errored. Flip to draft and back to ready, or mention @claude, to retry.

@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
@CamSoper CamSoper marked this pull request as draft April 23, 2026 22:03
@CamSoper CamSoper marked this pull request as ready for review April 23, 2026 22:03
@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
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 23, 2026

🐿️ Review updated.

@github-actions github-actions Bot added review:claude-working Claude is running a review right now; auto-removed when finishes and removed review:claude-working Claude is running a review right now; auto-removed when finishes labels Apr 23, 2026
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.
@CamSoper CamSoper marked this pull request as draft April 23, 2026 22:30
@CamSoper CamSoper marked this pull request as ready for review April 23, 2026 22:30
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 23, 2026

🐿️ Review errored. Flip to draft and back to ready, or mention @claude, to retry.

@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
Adds a minimal TypeScript S3 bucket example for the PR review pipeline
to exercise. Added to ignore.txt so the real program test suite skips
it; the Claude review should flag the deliberate bugs.
@CamSoper CamSoper force-pushed the test-pipeline/programs-edit branch from bfde16b to 7c9a110 Compare April 23, 2026 22:32
@CamSoper CamSoper marked this pull request as draft April 23, 2026 22:32
@CamSoper CamSoper marked this pull request as ready for review April 23, 2026 22:32
@github-actions github-actions Bot added review:claude-stale New commits since last Claude review; refresh on next ready-transition or @claude mention and removed review:infra PR touches workflows, scripts, infra, Makefile, or build config review:mixed PR touches more than one domain review:claude-working Claude is running a review right now; auto-removed when finishes labels Apr 23, 2026
@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 claude Bot removed the review:claude-stale New commits since last Claude review; refresh on next ready-transition or @claude mention 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
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/programs-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

fact-check:needed PR introduces factual claims; fact-check runs review:claude-ran Claude review has completed for this PR's current state review:programs PR touches static/programs/

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant