Skip to content

Small fixes from the v1 review #9

New issue
New issue
Closed
#16
Closed
Small fixes from the v1 review#9
#16
Labels
codexSymphony: the Codex agent picks this card upenhancementNew feature or request

Description

@dorelljames
dorelljames
opened on Oct 28, 2026

Deferred minor findings from the whole-branch review before merge:

  • worker/index.ts fetches /404.html, which redirects. Fetch /404, or drop main. Today the assets layer serves misses, so it doesn't run.
  • brand.md links /brand/templates/*.html, which redirect (307) to the extensionless paths. Publish the extensionless URLs, and teach check-dist about it.
  • Old URLs with a trailing slash (/members/, /events/…) return 404. Add slash rows to _redirects.
  • No type check: tsc --noEmit fails in apps/website (test/content.test.ts), and .astro files are never checked. Add astro check or tsc to test.
  • /brand/tokens.css is served without a charset. Add a _headers rule.
  • check-dist: verify redirect #fragments exist, link-check sitemap.xml, and dedupe the color regexes shared with lint.ts.
  • /links: the spec lists a "website" channel that links.json omits. Confirm which is right.
  • Bump CI actions off Node 20 (checkout, setup-node, action-setup @v4).
  • Pin the DNS surface: a test that wrangler.jsonc routes are exactly www.jscebu.org as a Custom Domain, plus a CODEOWNERS entry. Non-interactive deploys override DNS for any Custom Domain listed.

Activity

  1. dorelljames
    added
    enhancementNew feature or request
    on Oct 28, 2026

  2. dorelljames commented on Oct 30, 2026

    @dorelljames
    dorelljames
    on Oct 30, 2026
    MemberAuthor

    Codex Workpad

    Phase: build, handed off · All nine fixes are implemented and validated; host handoff files are ready for PR #16. Sandbox blocks cleanup of two detached local Astro servers.

    Needs you

    Review PR #16, especially brand 0.1.1/template navigation and the www-only configuration guard with @dorelljames ownership.

    Checklist, checks and notes

    Dorells-MacBook-Pro-X.local:/Users/dorelljames/Projects/jscebu-symphony-workspaces/GH-9@4ad1ef2

    Plan

    • Read clean sync, rules, card, workpad and all issue/PR review channels.
    • Record approval and implement the approved plan, tests first.
    • Validate frozen install, tests, build and local HTTP behavior.
    • Review the whole branch and all untracked files; prepare host PR/commit files.
    • Stop detached Astro daemons: blocked by sandbox EPERM; details below.

    Acceptance criteria

    • Worker fallback requests canonical /404 and preserves body/headers with status 404.
    • Brand publishes extensionless template URLs, with version 0.1.1 and changelog; both rendered download-section template links navigate without download attributes. Cover iframe/link are canonical.
    • All twenty legacy redirect rows are pinned and pass local HTTP checks.
    • Astro and TypeScript checks run before website Vitest; production image inference and existing content/photo validation remain intact.
    • Tokens CSS has text/css; charset=utf-8 plus existing CORS.
    • check-dist validates redirect fragments and XML sitemap destinations, and shares color detection with brand lint.
    • Approved four-channel content matches the corrected v1 spec and exact content assertion.
    • All three selected CI actions declare node24; project Node 24 and pnpm 11.3.0 remain configured.
    • Structural JSONC guard pins only the existing www Custom Domain and rejects singular/plural route overrides anywhere else; CODEOWNERS covers Wrangler and itself. Actual routes are untouched.
    • Astro dev/preview template behavior checked separately and recorded.

    Checks

    • Red signals: /404.html handler path, 10-vs-20 redirect rows, missing CSS charset, old brand template URLs/version, and missing checker/helper exports failed their regressions before implementation.
    • Built-output regression: after metadata changed, build failed on exactly both template download attributes; fixing stable template identification made it pass.
    • pnpm install --frozen-lockfile: passed, already up to date.
    • pnpm test: passed, 170 tests in 15 files (112 brand, 58 website). Includes Astro diagnostics for 39 files: 0 errors, 0 warnings, 3 existing URL API deprecation hints; tsc passes.
    • pnpm build: passed; check-dist reports 15 required files, 6 pages, 20 redirects, brand parity and 30 source files without literals.
    • bash apps/website/scripts/url-matrix.sh http://127.0.0.1:8799: all 36 assertions pass, including both extensionless templates 200 without Location, CSS charset/CORS, all redirects and genuine 404.
    • Astro dev localhost:4391: /brand/ 200; both extensionless templates including the iframe URL 404, no redirects. Canonical iframe and template links without download confirmed in rendered HTML.
    • Astro preview localhost:4392: /brand/ and both extensionless templates 200, no redirects; same iframe/link checks pass.
    • bash -n apps/website/scripts/url-matrix.sh and git diff --check: pass.
    • Whole-branch git diff origin/main and untracked source review: no generated output, secrets, unrelated changes or Wrangler route edits.

    Feedback

    • Lead 1A/2A → keep Facebook, X, GitHub and Email; @dorelljames owns config/ownership review. Recorded in plan/spec and implemented.
    • Claude round 1 must-fix → replace suffix-based download identity and cover empty download attributes in fixtures plus real built output.
    • Claude round 1 suggestions → separate Workers/Astro HTTP evidence and singular/plural override mutation cases.
    • Claude round 2 suggestions → resolver implemented before metadata change; approved 0.1.1 stated in approval block; spec approval wording settled. Add-Website branch is inapplicable to 1A.

    Notes

    • Approval source: MEMBER dorelljames answer 1A 2A at issuecomment-6525522776, plus the card returned to Todo. Existing PR Resolve the deferred v1 review fixes #16 is open; clean sync started at 4ad1ef2.
    • Website scopes TypeScript 6.0.3 because @astrojs/check 0.9.12 supports TS 5/6 and Astro CLI rejects TS 7. Root TypeScript remains 7.0.2. Typed collection callbacks preserve real image metadata.
    • Fresh install repaired stale pnpm skipped-dependency state; no extra transitive workaround dependency was added. Local package store/log/registry state stays ignored.
    • Official manifests and release/migration notes verified through github_api: checkout v7.0.1, setup-node v7.1.0, pnpm/action-setup v6.1.0 each uses node24. Hosted CI on this implementation is not observed.
    • Known local limitation: Astro dev returns 404 for canonical extensionless public templates; Workers and Astro preview return 200 without redirects. The plan explicitly permits documenting this server difference.
    • Cleanup blocker: Workers stopped. Astro auto-detached PIDs 29334 (127.0.0.1:4391) and 29496 (127.0.0.1:4392). Ctrl-C ended CLI parents; sandbox EPERM denies kill and Astro's liveness probe, so its stop command reports none while both ports still answer. Host process lifecycle/cleanup outside this sandbox can clear it; no implementation or secret blocker remains.
    • Routes, DNS, deployment and branch protection remain unchanged. CODEOWNERS requests review without changing enforcement.
    • Host PR title/body and commit message are ready; commit message was the last local file write. Host will commit/push/update the existing PR after this run; no hosted CI, push, merge or deployment is claimed.
  3. dorelljames
    added
    codexSymphony: the Codex agent picks this card up
    on Oct 30, 2026

  4. dorelljames commented on Oct 30, 2026

    @dorelljames
    dorelljames
    on Oct 30, 2026
    MemberAuthor

    Codex Workpad

    Phase: plan, awaiting approval · The plan for all nine v1 review fixes is ready in PR #16. Claude's second review found nothing that must change.

    Needs you

    1. Should Website be one of the official channels on /links/? The same list feeds the home page's Join section.

      • A) No. Keep Facebook, X, GitHub and Email, and correct the v1 spec to match. (recommended)
      • B) Yes. Add "Website" last, linking to https://www.jscebu.org/.

    2. Who should own reviews of apps/website/wrangler.jsonc and .github/CODEOWNERS?

      • A) dorelljames (recommended: you already review the agents' PRs)
      • B) All five admins: dorelljames, harlekoy, arielpasilang, siegblink, jawogamay. Any one of them can approve.

    Reply here with your picks, like 1A 2B, then move the card to Todo. Moving it without a reply accepts the recommended options.

    Checklist, checks and notes

    1. A
    2. A
  5. dorelljames
    closed this as completedin #16on Oct 30, 2026
  6. dorelljames
    added a commit that references this issue on Oct 30, 2026

    Merge pull request #16 from javascriptcebu/symphony/gh-9

    09cd7ee
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    codexSymphony: the Codex agent picks this card upenhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions