ci: assert on adapter-node artifacts in validate smoke check #13

Merged

forgejo_admin merged 1 commit from 12-fix-validate-adapter-node-artifact into main

2026-04-30 12:26:39 +00:00

forgejo_admin commented

2026-04-30 12:21:38 +00:00

Owner

Summary

The .woodpecker.yaml validate step's smoke check test -f build/index.html is wrong for @sveltejs/adapter-node, which produces build/index.js + build/handler.js instead of a static HTML entry.
Build itself was always green; only the assertion failed, which blocked Woodpecker pipeline #1 from reaching the Kaniko build-and-push step.
Switches to asserting on build/index.js (the entry script package.json's start invokes) plus build/handler.js for completeness.

Changes

.woodpecker.yaml: replace test -f build/index.html with test -f build/index.js and test -f build/handler.js in the validate step (line 33).

Test Plan

PR pipeline runs: validate step now passes (both adapter-node artifacts exist after npm run build)
After merge, push event triggers full pipeline on main: validate → build-and-push (Kaniko → Harbor) → update-kustomize-tag (auto-PR to pal-e-deployments)
No regressions on pull_request runs (validate still gates Kaniko, Kaniko still skips on PR per existing when: branch: main)
After ArgoCD picks up the new tag, westside-admin deployment transitions from 0/1 to 1/1 ready

Review Checklist

Passed automated review-fix loop (review approved via /review-ticket → review-1117-2026-04-30)
No secrets committed
No unnecessary file changes (single line in .woodpecker.yaml, +2/-1)
Commit message describes the why (not just the what)

forgejo_admin/westside-admin #12 — the Forgejo issue this PR closes
westside-admin — the project this work belongs to
story-westside-admin-admin-row-crud — the user story this traces to
arch-ci-pipeline — the architecture component touched
review-1117-2026-04-30 — scope review note (verdict: APPROVED)

Discovered scope

westside-contracts also uses adapter-node but has no smoke check at all (per /review-ticket). Not a regression — won't be addressed here. Worth a separate ticket if Lucas wants parity.

Closes #12

## Summary - The `.woodpecker.yaml` validate step's smoke check `test -f build/index.html` is wrong for `@sveltejs/adapter-node`, which produces `build/index.js` + `build/handler.js` instead of a static HTML entry. - Build itself was always green; only the assertion failed, which blocked Woodpecker pipeline #1 from reaching the Kaniko `build-and-push` step. - Switches to asserting on `build/index.js` (the entry script `package.json`'s `start` invokes) plus `build/handler.js` for completeness. ## Changes - `.woodpecker.yaml`: replace `test -f build/index.html` with `test -f build/index.js` and `test -f build/handler.js` in the `validate` step (line 33). ## Test Plan - [ ] PR pipeline runs: `validate` step now passes (both adapter-node artifacts exist after `npm run build`) - [ ] After merge, `push` event triggers full pipeline on main: `validate` → `build-and-push` (Kaniko → Harbor) → `update-kustomize-tag` (auto-PR to pal-e-deployments) - [ ] No regressions on `pull_request` runs (validate still gates Kaniko, Kaniko still skips on PR per existing `when: branch: main`) - [ ] After ArgoCD picks up the new tag, westside-admin deployment transitions from 0/1 to 1/1 ready ## Review Checklist - [ ] Passed automated review-fix loop (review approved via `/review-ticket` → `review-1117-2026-04-30`) - [ ] No secrets committed - [ ] No unnecessary file changes (single line in `.woodpecker.yaml`, +2/-1) - [ ] Commit message describes the why (not just the what) ## Related Notes - `forgejo_admin/westside-admin #12` — the Forgejo issue this PR closes - `westside-admin` — the project this work belongs to - `story-westside-admin-admin-row-crud` — the user story this traces to - `arch-ci-pipeline` — the architecture component touched - `review-1117-2026-04-30` — scope review note (verdict: APPROVED) ## Discovered scope - `westside-contracts` also uses `adapter-node` but has no smoke check at all (per /review-ticket). Not a regression — won't be addressed here. Worth a separate ticket if Lucas wants parity. Closes #12

forgejo_admin added 1 commit

2026-04-30 12:21:38 +00:00

ci: assert on adapter-node artifacts in validate smoke check

ci/woodpecker/pr/woodpecker Pipeline was successful

Details

b94b444ebf

The post-build smoke check `test -f build/index.html` was wrong for
@sveltejs/adapter-node, which produces a Node server bundle (build/index.js,
build/handler.js) instead of a static HTML entry. The build itself succeeded
on every run; only the assertion failed. This blocked Woodpecker pipeline #1
from reaching the build-and-push (Kaniko) step, which is why no image has
landed in Harbor yet.

Switch to asserting on build/index.js (the canonical Node entry that
package.json's `start: node build/index.js` script invokes) plus
build/handler.js for completeness.

Closes #12

forgejo_admin commented

2026-04-30 12:23:17 +00:00

Author

Owner

PR #13 Review

DOMAIN REVIEW

Tech stack: Woodpecker CI + SvelteKit adapter-node. Single-line CI smoke-check correction in .woodpecker.yaml.

Correctness:

@sveltejs/adapter-node emits a Node entry script (build/index.js) plus a request handler module (build/handler.js); it never produces a static build/index.html. The previous assertion was structurally impossible to satisfy.
Asserting on build/index.js mirrors package.json's "start": "node build/index.js", so the smoke check now matches the actual runtime entrypoint.
Adding build/handler.js as a second assertion is solid defense-in-depth — the issue body explicitly allowed this and it catches partial-build failures where the entry script exists but handler emit broke.

YAML safety: Diff replaces one - test -f ... line with two at the same 6-space indent inside the validate step's commands: list. No colons-in-values, no quoting hazards, no anchors touched. Low risk per feedback_yaml_parse_validation. Woodpecker parse-validates on push regardless.

Pipeline gating preserved:

validate still runs on event: [push, pull_request, manual] (line 34-35) — gating intact.
Kaniko build-and-push step's when block (excluding pull_request) is untouched — PR runs still won't push to Harbor.
Step ordering (validate before build-and-push) means assertion failure still blocks Kaniko, which is the intended behavior.

Blast radius (verified via the linked review note review-1117-2026-04-30):

westside-app uses adapter-static — build/index.html IS correct there. No fan-out fix needed.
westside-contracts uses adapter-node but has no smoke check at all. PR explicitly defers this as discovered scope. Correct call.
No other sibling repos affected.

BLOCKERS

None.

NITS

None for this PR. The discovered-scope item (westside-contracts adapter-node parity smoke check) belongs in its own ticket if Lucas wants the parity, as the PR body states.

SOP COMPLIANCE

Branch named after issue (12-fix-validate-adapter-node-artifact — {issue}-{kebab-purpose})
PR body follows template: ## Summary, ## Changes, ## Test Plan, ## Review Checklist, ## Related Notes, ## Discovered scope
Related references story slug (story-westside-admin-admin-row-crud), arch slug (arch-ci-pipeline), and review slug (review-1117-2026-04-30)
Closes #12 link present
No secrets, no .env, no credentials in diff
No scope creep — single file, +2/-1 lines, surgically targeted
Commit/PR title is descriptive (ci: assert on adapter-node artifacts in validate smoke check) — explains the why, not just the what
Discovered scope (westside-contracts) is flagged but explicitly deferred per feedback_stay_in_scope

PROCESS OBSERVATIONS

Deployment frequency: positive impact. Pipeline #1 was blocked at validate and never reached Kaniko, so the westside-admin image has never been pushed to Harbor. This unblocks the first real deploy of the admin app and unblocks downstream feature merges (#1, #2, #4) from being shippable.
Change failure risk: minimal. Smoke-check change has zero runtime blast radius — assertion paths are CI-only; production runtime already targets build/index.js. If somehow wrong, it would simply fail CI on the next push, not break prod.
MTTR signal: ~5 days from CI being added (#11, 2026-04-25) to discovery (manual pipeline #1 trigger) to fix scoped+approved+PR'd in one cycle today. The latent gap was webhook-install lag rather than a missed test, so QA-side preventability is limited; the relevant control is exactly the smoke-check assertion this PR is correcting.
Documentation gaps: none introduced. Discovered-scope flag for westside-contracts is the correct documentation-side outcome.
Validation: pipeline-driven verification is the appropriate path for a CI-only change. The act of this PR's validate step passing is itself the proof of AC1-AC3.

VERDICT: APPROVED

## PR #13 Review ### DOMAIN REVIEW **Tech stack**: Woodpecker CI + SvelteKit `adapter-node`. Single-line CI smoke-check correction in `.woodpecker.yaml`. **Correctness**: - `@sveltejs/adapter-node` emits a Node entry script (`build/index.js`) plus a request handler module (`build/handler.js`); it never produces a static `build/index.html`. The previous assertion was structurally impossible to satisfy. - Asserting on `build/index.js` mirrors `package.json`'s `"start": "node build/index.js"`, so the smoke check now matches the actual runtime entrypoint. - Adding `build/handler.js` as a second assertion is solid defense-in-depth — the issue body explicitly allowed this and it catches partial-build failures where the entry script exists but handler emit broke. **YAML safety**: Diff replaces one `- test -f ...` line with two at the same 6-space indent inside the `validate` step's `commands:` list. No colons-in-values, no quoting hazards, no anchors touched. Low risk per `feedback_yaml_parse_validation`. Woodpecker parse-validates on push regardless. **Pipeline gating preserved**: - `validate` still runs on `event: [push, pull_request, manual]` (line 34-35) — gating intact. - Kaniko `build-and-push` step's `when` block (excluding `pull_request`) is untouched — PR runs still won't push to Harbor. - Step ordering (`validate` before `build-and-push`) means assertion failure still blocks Kaniko, which is the intended behavior. **Blast radius (verified via the linked review note `review-1117-2026-04-30`)**: - `westside-app` uses adapter-static — `build/index.html` IS correct there. No fan-out fix needed. - `westside-contracts` uses adapter-node but has no smoke check at all. PR explicitly defers this as discovered scope. Correct call. - No other sibling repos affected. ### BLOCKERS None. ### NITS None for this PR. The discovered-scope item (westside-contracts adapter-node parity smoke check) belongs in its own ticket if Lucas wants the parity, as the PR body states. ### SOP COMPLIANCE - [x] Branch named after issue (`12-fix-validate-adapter-node-artifact` — `{issue}-{kebab-purpose}`) - [x] PR body follows template: `## Summary`, `## Changes`, `## Test Plan`, `## Review Checklist`, `## Related Notes`, `## Discovered scope` - [x] Related references story slug (`story-westside-admin-admin-row-crud`), arch slug (`arch-ci-pipeline`), and review slug (`review-1117-2026-04-30`) - [x] `Closes #12` link present - [x] No secrets, no `.env`, no credentials in diff - [x] No scope creep — single file, +2/-1 lines, surgically targeted - [x] Commit/PR title is descriptive (`ci: assert on adapter-node artifacts in validate smoke check`) — explains the why, not just the what - [x] Discovered scope (westside-contracts) is flagged but explicitly deferred per `feedback_stay_in_scope` ### PROCESS OBSERVATIONS - **Deployment frequency**: positive impact. Pipeline #1 was blocked at `validate` and never reached Kaniko, so the westside-admin image has never been pushed to Harbor. This unblocks the first real deploy of the admin app and unblocks downstream feature merges (#1, #2, #4) from being shippable. - **Change failure risk**: minimal. Smoke-check change has zero runtime blast radius — assertion paths are CI-only; production runtime already targets `build/index.js`. If somehow wrong, it would simply fail CI on the next push, not break prod. - **MTTR signal**: ~5 days from CI being added (#11, 2026-04-25) to discovery (manual pipeline #1 trigger) to fix scoped+approved+PR'd in one cycle today. The latent gap was webhook-install lag rather than a missed test, so QA-side preventability is limited; the relevant control is exactly the smoke-check assertion this PR is correcting. - **Documentation gaps**: none introduced. Discovered-scope flag for westside-contracts is the correct documentation-side outcome. - **Validation**: pipeline-driven verification is the appropriate path for a CI-only change. The act of this PR's validate step passing is itself the proof of AC1-AC3. ### VERDICT: APPROVED