diff --git a/.planning/phases/11.2-ready-to-share-summercms-io-website-and-newsletter-plugin/11.2-REVIEW-DISPOSITION.md b/.planning/phases/11.2-ready-to-share-summercms-io-website-and-newsletter-plugin/11.2-REVIEW-DISPOSITION.md index f830b7d..131cb93 100644 --- a/.planning/phases/11.2-ready-to-share-summercms-io-website-and-newsletter-plugin/11.2-REVIEW-DISPOSITION.md +++ b/.planning/phases/11.2-ready-to-share-summercms-io-website-and-newsletter-plugin/11.2-REVIEW-DISPOSITION.md @@ -10,13 +10,13 @@ One row per finding in 11.2-REVIEW.md. Default `open`; set to `fixed`, `accepted | ID | Severity | Finding | Disposition | Note | |----|----------|---------|-------------|------| -| CR-01 | critical | The install root is owned by the service account, so root's deploy steps can be redirected through symlinks | open | | -| WR-01 | warning | The HTTPS server block sends no Strict-Transport-Security header | open | | -| WR-02 | warning | The HTTP block redirects ACME challenges, and the app 404s `/.well-known/` | open | | -| WR-03 | warning | The D-40 terminal check puts GOBIN on PATH, so it does not prove the page's commands work in a fresh shell | open | | -| WR-04 | warning | The site test gate and the `pnpm test` script only work on Node 22.18 to 22.x | open | | -| WR-05 | warning | The smoke script can test a different server that already holds the port | open | | -| WR-06 | warning | A release build does not compile what its plugin tests checked, and does not require clean submodules | open | | +| CR-01 | critical | The install root is owned by the service account, so root's deploy steps can be redirected through symlinks | fixed | sm-summercmsio-app 8e5765e | +| WR-01 | warning | The HTTPS server block sends no Strict-Transport-Security header | fixed | sm-summercmsio-app 1dcf774 (no includeSubDomains; check on rome) | +| WR-02 | warning | The HTTP block redirects ACME challenges, and the app 404s `/.well-known/` | fixed | sm-summercmsio-app 1e934ab (webroot path to check on rome) | +| WR-03 | warning | The D-40 terminal check puts GOBIN on PATH, so it does not prove the page's commands work in a fresh shell | accepted | user chose option 1: card unchanged, PATH step added to the installation guide (summercms.go bd2f7e7) | +| WR-04 | warning | The site test gate and the `pnpm test` script only work on Node 22.18 to 22.x | fixed | vue-summercmsio-app 60c3c4a, summercms.go be51bfe, sm-summercmsio-app aeac7bf | +| WR-05 | warning | The smoke script can test a different server that already holds the port | fixed | sm-summercmsio-app 8cfe055 | +| WR-06 | warning | A release build does not compile what its plugin tests checked, and does not require clean submodules | fixed | sm-summercmsio-app fffa38c | | IN-01 | info | The extension-less docs 301 drops the query string | open | | | IN-02 | info | `site_label` in site.yaml cannot be paired with `--site-url` alone | open | | | IN-03 | info | The og:image alt text is not reactive, unlike the other SEO fields | open | | @@ -26,4 +26,4 @@ One row per finding in 11.2-REVIEW.md. Default `open`; set to `fixed`, `accepted | IN-07 | info | The external link check follows redirects, so a login redirect would count as a pass | open | | | IN-08 | info | The scroll-spy section list is defined twice | open | | -open: 15 of 15 +fixed: 6, accepted: 1, open: 8 of 15 (IN-01..IN-08, info, out of fix scope) diff --git a/.planning/phases/11.2-ready-to-share-summercms-io-website-and-newsletter-plugin/11.2-REVIEW-FIX.md b/.planning/phases/11.2-ready-to-share-summercms-io-website-and-newsletter-plugin/11.2-REVIEW-FIX.md new file mode 100644 index 0000000..631c624 --- /dev/null +++ b/.planning/phases/11.2-ready-to-share-summercms-io-website-and-newsletter-plugin/11.2-REVIEW-FIX.md @@ -0,0 +1,129 @@ +--- +phase: 11.2-ready-to-share-summercms-io-website-and-newsletter-plugin +fixed_at: 2026-10-01T15:11:41Z +review_path: /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 +findings_in_scope: 7 +fixed: 6 +skipped: 1 +status: 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//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_ diff --git a/.planning/phases/11.2-ready-to-share-summercms-io-website-and-newsletter-plugin/11.2-VALIDATION.md b/.planning/phases/11.2-ready-to-share-summercms-io-website-and-newsletter-plugin/11.2-VALIDATION.md index ad82091..abf0348 100644 --- a/.planning/phases/11.2-ready-to-share-summercms-io-website-and-newsletter-plugin/11.2-VALIDATION.md +++ b/.planning/phases/11.2-ready-to-share-summercms-io-website-and-newsletter-plugin/11.2-VALIDATION.md @@ -21,7 +21,7 @@ validated: "2026-10-01" | Property | Value | |----------|-------| | **Framework** | Go `testing` (stdlib, `httptest`, `testing/fstest`) in summercms.go, sm-summercmsio-plugin and sm-summercmsio-app; `node:test` in vue-summercmsio-app | -| **Config file** | none; `vue-summercmsio-app/package.json` script `"test": "node --test tests/*.test.ts"` | +| **Config file** | none; `vue-summercmsio-app/package.json` script `"test": "node --test --test-reporter=tap tests/*.test.ts"` | | **Quick run command** | touched repo: `go vet ./... && go test ./...`; site: `pnpm generate && pnpm test` | | **Full suite command** | framework `go vet ./... && go test ./...`; app `scripts/build.sh dev` then `go vet ./... && go test ./...` (plugin with `SUMMERCMS_REQUIRE_BUILD=1`); site `pnpm generate && pnpm test` | | **Phase gate** | `scripts/check-phase11.2.sh --all` (stages `--framework`, `--plugin`, `--app`, `--site`, `--built`, `--smoke`, `--deploy`, `--terminal`, `--full`) |