golem15/summercms
3
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
570be0a49372eec35232c8c113b91c28eaa4cc6a
summercms/.planning/phases/11.2-ready-to-share-summercms-io-website-and-newsletter-plugin/11.2-REVIEW-FIX.md

9.0 KiB
Raw Blame History

phase, fixed_at, review_path, iteration, findings_in_scope, fixed, skipped, status
phase fixed_at review_path iteration findings_in_scope fixed skipped status
11.2-ready-to-share-summercms-io-website-and-newsletter-plugin 2026-10-01T15:11:41Z /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/.planning/phases/11.2-ready-to-share-summercms-io-website-and-newsletter-plugin/11.2-REVIEW.md 1 7 6 1 partial

Phase 11.2: Code Review Fix Report

Fixed at: 2026-10-01T15:11:41Z Source review: /media/nvme/dev/golem15/summercms.io/summercms/summercms.go/.planning/phases/11.2-ready-to-share-summercms-io-website-and-newsletter-plugin/11.2-REVIEW.md Iteration: 1

Summary:

  • Findings in scope: 7 (CR-01, WR-01 to WR-06; IN-* out of scope)
  • Fixed: 6
  • Skipped: 1 (WR-03, needs a user decision)

workflow.use_worktrees is false, so every edit and commit happened in the main checkouts. No worktree was created and nothing was pushed or tagged.

Fixed Issues

CR-01: The install root is owned by the service account, so root's deploy steps can be redirected through symlinks

Repo: sm-summercmsio-app Files modified: DEPLOY.md, deploy/env.example Commit: 8e5765e Applied fix:

  • The service account is now created with --no-create-home --home /nonexistent.
  • /srv/summercms-io, bin/ and config/ are created root:root 0755. Only storage/ belongs to the service.
  • .env is created with install -o root -g summercms -m 0640 /dev/null, so the service can read it but not write it. The overview table and the env.example header now say the same.
  • DEPLOY.md explains why the install root must stay root-owned and adds a stat ownership check to run before each deploy.
  • It also records that serve writes only below storage/. I checked the framework: the uploads bucket uses create_dir. The only other runtime writer is compass.Persist, which writes config/env/<env>/overrides.yaml, and only the admin calls it. The site has no admin, and that write would now fail on the root-owned config/.

WR-01: The HTTPS server block sends no Strict-Transport-Security header

Repo: sm-summercmsio-app Files modified: deploy/nginx/summercms.io.conf, DEPLOY.md Commit: 1dcf774 Applied fix:

  • Added add_header Strict-Transport-Security "max-age=31536000" always; to both the apex and the www HTTPS server blocks. Neither block's location has its own add_header, so the header is inherited.
  • Left out includeSubDomains on purpose. Which subdomains on rome serve HTTPS cannot be checked from here, so it is marked check on rome in a comment.
  • Added two curl checks for the header to "Verify after cutover".

WR-02: The HTTP block redirects ACME challenges, and the app 404s /.well-known/

Repo: sm-summercmsio-app Files modified: deploy/nginx/summercms.io.conf, DEPLOY.md Commit: 1e934ab Applied fix:

  • The port-80 server now serves location ^~ /.well-known/acme-challenge/ from root /var/www/letsencrypt. Every other path still gets the 301 to the HTTPS apex, now from location /.
  • Cutover step 2 now checks the lineage's authenticator and webroot_path and says what to do for webroot and for nginx.
  • Step 3 now runs certbot renew --cert-name summercms.io --dry-run after the reload.
  • The webroot path is a default and is marked check on rome.
  • The config comment avoids the literal /etc/letsencrypt path, because check-deploy.sh refuses any /etc/letsencrypt string it has not replaced.

WR-04: The site test gate and the pnpm test script only work on Node 22.18 to 22.x

Repos: vue-summercmsio-app, summercms.go, sm-summercmsio-app (gitlink) Files modified: vue-summercmsio-app/package.json, vue-summercmsio-app/README.md, summercms.go/scripts/check-phase11.2.sh, sm-summercmsio-app gitlink for vue-summercmsio-app Commits: 60c3c4a (vue-summercmsio-app), be51bfe (summercms.go), aeac7bf (sm-summercmsio-app gitlink) Applied fix:

  • The test script is now node --test --test-reporter=tap tests/*.test.ts.
  • engines.node is now >=22.18, and the README requirement line matches.
  • In run_site, the informational summary grep is now || true, so the explicit refuse checks report a missing TAP summary.
  • The test command quoted in 11.2-VALIDATION.md (lines 24 and 49) is now stale. That is a planning doc, so I left it for the orchestrator.

WR-05: The smoke script can test a different server that already holds the port

Repo: sm-summercmsio-app Files modified: scripts/smoke.sh Commit: 8cfe055 Applied fix:

  • Before serve starts, the script refuses a busy port. It requires curl exit 7 (connection refused) and fails on any other result, with a message that names SMOKE_PORT.
  • The readiness loop now checks that $PID is alive before each curl.
  • After the first response it waits 0.5 s, requires $PID to still be alive (a failed bind exits serve), and requires listening on 127.0.0.1:$PORT in serve.log.

WR-06: A release build does not compile what its plugin tests checked, and does not require clean submodules

Repo: sm-summercmsio-app Files modified: scripts/build.sh, DEPLOY.md Commit: fffa38c Applied fix:

  • New require_clean step. It runs git status --porcelain --ignore-submodules=none on the app, the site and the plugin, so it also catches app gitlinks that differ from the checked-out submodule commits. A release build runs it before the build and again after summer build; the existing check on main.go and plugins.gen.go is kept.
  • I left out the reviewer's ':!public' exclusion. The plugin's .gitignore already ignores public/site/ and public/docs/, and the exclusion would also have hidden edits to the tracked public/README.md.
  • The temp release go.work is now written by write_release_work after the tag export (step 2). In release mode, step 3 runs the plugin tests with GOWORK="$TMP/go.work", the same file the final go build uses. Dev mode is unchanged.
  • The build.sh header and the build paragraph in DEPLOY.md describe the new release rules.

Skipped Issues

WR-03: The D-40 terminal check puts GOBIN on PATH, so it does not prove the page's commands work in a fresh shell

File: /media/nvme/dev/golem15/summercms.io/summercms/sm-summercmsio-app/terminal_check_test.go:68-88,107-109 (copy: vue-summercmsio-app/app/data/terminal.json:9,13) Reason: Needs a user decision (D-40: page copy changes require approval). Resolution (2026-10-01, user decision, option 1): the page copy and the check stay as they are. docs/setup/installation.md (the card's "Installation guide" target) now explains that go install writes to $(go env GOPATH)/bin and shows export PATH="$(go env GOPATH)/bin:$PATH"; summercms.go commit bd2f7e7. Original issue: terminalEnv prepends the temp GOBIN to PATH, so TestTerminalCommands passes even though summer build after go install ./cmd/summer fails in a common fresh shell where $(go env GOPATH)/bin is not on PATH. The options are to change the check so it fails today and documents the gap, or to make the copy PATH-independent with approval.

Verification

All checks ran in the main checkouts (workflow.use_worktrees: false), so they can be reproduced from the trees as committed.

Fix Check Result
CR-01, WR-01, WR-02 scripts/check-deploy.sh (nginx -t on the edited config, supervisor parse) after each edit check-deploy: ok
WR-04 pnpm test in vue-summercmsio-app (Node v22.23.2) # tests 30, # pass 30, # fail 0
WR-04 bash scripts/check-phase11.2.sh --site phase11.2 site passed
WR-05 scripts/smoke.sh on the built binary smoke: ok
WR-05 the same, with python3 -m http.server 18095 holding the port refused: port 18095 is already in use (curl exit 0)
WR-06 scripts/build.sh dev bin/summercms-io (dev) ready
WR-06 release rehearsal while the app was dirty, against a scratch framework clone with a local v0.1.0 tag refused, listing the dirty files
WR-06 release rehearsal after the commit bin/summercms-io (release v0.1.0) ready; plugin tests passed under the release go.work; go version -m shows git.golem15.com/golem15/summercms v0.1.0
WR-06 release rehearsal with a tracked edit in the plugin submodule (reverted afterwards) refused: M plugins/golem15/summercms
All bash scripts/check-phase11.2.sh --all in summercms.go phase11.2 all passed (framework, plugin, app, site, built, smoke, deploy, terminal and full stages)
All go vet ./... && go test ./... in summercms.go green
  • The scratch framework clone with the local tag was deleted. No real v0.1.0 tag was created.
  • The last gate run left bin/summercms-io as a dev build.
  • SummerCMS landing page.zip was already untracked in summercms.go before this run, and I did not touch it.
  • The new nginx directives were checked with nginx -t only. The HSTS header, the ACME webroot and the certbot dry run can only be checked on rome at cutover; DEPLOY.md now includes those steps.

Fixed: 2026-10-01T15:11:41Z Fixer: Claude (gsd-code-fixer) Iteration: 1

Reference in New Issue View Git Blame Copy Permalink