Repository navigation
Contribution standards drift from what CI enforces and what reviewers ask for #2736
Description
CONTRIBUTING.md, AGENTS.md, the PR template and the AI review prompt are the four files a new contributor reads before their first PR. Several things they need are missing, and one is wrong. Each item below is checkable in this repo.
Wrong
AGENTS.mdtells contributors to regeneratecommand.mdafter changing a command name, flag or description. The release workflow already does this —.github/workflows/release-prepare.ymlrunsbun scripts/generate-command-reference.mjsand commits the result in the release PR. Regenerating on a feature branch also picks up every command change merged since the last release, so the PR carries a large unrelated diff that gets asked back out.
Missing from the required checks
bun run auditandbun run secrets:checkare gates in.github/workflows/verify.yml, but neitherCONTRIBUTING.md's check list nor the PR template mentions them. A contributor who runs the five documented commands still fails CI.
Missing: the golden-file / record-replay pattern
CONTRIBUTING.md explains the pattern and AGENTS.md gives the recipe, but four things that decide whether a recorded test reproduces on anyone else's machine are undocumented:
matchGoldenstrips presigned URLs and normalises JSON (src/testing/fixtures.tsx), and nothing else, so recordings carry the recording account's IDs in their ARNs (most committed fixtures and goldens do). A re-record from another account rewrites every ARN, and nothing tells a contributor that this churn is expected.Missing fixture …andMissing golden file …have different causes and different fixes; only the first is documented.- A fresh
CoreClientper run is required because it carries per-run caches. This is stated only in a code comment insrc/handlers/harness/harness.test.tsx. fixtureFactoriesreturns one factory per AWS client the CLI talks to. A handler reaching a service not in that list cannot replay until the factory is added.
Missing: cross-repository and architectural rules
- A field that must reach a deployed resource needs the CloudFormation property, then the released L3 construct from
aws/agentcore-l3-cdk-constructs, then the@aws/agentcore-cdkpin bump insrc/assets/cdk/package.json. Nothing says this, and the PR template has nowhere to link the companion PR. src/core/must not import fromsrc/handlers/. The dependency-inversion section implies it; it is not stated.- The e2e suite is not a PR gate —
.github/workflows/e2e-test.ymlruns only onworkflow_call/workflow_dispatchbehind an authorization check — so a contributor has to run it locally. The PR template's e2e box reads as though CI covers it.
Missing: things reviewers ask for repeatedly
The AI review prompt lists one pattern. Recurring review findings that it could catch first include comments that restate the code, tests that assert the implementation rather than behaviour, a new flag whose absent case is untested, dead guards on middleware-resolved context values, and an error hint added to a screen but missing from the headless path.
A PR follows.
Activity