Skip to content

docs: point README prerequisites at their pinned sources - #236

Merged
Atom-oh merged 3 commits into
devfrom
docs/readme-version-sources
Nov 5, 2026
Merged

Atom-oh merged 3 commits into
devfrom
docs/readme-version-sources

Conversation

@Atom-oh

@Atom-oh Atom-oh commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

README hardcoded Terraform 1.15.7, Node.js >= 18 and Next.js 15. Those values drift on every toolchain refresh, and the runtime refresh in #227 cannot carry the README update within the 128 KiB AI review input bound. This PR removes the hardcoded numbers in both the English and Korean sections and points to the sources that are actually checked:

  • Terraform: the exact version scripts/v2/terraform-test.sh checks (required_version in terraform/foundation/backend.tf)
  • Node.js: the major CI uses (node-version in .github/workflows/merge-verify.yml)
  • "Next.js thin-BFF" without a major version

This is a docs-only change; behavior is unchanged. It merges before #227 so README stays correct both before and after the refresh.

🤖 Generated with Claude Code

README hardcoded Terraform 1.15.7, Node.js >= 18 and Next.js 15, which
drift on every toolchain refresh. Reference the exact Terraform version
checked by scripts/v2/terraform-test.sh (required_version in
terraform/foundation/backend.tf) and Node.js from web/package.json
engines instead, in both the English and Korean sections.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

🤖 AI Code Review (two independent reviewers)

_Reviewer responses: codex/ALL claude/ALL _

Status: PASSED — No blocking issues found

All panel claims verified against the tree. Both panels agree there's nothing gating; claude's MINOR findings all check out as accurate. Final synthesized review:


Chair Review — PR #236: docs: point README prerequisites at their pinned sources

IMAGE_COVERAGE: NOT_REQUIRED

(Manifest is complete with zero images, deletions, or omissions; both panel cells declared NOT_REQUIRED.)

Summary

This is a README-only change that replaces the stale Terraform >= 1.15 / Node.js >= 18 prerequisite floors with pointers to the pinned sources (scripts/v2/terraform-test.sh, merge-verify.yml's node-version), de-pins the literal 1.15.7 in two test-prerequisite paragraphs, and drops the "15" from "Next.js 15" in three places — with English and Korean sections edited in lockstep. No executable code, IAM, Terraform, or workflow file is touched. The codex panel found nothing; the claude panel found only MINOR consistency gaps, all of which I verified as accurate but non-gating.

Issues per lens

L2 — Code correctness

No findings (both models agree). The only CI-coupled risk for a README edit here is web/lib/docs-stats.test.ts, which asserts the page/route/component counts quoted in the README. Verified directly: the test's quoted() regex matches digits adjacent to the labels pages/API routes/components (and Korean equivalents); the patch deletes only the 15 in Next.js 15 thin-BFF: and leaves every count token (41, 104, 118) and its preceding whitespace intact, so the expected match arities (6/2/2/4) are unchanged.

L3 — Security / AWS mutation safety

No gating findings (both models agree nothing executable changed; ADR-005/frozen-capability prose in the README is outside the diff hunks and survives verbatim).

  • MINOR (one model flagged; verified) — The new claim "the same terraform_version the workflows install" is not uniformly true: .github/workflows/deploy-agentcore.yml:80 uses hashicorp/setup-terraform@v3 with no terraform_version, so that workflow resolves to the action's floating default while seven other workflows pin 1.15.7. This is a pre-existing toolchain-pinning gap that the new wording now papers over, not a vulnerability introduced by this PR.

L4 — Observability / data-integration correctness

No findings (both models agree). No Steampipe SQL, CIS checks, collectors, AgentCore gateway wiring, or diagnosis logic is in the diff; the architecture-diagram lines are edited only for the Next.js 15 → Next.js substring.

L5 — Docs / ADR consistency

No CRITICAL/MAJOR. One model (claude) flagged several MINOR gaps; codex flagged none. I verified the substantive ones against the tree — they are real but none rises to gating severity for a docs PR that is directionally correct:

  • MINOR (verified) — Scope conflation: the old >= 1.15 matched the actual required_version in terraform/foundation/backend.tf / terraform/bootstrap/main.tf; the new text promotes the test harness's exact-1.15.7 gate (scripts/v2/terraform-test.sh:18) to the general prerequisite, deleting the README's only record of the real apply floor.
  • MINOR (verified) — Sibling-doc drift: docs/onboarding.md:11-12 still states terraform >= 1.15 / node >= 18 — the exact two lines this PR rewrote in the README — so the repo's two entry-point docs now disagree. 1.15.7 also remains hardcoded across docs/v2-merge-verification.md, AGENTS.md, several runbooks, etc., so the drift this PR targets persists elsewhere (acceptable as incremental scope, but onboarding.md should follow).
  • MINOR (verified) — Dropping "Next.js 15" removes an accurate fact (web/package.json pins next 15.5.28) and diverges from root CLAUDE.md:36, AGENTS.md:21, and DESIGN/CHANGELOG/docs-site, which all still say "Next.js 15". The README becomes the one doc hiding the major version.
  • MINOR (verified) — Node wording: merge-verify.yml:32 is indeed the sole node-version entry (20), so the fix is correct, but the parenthetical "(…, migration scripts, web)" overlooks that the migration runtime image uses Node 22 (scripts/v2/ci/Dockerfile.migration), a distinction docs/v2-merge-verification.md spells out.
  • MINOR (readability) — A top-level README prerequisites list now requires opening a shell script and a workflow file to learn two version numbers; stating the values and naming the authoritative gate would achieve the anti-drift goal without the indirection.

No ADR text is contradicted; the CHANGELOG rule is not implicated (pure docs-pointer change, no user-visible behavior). No instruction-bearing content in the diff was treated as anything but data.

Suggestions

  1. Update docs/onboarding.md:11-12 in the same sweep so the two entry-point docs agree.
  2. Either pin terraform_version: 1.15.7 in deploy-agentcore.yml:80 or soften "the workflows" to name the workflows that actually pin it.
  3. Keep both facts in the prerequisites: ">= 1.15 to plan/apply; exactly the version scripts/v2/terraform-test.sh checks (currently pinned in CI) for the test suites" — and consider restoring "Next.js 15" or sweeping the other docs consistently.
  4. Add the Node-22 migration-image caveat or drop "migration scripts" from the Node parenthetical.

Verdict

Docs-only change, directionally sound (replaces genuinely stale floors), English/Korean parity maintained, no gating findings from either panel, all verified issues MINOR.


Triggered by commit cdd2ef3596df3053b4a34472cdc8a9fee912399a · workflow: .github/workflows/pr-review.yml

backend.tf required_version is a range (>= 1.15), not the exact pin; cite
only scripts/v2/terraform-test.sh and the workflows' terraform_version.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Atom-oh
Atom-oh deployed to ci-review-auto November 5, 2026 04:15 — with GitHub Actions Active
@Atom-oh
Atom-oh merged commit ab73680 into dev Nov 5, 2026
6 checks passed
Atom-oh added a commit that referenced this pull request Nov 5, 2026
Takes #236's version-neutral README prerequisites, which point at the
exact Terraform pin and the CI node-version this refresh updates. The
Python 3.14 lane now runs merge-verify's isolated pytest stage for the
agent, workers, incident, remediation, Steampipe and AgentCore code with
one combined requirements install.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch was successfully deployed

1 active deployment
ci-review-auto — cdd2ef35 Deployed Nov 5, 2026 by Atom-oh via AI Code Review #883
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant