fix: CSP Clerk-origin drift + Production Smoke failure paging (linejam-912, linejam-913) - #300
Closed
moomooskycow wants to merge 9 commits into
Closed
fix: CSP Clerk-origin drift + Production Smoke failure paging (linejam-912, linejam-913)#300moomooskycow wants to merge 9 commits into
moomooskycow wants to merge 9 commits into
Conversation
… list PR #291/#299 hand-listed Clerk domains in next.config.ts and missed the production custom domain (clerk.linejam.app), blocking auth site-wide for ~16h on 2026-07-04 while preview smoke stayed green (previews use the allowed dev Clerk domain). A hand-maintained list can drift from the live Clerk config again the next time the domain changes. Extract the existing pk_(test|live)_<base64url(host)> decoder (previously duplicated between bootstrap-convex-env.mjs and dagger-call.sh) into scripts/lib/clerk-domain.mjs as the one canonical implementation, and have next.config.ts derive its Clerk CSP sources from it at config-eval time. The static clerk.linejam.app assertion stays as a regression tripwire; new tests prove genuine derivation against a different custom domain and against a dev/preview-style key. Powder: linejam-912 Agent: linejam-overhaul Agent-Surface: Claude Code Agent-Model: anthropic/claude-fable-5 Agent-Task: linejam-912
2026-07-04 outage: Production Smoke failed hourly for ~15h before the operator found the outage by hand. The gate was working; nothing wired the red signal to a human or to BB triage. Add a linejam-production-smoke Canary TTL monitor (expected_every_ms=1h, grace_ms=30m, live at MON-28junwbo5mgv) and report every run's outcome to it via POST /api/v1/check-ins, reusing the existing NEXT_PUBLIC_CANARY_API_KEY/ NEXT_PUBLIC_CANARY_ENDPOINT secrets (no new secret provisioned): - success -> "ok" (Up; resolves any open incident) - failure, streak < 2 -> "alive" (Up; recorded, not escalated -- a single blip is an annotation) - failure, streak >= 2 -> "error" (Canary maps this directly to its Down health state, opening/holding a health_transition incident that BB triage and the bridge feed already consume) The consecutive-failure streak is computed from the workflow's own run history via the GitHub API (scripts/ops/count-consecutive-prod-smoke-failures.mjs), failing OPEN toward escalation rather than silence if that lookup errors. A GitHub step-summary annotation is written on every failure regardless of escalation. Live-verified end to end against the deployed monitor: first simulated failure stayed up/alive, second consecutive failure flipped to down and opened INC-bhg3g284flhp, and a simulated recovery run resolved it immediately (see docs/ops/canary-responder.md for the full transcript). Powder: linejam-913 Agent: linejam-overhaul Agent-Surface: Claude Code Agent-Model: anthropic/claude-fable-5 Agent-Task: linejam-913
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
PR SummaryMedium Risk Overview Auth / CSP (linejam-912): CSP Clerk allowlists are derived from Production smoke → Canary (linejam-913): CI / Convex (linejam-914): PR quality gates use full git history and run Releases (linejam-915/916): Landmark uses Onboarding / floor: Adds Reviewed by Cursor Bugbot for commit 65de13f. Configure here. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_6eeab58b-f9e0-4cf8-83c4-ad0896959dc7) |
📝 WalkthroughWalkthroughAdds production smoke escalation reporting, shared Clerk origin derivation, schema-migration guardrails, onboarding diagnostics, and a static release-store pipeline with matching workflow, documentation, test, and content updates. ChangesProduction Smoke Failure Escalation
Clerk Frontend Origin Derivation
Doctor and Schema-Migration Guardrails
Static Release Store and Release Generation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
…tion 2026-07-04 outage, part two: PR #298 removed convex/schema.ts fields and added their migration (dropLegacyModeColumns) in the same commit. Convex validates schema against live data at push time, so the contraction wedged every deploy -- including the unrelated P0 CSP hotfix production needed immediately -- until an operator ran the migration manually against production, outside the normal pipeline. Document the expand-migrate-contract sequence in docs/convex-migrations.md with this incident as the worked example, and add a CI gate (scripts/ci/check-schema-migration-sequencing.mjs, wired into the quality-gates job for pull requests) that fails any PR whose diff both removes a schema.ts field and adds a migrations.ts export. It's a plain git-diff heuristic rather than a Dagger container, since it needs git history/merge-base rather than the source tree. Its regression test replays the actual PR #298 diff (git show 684de32) and the CLI is separately live-verified to block against that same historical diff (exit 1, naming the exact removed fields and added migration). Annotated linejam-019 (the card #298 shipped under) with the sequencing lesson. Powder: linejam-914 Agent: linejam-overhaul Agent-Surface: Claude Code Agent-Model: anthropic/claude-fable-5 Agent-Task: linejam-914
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
scripts/ops/count-consecutive-prod-smoke-failures.mjs (1)
46-74: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winConsider adding a request timeout to the GitHub API call.
fetchImplhere has no timeout, unlike the Canary POST inreport-prod-smoke-status.mjs(which usesAbortSignal.timeout(5_000)). If the GitHub API hangs, this step blocks until the job's 15-minute timeout rather than failing fast into the existing catch-based fallback.♻️ Proposed fix
const response = await fetchImpl(url, { headers: { Authorization: `Bearer ${token}`, Accept: 'application/vnd.github+json', 'X-GitHub-Api-Version': '2022-11-28', }, + signal: AbortSignal.timeout(5_000), });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/ops/count-consecutive-prod-smoke-failures.mjs` around lines 46 - 74, The GitHub API request in fetchPriorRunConclusions has no timeout, so it can hang until the job times out; add a fast-fail timeout like the one used in report-prod-smoke-status.mjs. Update the fetchImpl call in fetchPriorRunConclusions to pass an AbortSignal timeout (for example via AbortSignal.timeout) and make sure the existing error handling still surfaces the fetch failure cleanly. Use the fetchPriorRunConclusions symbol to locate the request and keep the fallback behavior unchanged..github/workflows/prod-smoke.yml (1)
93-100: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPrefer env-var indirection for
${{ steps.smoke.outcome }}to avoid the flagged template-injection pattern.zizmor flags this expansion directly into the shell command.
steps.<id>.outcomeis a GitHub-controlled enum (success/failure/cancelled/skipped), so exploitability is low here, but routing it throughenv:avoids the pattern entirely and is the standard mitigation for this class of finding.🔒 Proposed fix
- name: Determine consecutive-failure streak id: streak if: always() env: GITHUB_TOKEN: ${{ github.token }} + SMOKE_OUTCOME: ${{ steps.smoke.outcome }} run: | - count="$(node scripts/ops/count-consecutive-prod-smoke-failures.mjs '${{ steps.smoke.outcome }}')" + count="$(node scripts/ops/count-consecutive-prod-smoke-failures.mjs "$SMOKE_OUTCOME")" echo "count=$count" >> "$GITHUB_OUTPUT"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/prod-smoke.yml around lines 93 - 100, The workflow step in Determine consecutive-failure streak is directly interpolating steps.smoke.outcome into the shell command, which triggers the template-injection warning. Move that GitHub expression into an env variable on the same step, then have the run script read from that env value instead of embedding the expression inline. Keep the change localized to the streak step in prod-smoke.yml and preserve the existing count invocation and GITHUB_OUTPUT write.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/prod-smoke.yml:
- Around line 49-61: The smoke job output block uses a fixed multiline
delimiter, which can be collided with by captured stderr and corrupt the GitHub
Actions output parsing. Update the `id: smoke` step in the workflow to generate
a unique per-run delimiter before writing to `GITHUB_OUTPUT`, and use that
dynamic marker for the `detail` multiline value instead of the hardcoded
`SMOKE_DETAIL_EOF`.
In `@docs/ops/canary-responder.md`:
- Line 117: The phrase in the canary responder documentation should use the
standard compound modifier form; update the sentence containing “end to end” in
the docs text to “end-to-end.” Keep the surrounding wording unchanged and make
the edit in the same prose block so the language is consistent throughout the
canary-responder.md content.
In `@scripts/lib/clerk-domain.mjs`:
- Around line 25-26: The domain extraction in clerk-domain.mjs is using
split('_').at(-1), which can lose part of the base64url payload when the encoded
segment contains underscores. Update the parsing logic in the key-handling flow
so the function that derives encodedDomain strips the pk_(test|live)_ prefix
directly instead of splitting on every underscore, then continue decoding the
remaining payload as before.
In `@scripts/ops/report-prod-smoke-status.mjs`:
- Around line 124-151: The `run` function in `report-prod-smoke-status.mjs`
currently parses `LINEJAM_SMOKE_CONSECUTIVE_FAILURES` with a default of 0, which
can cause missing or invalid streak values to be treated as non-escalated.
Update the `consecutiveFailures` handling so empty, non-numeric, or otherwise
invalid values fall back to `ESCALATION_THRESHOLD` (or validate before parsing)
before calling `planCheckIn`, and keep the `context` object aligned with the
sanitized streak value.
---
Nitpick comments:
In @.github/workflows/prod-smoke.yml:
- Around line 93-100: The workflow step in Determine consecutive-failure streak
is directly interpolating steps.smoke.outcome into the shell command, which
triggers the template-injection warning. Move that GitHub expression into an env
variable on the same step, then have the run script read from that env value
instead of embedding the expression inline. Keep the change localized to the
streak step in prod-smoke.yml and preserve the existing count invocation and
GITHUB_OUTPUT write.
In `@scripts/ops/count-consecutive-prod-smoke-failures.mjs`:
- Around line 46-74: The GitHub API request in fetchPriorRunConclusions has no
timeout, so it can hang until the job times out; add a fast-fail timeout like
the one used in report-prod-smoke-status.mjs. Update the fetchImpl call in
fetchPriorRunConclusions to pass an AbortSignal timeout (for example via
AbortSignal.timeout) and make sure the existing error handling still surfaces
the fetch failure cleanly. Use the fetchPriorRunConclusions symbol to locate the
request and keep the fallback behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2d406cdf-c517-49e6-9a09-55bf734faecb
📒 Files selected for processing (10)
.github/workflows/prod-smoke.ymldocs/ops/canary-responder.mdnext.config.tsscripts/ci/bootstrap-convex-env.mjsscripts/lib/clerk-domain.mjsscripts/ops/count-consecutive-prod-smoke-failures.mjsscripts/ops/report-prod-smoke-status.mjstests/next-config.test.tstests/scripts/count-consecutive-prod-smoke-failures.test.tstests/scripts/report-prod-smoke-status.test.ts
Comment on lines
+49
to
+61
| id: smoke | ||
| run: | | ||
| mkdir -p "$RUNNER_TEMP/linejam-smoke" | ||
| set +e | ||
| pnpm canary:smoke > "$RUNNER_TEMP/linejam-smoke/stdout.log" 2> "$RUNNER_TEMP/linejam-smoke/stderr.log" | ||
| code=$? | ||
| cat "$RUNNER_TEMP/linejam-smoke/stdout.log" | ||
| cat "$RUNNER_TEMP/linejam-smoke/stderr.log" >&2 | ||
| { | ||
| echo "detail<<SMOKE_DETAIL_EOF" | ||
| tail -c 2000 "$RUNNER_TEMP/linejam-smoke/stderr.log" | ||
| echo "SMOKE_DETAIL_EOF" | ||
| } >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the workflow around the referenced lines
file=".github/workflows/prod-smoke.yml"
wc -l "$file"
sed -n '1,160p' "$file"
echo
echo "---- search for LINEJAM_SMOKE_FAILURE_DETAIL and steps.smoke.outputs.detail ----"
rg -n "LINEJAM_SMOKE_FAILURE_DETAIL|steps\.smoke\.outputs\.detail|SMOKE_DETAIL_EOF|GITHUB_OUTPUT" .github/workflows . -g '*.yml' -g '*.yaml' -g '*.md'Repository: misty-step/linejam
Length of output: 5521
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect GitHub Actions docs locally? (repo files only)
rg -n "multiline|GITHUB_OUTPUT|delimiter|random" .github README.md docs -g '*.md' -g '*.yml' -g '*.yaml' || trueRepository: misty-step/linejam
Length of output: 837
Use a unique GITHUB_OUTPUT delimiter here. A fixed SMOKE_DETAIL_EOF marker can appear in the captured stderr and truncate the multiline value, letting extra text be parsed as new outputs. Generate a per-run delimiter instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/prod-smoke.yml around lines 49 - 61, The smoke job output
block uses a fixed multiline delimiter, which can be collided with by captured
stderr and corrupt the GitHub Actions output parsing. Update the `id: smoke`
step in the workflow to generate a unique per-run delimiter before writing to
`GITHUB_OUTPUT`, and use that dynamic marker for the `detail` multiline value
instead of the hardcoded `SMOKE_DETAIL_EOF`.
|
|
||
| Reuses the existing `NEXT_PUBLIC_CANARY_API_KEY`/`NEXT_PUBLIC_CANARY_ENDPOINT` repository secrets (already ingest-scoped for client-side error reporting); no new secret was provisioned. | ||
|
|
||
| Live-verified end to end against the deployed monitor: a first simulated failure stayed `up`/`alive`; a second consecutive simulated failure flipped the monitor to `down` and opened `INC-bhg3g284flhp` (`signal_type: health_transition`); a simulated recovery run immediately resolved it. Query current state any time with: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Minor: hyphenate "end-to-end."
LanguageTool flags this as a compound-modifier hyphenation opportunity.
✏️ Proposed fix
-Live-verified end to end against the deployed monitor: a first simulated failure stayed `up`/`alive`; a second consecutive simulated failure flipped the monitor to `down` and opened `INC-bhg3g284flhp` (`signal_type: health_transition`); a simulated recovery run immediately resolved it. Query current state any time with:
+Live-verified end-to-end against the deployed monitor: a first simulated failure stayed `up`/`alive`; a second consecutive simulated failure flipped the monitor to `down` and opened `INC-bhg3g284flhp` (`signal_type: health_transition`); a simulated recovery run immediately resolved it. Query current state any time with:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Live-verified end to end against the deployed monitor: a first simulated failure stayed `up`/`alive`; a second consecutive simulated failure flipped the monitor to `down` and opened `INC-bhg3g284flhp` (`signal_type: health_transition`); a simulated recovery run immediately resolved it. Query current state any time with: | |
| Live-verified end-to-end against the deployed monitor: a first simulated failure stayed `up`/`alive`; a second consecutive simulated failure flipped the monitor to `down` and opened `INC-bhg3g284flhp` (`signal_type: health_transition`); a simulated recovery run immediately resolved it. Query current state any time with: |
🧰 Tools
🪛 LanguageTool
[grammar] ~117-~117: Use a hyphen to join words.
Context: ...cret was provisioned. Live-verified end to end against the deployed monitor: a firs...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/ops/canary-responder.md` at line 117, The phrase in the canary responder
documentation should use the standard compound modifier form; update the
sentence containing “end to end” in the docs text to “end-to-end.” Keep the
surrounding wording unchanged and make the edit in the same prose block so the
language is consistent throughout the canary-responder.md content.
Source: Linters/SAST tools
Comment on lines
+25
to
+26
| const encodedDomain = key.split('_').at(-1); | ||
| if (!encodedDomain) return ''; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
node -e '
const hosts = ["clerk.linejam.app","auth.example-tenant.com","great-moose-1.clerk.accounts.dev"];
// brute-force a few synthetic hosts to prove _ is reachable in base64url output
for (let i=0;i<5000;i++){ hosts.push("clerk-"+i+".linejam.app"); }
let hit=null;
for (const h of hosts){
const enc = Buffer.from(h+"$").toString("base64url");
if (enc.includes("_")){ hit={h,enc}; break; }
}
console.log("example host whose key segment contains _ :", hit);
if (hit){
const key = "pk_live_"+hit.enc;
const tail = key.split("_").at(-1);
console.log("split-based decode:", Buffer.from(tail,"base64url").toString("utf8"));
console.log("prefix-strip decode:", Buffer.from(hit.enc,"base64url").toString("utf8"));
}
'Repository: misty-step/linejam
Length of output: 205
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== scripts/lib/clerk-domain.mjs ==\n'
sed -n '1,220p' scripts/lib/clerk-domain.mjs
printf '\n== related references ==\n'
rg -n "deriveClerkFrontendOrigin|clerk-domain|frontendApiHost|Convex JWT issuer|bootstrap-convex-env|next.config" scripts . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' || true
printf '\n== tests around clerk domain ==\n'
rg -n "clerk.*domain|frontendApiHost|base64url|pk_(test|live)_" scripts test tests . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' || trueRepository: misty-step/linejam
Length of output: 14127
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== file inventory ==\n'
git ls-files | rg '^scripts/lib/clerk-domain\.mjs$|^scripts/.*(bootstrap-convex-env|next\.config|clerk).*|(^|/)(test|tests)/.*clerk|.*clerk.*test.*'
printf '\n== source around the target ==\n'
nl -ba scripts/lib/clerk-domain.mjs | sed -n '1,120p'Repository: misty-step/linejam
Length of output: 473
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and nearby callers/tests without executing repository code.
printf '\n== scripts/lib/clerk-domain.mjs ==\n'
nl -ba scripts/lib/clerk-domain.mjs | sed -n '1,200p'
printf '\n== references to the helper and related derivation ==\n'
rg -n "deriveClerkFrontendOrigin|clerk-domain|frontendApiHost|base64url|pk_(test|live)_" scripts . \
-g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' || true
printf '\n== test files referencing clerk domain ==\n'
rg -n "clerk.*domain|frontendApiHost|base64url|pk_(test|live)_" tests test scripts \
-g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' || trueRepository: misty-step/linejam
Length of output: 232
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
import base64
import itertools
import string
alphabet = string.ascii_lowercase + string.digits + "-."
# Search short, syntactically plausible host labels first.
def enc(s: str) -> str:
return base64.urlsafe_b64encode(s.encode()).decode().rstrip("=")
for n in range(1, 7):
for s in itertools.product(alphabet, repeat=n):
host = "".join(s)
e = enc(host)
if "_" in e:
print("found_host=", host)
print("encoded=", e)
key = "pk_live_" + e
tail = key.split("_")[-1]
print("split_tail=", tail)
print("split_decoded=", base64.urlsafe_b64decode(tail + "==").decode("utf-8", "replace"))
print("full_decoded=", base64.urlsafe_b64decode(e + "==").decode("utf-8", "replace"))
raise SystemExit(0)
print("no host found in search space")
PYRepository: misty-step/linejam
Length of output: 148
split('_').at(-1) can truncate the encoded segment
pk_(test|live)_<base64url(...)> keys can carry _ inside the base64url payload, so splitting on every underscore can drop everything before the last _ and decode the wrong host. Strip the pk_(test|live)_ prefix instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/lib/clerk-domain.mjs` around lines 25 - 26, The domain extraction in
clerk-domain.mjs is using split('_').at(-1), which can lose part of the
base64url payload when the encoded segment contains underscores. Update the
parsing logic in the key-handling flow so the function that derives
encodedDomain strips the pk_(test|live)_ prefix directly instead of splitting on
every underscore, then continue decoding the remaining payload as before.
Comment on lines
+124
to
+151
| export async function run({ | ||
| outcome = process.env.LINEJAM_SMOKE_OUTCOME, | ||
| consecutiveFailures = Number.parseInt( | ||
| process.env.LINEJAM_SMOKE_CONSECUTIVE_FAILURES || '0', | ||
| 10 | ||
| ), | ||
| runUrl = process.env.LINEJAM_SMOKE_RUN_URL, | ||
| failureDetail = process.env.LINEJAM_SMOKE_FAILURE_DETAIL, | ||
| env = process.env, | ||
| fetchImpl = globalThis.fetch, | ||
| } = {}) { | ||
| if (outcome !== 'success' && outcome !== 'failure') { | ||
| throw new Error( | ||
| `LINEJAM_SMOKE_OUTCOME must be "success" or "failure", got: ${outcome}` | ||
| ); | ||
| } | ||
|
|
||
| const plan = planCheckIn({ outcome, consecutiveFailures }); | ||
| const context = { | ||
| consecutiveFailures, | ||
| ...(runUrl ? { runUrl } : {}), | ||
| ...(outcome === 'failure' && failureDetail?.trim() | ||
| ? { failureDetail: truncate(failureDetail.trim(), 1_000) } | ||
| : {}), | ||
| }; | ||
|
|
||
| return sendCheckIn({ ...plan, context, env, fetchImpl }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== scripts/ops/report-prod-smoke-status.mjs ==\n'
wc -l scripts/ops/report-prod-smoke-status.mjs
sed -n '1,220p' scripts/ops/report-prod-smoke-status.mjs
printf '\n== scripts/ops/count-consecutive-prod-smoke-failures.mjs ==\n'
wc -l scripts/ops/count-consecutive-prod-smoke-failures.mjs
sed -n '1,220p' scripts/ops/count-consecutive-prod-smoke-failures.mjs
printf '\n== .github/workflows/prod-smoke.yml ==\n'
wc -l .github/workflows/prod-smoke.yml
sed -n '1,220p' .github/workflows/prod-smoke.ymlRepository: misty-step/linejam
Length of output: 12956
Treat missing or invalid streak counts as escalated.
LINEJAM_SMOKE_CONSECUTIVE_FAILURES can be empty or non-numeric if the streak step fails, and this path falls back to 0/NaN, so a failed smoke run is reported as alive instead of error. The summary step also breaks on a missing count. Fallback to ESCALATION_THRESHOLD (or validate the env before parsing) instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/ops/report-prod-smoke-status.mjs` around lines 124 - 151, The `run`
function in `report-prod-smoke-status.mjs` currently parses
`LINEJAM_SMOKE_CONSECUTIVE_FAILURES` with a default of 0, which can cause
missing or invalid streak values to be treated as non-escalated. Update the
`consecutiveFailures` handling so empty, non-numeric, or otherwise invalid
values fall back to `ESCALATION_THRESHOLD` (or validate before parsing) before
calling `planCheckIn`, and keep the `context` object aligned with the sanitized
streak value.
…-brain
Two entangled release-pipeline failures from 2026-07-04, fixed together
because linejam-915's fix depends on linejam-916's:
linejam-916 (why Release stopped working): the Release workflow used
GH_RELEASE_TOKEN, a personal gh-CLI OAuth token pasted into a repo secret,
which started 403ing ("You do not have permission to create labels on this
repository") the moment linejam went public (PR #296) -- verified against
the actual failed-run logs (x-oauth-client-id matches the GitHub CLI's own
OAuth App). master has no branch protection and no rulesets (verified via
the API), so nothing here needs bypassing: switch to the job's own
GITHUB_TOKEN, whose already-declared contents/issues/pull-requests write
permissions are exactly what semantic-release + @semantic-release/github
need. No new secret provisioned -- GITHUB_TOKEN is ephemeral per run.
Verified structurally (no branch protection to fight) and functionally (a
throwaway label create/delete against this exact repo endpoint with an
equivalently-scoped token succeeded); full end-to-end proof needs the next
actual merge to master, per the card's own acceptance criterion.
linejam-915 (the split-brain itself): app/releases/page.tsx read
content/releases/, untouched since v0.1.0 (Jan 2026), while Landmark kept a
second store (docs/releases/feed.xml RSS) current through v1.15.1. Fix:
release.yml now writes content/releases/ on every release
(scripts/release/write-release-from-git.mjs), deriving the technical
changelog deterministically from git history (no LLM in that path) and
using Landmark's own synthesized notes for prose (the same synthesis
already producing every GH Release body and RSS entry in this repo --
not a new trust surface; its known fabrication risk, landmark-907, is
tracked upstream). manifest.json is regenerated from the version
directories actually on disk, so it can't itself drift.
scripts/release/backfill-static-releases.mjs backfilled all 21 historical
v1.x tags (deterministic changelog.json only, no fabricated prose).
tests/scripts/release-manifest-version.test.ts gates manifest.json against
package.json's version on every test run.
Live-verified end to end via a real `next build` (force-static
prerendering, the actual pipeline app/releases/page.tsx runs through): the
generated HTML shows "Version 1.15.1" first, followed by all 22 versions
in descending order down to 0.1.0, with real (non-fabricated) technical
detail entries pulled from actual commits.
Powder: linejam-915, linejam-916
Agent: linejam-overhaul
Agent-Surface: Claude Code
Agent-Model: anthropic/claude-fable-5
Agent-Task: linejam-915,linejam-916
…on fixture CI's Test & Build job runs unit tests inside a Dagger container that copies the working tree but not .git (a hermetic source-tree snapshot, by design). The PR #298 regression fixture called `git show 684de32` at test time, which failed there with "fatal: not a git repository" even though it passed locally and in the plain-runner quality-gates job. Embed the two diffs (schema.ts, migrations.ts) as literal fixture strings instead -- they are frozen historical data, not something that needs to be re-derived from a live repository at test time. Found live: PR #300's Test & Build check failed on push; root-caused from the actual job log rather than guessed. Agent: linejam-overhaul Agent-Surface: Claude Code Agent-Model: anthropic/claude-fable-5 Agent-Task: linejam-914
linejam-909: scripts/setup.sh installs dependencies and writes placeholder .env.local, then prints "setup complete" -- an installed-but-dead workspace looked identical to a working one (application-floor item 9, the exact Counterspell case study: onboarding must end at verified-live, not installed). pnpm doctor checks required env, that the Convex URL looks like a real deployment, that the Clerk publishable key actually decodes to a Frontend API host (not just "is present"), that Canary config isn't the .env.example placeholder, and probes both Canary and the running app's /api/health -- warning rather than failing when the app or Canary aren't reachable yet (expected before `pnpm dev`), but failing hard on missing or placeholder secrets. Wired into README and scripts/setup.sh's completion message. Writing doctor's tests surfaced a real latent bug in scripts/lib/clerk-domain.mjs (linejam-912): base64url decoding rarely throws on arbitrary input, so a malformed NEXT_PUBLIC_CLERK_PUBLISHABLE_KEY silently decoded to control-character garbage and reported success. Hardened it to reject anything that doesn't look like a real hostname. Also deleted scripts/generate-releases.ts + lib/releases/parser.ts (dead code discovered while building linejam-915): a pre-existing, untested, never-wired manual generator that called OpenRouter directly with a bare prompt and no fallback protection -- the exact fabrication risk landmark-907 exists to fix, and a second writer to content/releases/ that could have reintroduced this card's split-brain the moment anyone ran it. Live evidence (both required by linejam-909's proof plan): env -i node scripts/doctor.mjs # 4 failing checks, exit 1 (source real .env.local) node scripts/doctor.mjs # all pass/warn, exit 0 Powder: linejam-909 Agent: linejam-overhaul Agent-Surface: Claude Code Agent-Model: anthropic/claude-fable-5 Agent-Task: linejam-909
linejam-910 (application-floor real-engine tier b): existing E2E specs (game-flow, auth, favorites, room-chrome-layout, ...) cover behavioral golden paths well, but nothing smoke-loaded every major page asserting zero console/pageerror events at both a desktop and a mobile viewport -- the exact gap the Sanctum-artifacts case study (a raw-string escaping bug that broke all client JS while 84 substring-matching tests stayed green) names as the floor's reason for existing. Covers '/', '/host', '/join', '/releases' (this overhaul's own linejam-915 fix), the poem/recap not-found shapes, and the /me/* protected-route redirect, each at 1440x900 and iPhone 14 (~390px), asserting a visible landmark and zero console.error/pageerror events. No new CI wiring needed: Playwright auto-discovers every spec under tests/e2e/, so it runs inside the existing `pnpm test:e2e` invocation the `e2e` job already calls via Dagger on every non-draft PR. Live-verified: PR #300's "E2E Mirror" check passed with this spec included (https://github.com/misty-step/linejam/actions/runs/28724393292). Powder: linejam-910 Agent: linejam-overhaul Agent-Surface: Claude Code Agent-Model: anthropic/claude-fable-5 Agent-Task: linejam-910
linejam-911: coverage was measured at 90.9%/85.94%/92.08%/92.32% (stmts/branch/fn/lines) against a static 85% floor -- real headroom the gate never captured, so a genuine regression could land undetected until it dropped a full 5-6 points. Evidence-first, not a blind bump: - Identified app/join/page.tsx as the lowest-covered major page in the repo (48%/37%, zero tests) via `pnpm test:ci`'s per-file report -- exactly the highest-risk gap the card asks to close. Added tests/app/join-page.test.tsx (loading/auth-error/prefill/disabled-submit/ success/whitespace-strip/failure-path), taking it to 97%/89%. - Extracted the untested CLI-argv-parsing/notes-reading logic out of scripts/release/write-release-from-git.mjs (28% covered, all of it my own new code from this session) into parseCliArgs/readNotesFile/ defaultExec and tested them directly (65% covered now); did the same for scripts/ci/check-schema-migration-sequencing.mjs's message formatting (formatViolationMessage). - Re-measured (91.44%/86.32%/92.75%/92.9%) and set thresholds a few points below actual so the gate has real headroom against normal churn without being able to silently regress back toward 85%: lines 90%, functions 90%, branches 84%, statements 89%. `pnpm test:ci` verified green (exit 0) against the new thresholds. - Documented in docs/testing.md, including the known-weak modules NOT touched this pass (convex/ai.ts, personas.ts, errors.ts, RoomChrome.tsx, the auth callback page) so they aren't lost as the next ratchet target. No existing gate weakened; every number moved up. Powder: linejam-911 Agent: linejam-overhaul Agent-Surface: Claude Code Agent-Model: anthropic/claude-fable-5 Agent-Task: linejam-911
Fresh-context critic review (per the lane bar's required pre-done pass)
found a real bug: checkAppHealth defaulted to http://localhost:3333, but
`pnpm dev` serves Next.js on :3000 (README) -- :3333 is deliberately
reserved for Playwright E2E specifically to avoid colliding with a running
dev server (playwright.config.ts). Every doctor.test.ts case mocked
fetchImpl directly, so nothing ever exercised the real default URL, and
following doctor's own printed instruction ("start it with `pnpm dev` and
re-run `pnpm doctor`") always produced a false "no app running" warning --
defeating linejam-909's entire stated purpose.
Live-verified the fix: started a real `next dev --turbopack -p 3000`,
confirmed :3000/api/health responds (200) while :3333 has nothing
listening, then ran `node scripts/doctor.mjs` against the real env and
got "app health: ok (http://localhost:3000/api/health)".
Also documented a second critic finding as a known limitation rather than
a fix: check-schema-migration-sequencing.mjs (linejam-914) only matches
`+` lines, so a migration pre-scaffolded in an earlier PR and filled in
alongside a schema contraction in a later PR would not trip it. Recorded
in docs/convex-migrations.md rather than over-building the heuristic --
it still catches the exact 2026-07-04 failure shape, which is its job.
Agent: linejam-overhaul
Agent-Surface: Claude Code
Agent-Model: anthropic/claude-fable-5
Agent-Task: linejam-909
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_7986c372-92d3-43ea-9177-a635828d4e21) |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 39-45: The Checkout code step in the workflow should disable
credential persistence because this job does not push. Update the
actions/checkout usage in the checkout step to set persist-credentials to false
alongside the existing fetch-depth setting, so the GITHUB_TOKEN is not left in
local git config and the zizmor artipacked warning is resolved.
In `@scripts/release/backfill-static-releases.mjs`:
- Around line 21-34: The version sorting logic in listVersionTags duplicates
compareVersions from static-release-store.mjs, so update listVersionTags to
reuse the imported compareVersions helper instead of reimplementing the
comparator. Keep the tag listing/filtering flow the same, but replace the inline
sort callback with compareVersions so backfill-static-releases.mjs stays
consistent with the shared version ordering logic.
In `@tests/app/join-page.test.tsx`:
- Around line 41-51: The test is over-mocking internal `@/lib/*` utilities
(`useUser`, `trackGameJoined`, `captureError`) instead of only mocking external
boundaries. Update `join-page.test.tsx` to let the real `@/lib/auth`,
`@/lib/analytics`, and `@/lib/error` implementations run, and keep mocks only
for true nondeterministic/system dependencies like `next/navigation` and
`convex/react`; use the real `errorToFeedback`/`captureError` path to exercise
the join flow end to end.
In `@tests/e2e/major-page-smoke.spec.ts`:
- Around line 16-20: The skip guard in the major-page smoke test is too strict
because it only checks GUEST_TOKEN_SECRET, which still skips remote runs even
when E2E_BASE_URL is provided. Update the condition around test.skip in
major-page-smoke.spec.ts to reflect the intended behavior: either skip only when
both GUEST_TOKEN_SECRET and E2E_BASE_URL are missing, or narrow the skip message
so it matches the current guard. Use the existing missingGuestTokenSecret setup
as the place to adjust the logic.
- Around line 105-106: The smoke test in `assertVisible`/page navigation is
using `page.goto(..., { waitUntil: 'networkidle' })`, which is brittle for pages
with ongoing background requests. Update the navigation call in
`tests/e2e/major-page-smoke.spec.ts` to use `domcontentloaded` instead, and rely
on `assertVisible(page)` to verify the page is ready.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 734dc300-75eb-4bcf-8096-eea8af95aeac
📒 Files selected for processing (51)
.github/workflows/ci.yml.github/workflows/release.ymlAGENTS.mdREADME.mdcontent/releases/manifest.jsoncontent/releases/v1.0.0/changelog.jsoncontent/releases/v1.1.0/changelog.jsoncontent/releases/v1.1.1/changelog.jsoncontent/releases/v1.1.2/changelog.jsoncontent/releases/v1.1.3/changelog.jsoncontent/releases/v1.10.0/changelog.jsoncontent/releases/v1.11.0/changelog.jsoncontent/releases/v1.12.0/changelog.jsoncontent/releases/v1.13.0/changelog.jsoncontent/releases/v1.14.0/changelog.jsoncontent/releases/v1.15.0/changelog.jsoncontent/releases/v1.15.1/changelog.jsoncontent/releases/v1.2.0/changelog.jsoncontent/releases/v1.3.0/changelog.jsoncontent/releases/v1.4.0/changelog.jsoncontent/releases/v1.5.0/changelog.jsoncontent/releases/v1.6.0/changelog.jsoncontent/releases/v1.7.0/changelog.jsoncontent/releases/v1.8.0/changelog.jsoncontent/releases/v1.9.0/changelog.jsoncontent/releases/v1.9.1/changelog.jsondocs/convex-migrations.mddocs/releases-static-store.mddocs/testing.mdlib/releases/index.tslib/releases/parser.tspackage.jsonscripts/ci/check-schema-migration-sequencing.mjsscripts/doctor.mjsscripts/generate-releases.tsscripts/lib/clerk-domain.mjsscripts/release/backfill-static-releases.mjsscripts/release/conventional-commits.mjsscripts/release/static-release-store.mjsscripts/release/write-release-from-git.mjsscripts/setup.shtests/app/join-page.test.tsxtests/e2e/major-page-smoke.spec.tstests/scripts/check-schema-migration-sequencing.test.tstests/scripts/clerk-domain.test.tstests/scripts/conventional-commits.test.tstests/scripts/doctor.test.tstests/scripts/release-manifest-version.test.tstests/scripts/static-release-store.test.tstests/scripts/write-release-from-git.test.tsvitest.config.ts
💤 Files with no reviewable changes (2)
- lib/releases/parser.ts
- scripts/generate-releases.ts
✅ Files skipped from review due to trivial changes (25)
- content/releases/v1.12.0/changelog.json
- content/releases/v1.10.0/changelog.json
- content/releases/v1.4.0/changelog.json
- content/releases/v1.14.0/changelog.json
- content/releases/v1.11.0/changelog.json
- content/releases/v1.7.0/changelog.json
- content/releases/v1.1.1/changelog.json
- content/releases/v1.8.0/changelog.json
- content/releases/v1.5.0/changelog.json
- content/releases/v1.13.0/changelog.json
- content/releases/v1.15.0/changelog.json
- content/releases/v1.3.0/changelog.json
- content/releases/v1.9.0/changelog.json
- content/releases/v1.6.0/changelog.json
- content/releases/v1.15.1/changelog.json
- content/releases/manifest.json
- content/releases/v1.2.0/changelog.json
- content/releases/v1.1.2/changelog.json
- docs/testing.md
- content/releases/v1.9.1/changelog.json
- content/releases/v1.0.0/changelog.json
- content/releases/v1.1.3/changelog.json
- content/releases/v1.1.0/changelog.json
- docs/releases-static-store.md
- README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/lib/clerk-domain.mjs
Comment on lines
39
to
+45
| - name: Checkout code | ||
| uses: actions/checkout@v7 | ||
| with: | ||
| # linejam-914: the schema/migration sequencing check below needs | ||
| # full history to resolve a merge-base against the PR's base ref. | ||
| # This repo is small (a few hundred commits); a full fetch is cheap. | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Set persist-credentials: false on the checkout step.
This job never pushes; persisting the GITHUB_TOKEN in the local git config is unnecessary residual credential exposure. zizmor flags this as artipacked.
🔒 Proposed fix
- name: Checkout code
uses: actions/checkout@v7
with:
# linejam-914: the schema/migration sequencing check below needs
# full history to resolve a merge-base against the PR's base ref.
# This repo is small (a few hundred commits); a full fetch is cheap.
fetch-depth: 0
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Checkout code | |
| uses: actions/checkout@v7 | |
| with: | |
| # linejam-914: the schema/migration sequencing check below needs | |
| # full history to resolve a merge-base against the PR's base ref. | |
| # This repo is small (a few hundred commits); a full fetch is cheap. | |
| fetch-depth: 0 | |
| - name: Checkout code | |
| uses: actions/checkout@v7 | |
| with: | |
| # linejam-914: the schema/migration sequencing check below needs | |
| # full history to resolve a merge-base against the PR's base ref. | |
| # This repo is small (a few hundred commits); a full fetch is cheap. | |
| fetch-depth: 0 | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 39-45: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml around lines 39 - 45, The Checkout code step in the
workflow should disable credential persistence because this job does not push.
Update the actions/checkout usage in the checkout step to set
persist-credentials to false alongside the existing fetch-depth setting, so the
GITHUB_TOKEN is not left in local git config and the zizmor artipacked warning
is resolved.
Source: Linters/SAST tools
Comment on lines
+21
to
+34
| function listVersionTags() { | ||
| return exec('git', ['tag', '--list', 'v1.*']) | ||
| .split('\n') | ||
| .filter(Boolean) | ||
| .sort((a, b) => { | ||
| const pa = a.replace(/^v/, '').split('.').map(Number); | ||
| const pb = b.replace(/^v/, '').split('.').map(Number); | ||
| for (let i = 0; i < Math.max(pa.length, pb.length); i += 1) { | ||
| const diff = (pa[i] ?? 0) - (pb[i] ?? 0); | ||
| if (diff !== 0) return diff; | ||
| } | ||
| return 0; | ||
| }); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Reuse compareVersions instead of duplicating the sort logic.
This sort comparator (Lines 25-33) is identical to compareVersions already exported from static-release-store.mjs, which this file already imports from. Duplicating it risks the two diverging if one gets updated later.
♻️ Proposed fix
import { writeReleaseFromGit } from './write-release-from-git.mjs';
-import { regenerateManifest } from './static-release-store.mjs';
+import { compareVersions, regenerateManifest } from './static-release-store.mjs';
function exec(command, args) {
return execFileSync(command, args, { encoding: 'utf8' }).trim();
}
function listVersionTags() {
return exec('git', ['tag', '--list', 'v1.*'])
.split('\n')
.filter(Boolean)
- .sort((a, b) => {
- const pa = a.replace(/^v/, '').split('.').map(Number);
- const pb = b.replace(/^v/, '').split('.').map(Number);
- for (let i = 0; i < Math.max(pa.length, pb.length); i += 1) {
- const diff = (pa[i] ?? 0) - (pb[i] ?? 0);
- if (diff !== 0) return diff;
- }
- return 0;
- });
+ .sort((a, b) =>
+ compareVersions(a.replace(/^v/, ''), b.replace(/^v/, ''))
+ );
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function listVersionTags() { | |
| return exec('git', ['tag', '--list', 'v1.*']) | |
| .split('\n') | |
| .filter(Boolean) | |
| .sort((a, b) => { | |
| const pa = a.replace(/^v/, '').split('.').map(Number); | |
| const pb = b.replace(/^v/, '').split('.').map(Number); | |
| for (let i = 0; i < Math.max(pa.length, pb.length); i += 1) { | |
| const diff = (pa[i] ?? 0) - (pb[i] ?? 0); | |
| if (diff !== 0) return diff; | |
| } | |
| return 0; | |
| }); | |
| } | |
| function listVersionTags() { | |
| return exec('git', ['tag', '--list', 'v1.*']) | |
| .split('\n') | |
| .filter(Boolean) | |
| .sort((a, b) => | |
| compareVersions(a.replace(/^v/, ''), b.replace(/^v/, '')) | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/release/backfill-static-releases.mjs` around lines 21 - 34, The
version sorting logic in listVersionTags duplicates compareVersions from
static-release-store.mjs, so update listVersionTags to reuse the imported
compareVersions helper instead of reimplementing the comparator. Keep the tag
listing/filtering flow the same, but replace the inline sort callback with
compareVersions so backfill-static-releases.mjs stays consistent with the shared
version ordering logic.
Comment on lines
+41
to
+51
| vi.mock('@/lib/auth', () => ({ | ||
| useUser: () => mockUseUserReturn, | ||
| })); | ||
|
|
||
| vi.mock('@/lib/analytics', () => ({ | ||
| trackGameJoined: (...args: unknown[]) => mockTrackGameJoined(...args), | ||
| })); | ||
|
|
||
| vi.mock('@/lib/error', () => ({ | ||
| captureError: (...args: unknown[]) => mockCaptureError(...args), | ||
| })); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Internal @/lib/* modules are mocked, against tests guideline.
@/lib/auth, @/lib/analytics, and @/lib/error are internal domain utilities (per the lib/** guideline listing auth and error capture there), not system boundaries or sources of nondeterminism. The tests guideline says to mock only system boundaries/nondeterminism and not internal @/ modules or internal utilities. Consider letting these run for real (e.g. exercising real errorToFeedback/captureError logic) and only mocking true boundaries like next/navigation and convex/react, which are already handled correctly here.
Reasoning
Mocking internal utilities like captureError/trackGameJoined hides regressions in those modules and reduces confidence that the join flow's error/analytics wiring actually works end-to-end.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/app/join-page.test.tsx` around lines 41 - 51, The test is over-mocking
internal `@/lib/*` utilities (`useUser`, `trackGameJoined`, `captureError`)
instead of only mocking external boundaries. Update `join-page.test.tsx` to let
the real `@/lib/auth`, `@/lib/analytics`, and `@/lib/error` implementations run,
and keep mocks only for true nondeterministic/system dependencies like
`next/navigation` and `convex/react`; use the real
`errorToFeedback`/`captureError` path to exercise the join flow end to end.
Source: Path instructions
Comment on lines
+16
to
+20
| const missingGuestTokenSecret = !process.env.GUEST_TOKEN_SECRET; | ||
| test.skip( | ||
| missingGuestTokenSecret, | ||
| 'Set GUEST_TOKEN_SECRET for local E2E, or E2E_BASE_URL for a remote target' | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- file excerpt ---\n'
cat -n tests/e2e/major-page-smoke.spec.ts | sed -n '1,120p'
printf '\n--- env usage search ---\n'
rg -n "GUEST_TOKEN_SECRET|E2E_BASE_URL|test\.skip\(" tests/e2e tests -g '!**/node_modules/**'Repository: misty-step/linejam
Length of output: 14401
Skip on either missing env, not just GUEST_TOKEN_SECRET. The guard still skips remote runs even when E2E_BASE_URL is set, so the condition should match the message or the message should be narrowed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/e2e/major-page-smoke.spec.ts` around lines 16 - 20, The skip guard in
the major-page smoke test is too strict because it only checks
GUEST_TOKEN_SECRET, which still skips remote runs even when E2E_BASE_URL is
provided. Update the condition around test.skip in major-page-smoke.spec.ts to
reflect the intended behavior: either skip only when both GUEST_TOKEN_SECRET and
E2E_BASE_URL are missing, or narrow the skip message so it matches the current
guard. Use the existing missingGuestTokenSecret setup as the place to adjust the
logic.
Comment on lines
+105
to
+106
| await page.goto(path, { waitUntil: 'networkidle' }); | ||
| await assertVisible(page); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== test file outline ==\n'
ast-grep outline tests/e2e/major-page-smoke.spec.ts --view expanded || true
printf '\n== relevant lines around the goto call ==\n'
nl -ba tests/e2e/major-page-smoke.spec.ts | sed -n '85,120p'
printf '\n== locate Convex / websocket usage in app routes used by this test ==\n'
rg -n "Convex|useQuery|useMutation|createClient|WebSocket|networkidle|domcontentloaded" tests src app pages components -g '!**/node_modules/**' || true
printf '\n== find assertVisible definition ==\n'
rg -n "function assertVisible|const assertVisible|async function assertVisible" tests/e2e -g '!**/node_modules/**' || trueRepository: misty-step/linejam
Length of output: 427
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== tests/e2e/major-page-smoke.spec.ts ==\n'
cat -n tests/e2e/major-page-smoke.spec.ts | sed -n '1,220p'
printf '\n== assertVisible definition(s) ==\n'
rg -n -A 40 -B 10 "assertVisible" tests/e2e -g '!**/node_modules/**' || true
printf '\n== app routes referenced by the smoke test ==\n'
python3 - <<'PY'
from pathlib import Path
import re
p = Path("tests/e2e/major-page-smoke.spec.ts")
text = p.read_text()
paths = sorted(set(re.findall(r"path:\s*['\"]([^'\"]+)['\"]", text)))
print("\n".join(paths))
PY
printf '\n== likely Convex/live data usage in app sources ==\n'
rg -n "useQuery|useMutation|Convex|createClient|websocket|WebSocket|subscribe" app src tests -g '!**/node_modules/**' || trueRepository: misty-step/linejam
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== app/providers.tsx ==\n'
cat -n app/providers.tsx | sed -n '1,220p'
printf '\n== route files for the smoke-test paths ==\n'
for f in \
app/page.tsx \
app/host/page.tsx \
app/join/page.tsx \
app/releases/page.tsx \
app/me/poems/page.tsx \
app/poem/[id]/page.tsx \
app/recap/[code]/page.tsx
do
if [ -f "$f" ]; then
printf '\n--- %s ---\n' "$f"
cat -n "$f" | sed -n '1,260p'
fi
done
printf '\n== Playwright docs / local references to networkidle ==\n'
rg -n "networkidle|network idle|waitUntil" . -g '!**/node_modules/**' | sed -n '1,120p'Repository: misty-step/linejam
Length of output: 32024
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== app/poem/[id]/PoemDetail.tsx ==\n'
cat -n app/poem/[id]/PoemDetail.tsx | sed -n '1,260p'
printf '\n== lib/auth.ts ==\n'
cat -n lib/auth.ts | sed -n '1,260p'
printf '\n== routes with useQuery / useMutation in app ==\n'
rg -n "useQuery|useMutation" app -g '!**/node_modules/**' | sed -n '1,160p'
printf '\n== any explicit websocket client setup ==\n'
rg -n "WebSocket|websocket|persistent.*connection|networkidle" app lib tests -g '!**/node_modules/**' | sed -n '1,200p'Repository: misty-step/linejam
Length of output: 10997
🌐 Web query:
Playwright networkidle websockets documented wait forever persistent connections
💡 Result:
In Playwright, the 'networkidle' state is defined as the condition where there are no network connections for at least 500 milliseconds [1][2]. Because WebSockets, Server-Sent Events (SSE), and other persistent connections (such as analytics pings or health checks) often remain open indefinitely, they can prevent the 'networkidle' state from ever being reached [3][4][5]. Consequently, using 'networkidle' in pages with these types of connections will frequently cause tests to hang until they reach their configured timeout [6][3][7]. Playwright maintainers strongly discourage the use of 'networkidle' for production testing [1][5]. The official recommendation is to avoid "sniffing" the network state, as it is inherently unreliable in real-world applications [5]. Instead, users are advised to rely on web-first assertions and locators to verify that the application has reached the desired state [4][1][5]. For example, rather than waiting for the network to settle, you should wait for specific elements to become visible or contain expected text [5]: await page.locator('button').click; await expect(page.locator('status')).toHaveText('ready'); If you must wait for specific network activity, it is recommended to create custom utilities that track specific requests or responses rather than relying on the global 'networkidle' state [7][5][2]. Currently, Playwright does not provide a native option to ignore specific types of connections (like WebSockets) when calculating 'networkidle' [6][5][2].
Citations:
- 1: [Question| Slight documentation clarification for "networkidle" microsoft/playwright#22897
- 2: [Feature]: specify timeout ms for
networkidlemicrosoft/playwright#7856 - 3: [Feature]: Add networkidle2,networkidle1 support in waitForLoadState - networkidle method microsoft/playwright#37080
- 4: [Feature]: Add granular network idle options (networkidle0, networkidle2) to waitUntil microsoft/playwright#36730
- 5: [Feature] Ignore endpoints (SSE) on waitForLoadState(IDLE) microsoft/playwright#22242
- 6: [Feature] Add a flag to ignore webSockets when using
waitForLoadState('networkidle')microsoft/playwright#26487 - 7: [BUG] Infinite waiting for resolving waitForLoadState('networkidle') microsoft/playwright#19835
Avoid networkidle here tests/e2e/major-page-smoke.spec.ts:105-106
Several of these pages keep background activity alive via Convex and app-level providers, so waiting for global network quiescence is brittle and can stall CI. assertVisible already waits on the UI state; use domcontentloaded and let the assertions prove readiness.
♻️ Proposed fix
- await page.goto(path, { waitUntil: 'networkidle' });
+ await page.goto(path, { waitUntil: 'domcontentloaded' });
await assertVisible(page);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| await page.goto(path, { waitUntil: 'networkidle' }); | |
| await assertVisible(page); | |
| await page.goto(path, { waitUntil: 'domcontentloaded' }); | |
| await assertVisible(page); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/e2e/major-page-smoke.spec.ts` around lines 105 - 106, The smoke test in
`assertVisible`/page navigation is using `page.goto(..., { waitUntil:
'networkidle' })`, which is brittle for pages with ongoing background requests.
Update the navigation call in `tests/e2e/major-page-smoke.spec.ts` to use
`domcontentloaded` instead, and rely on `assertVisible(page)` to verify the page
is ready.
Contributor
Author
|
Closing this stale bundle without merging or deleting the donor branch. Current master is 24 commits ahead, this PR is conflicting, its latest E2E/merge-gate checks are red, and its release slice is superseded by PRs #312/#319 plus the later 0.x reset. Wholesale merge would also delete the release generator that the current Pages path invokes. Surviving donor map:
The commits remain available as donor evidence. New work should land as narrow, current-master PRs with fresh live proof. Agent: orchestrator |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Linejam deep-cleanup overhaul following the 2026-07-04 16-hour outage. All 5
incident cards plus 3 application-floor gaps, each independently live-verified:
NEXT_PUBLIC_CLERK_PUBLISHABLE_KEYat config time instead of a hand-listeddomain array (the exact bug that blocked auth site-wide for ~16h).
linejam-production-smokeCanary TTL monitor. A single failure is recordedwithout escalating; two consecutive failures open a real Canary incident
(live-verified: opened and auto-resolved a real incident against the
deployed Canary instance).
scripts/ci/check-schema-migration-sequencing.mjs)blocks any PR that both removes a
convex/schema.tsfield and adds itsmigration in the same diff — the exact class of bug that wedged every
deploy on 07-04. Regression test replays the actual outage commit's diff.
/releaseswas frozen at v0.1.0 for months while theapp was on v1.15.1 (two competing stores, one dead). Now
release.ymlwrites
content/releases/deterministically on every release; backfilledall 21 historical versions; a test gates
manifest.jsonagainstpackage.json's version on every run.gh auth logintoken pasted into a repo secret that silently broke whenthe repo went public; switched to the job's own ephemeral
GITHUB_TOKEN.pnpm doctor(verified-live onboarding check), a Playwright smoke matrix across major
pages at desktop + ~390px asserting zero console errors, and an
evidence-based coverage ratchet (85% → 89-90%, from 91.44%/86.32%/92.75%/92.9%
measured).
Also deleted a dead, untested, never-wired release-notes generator with an
ungated raw LLM call that could have reintroduced the split-brain
(
scripts/generate-releases.ts+lib/releases/parser.ts).Fresh-context critic pass
Two independent fresh-context critics (no access to this session's reasoning)
reviewed the finished branch. One found a real bug —
doctor's app-healthcheck defaulted to the wrong port — fixed and live-verified before this PR
was finalized. The other drove a real Chromium browser at 390px/1440px
against a real production build and confirmed the releases fix renders
correctly with no console errors (the actual Vercel preview is SSO-gated and
unreachable from this environment — documented, not silently skipped).
Test plan
pnpm exec vitest run— 117 files / 1256 tests pass, 1 pre-existing skippnpm test:ci(coverage) — exit 0 against ratcheted thresholdspnpm run typecheck:app/pnpm run lint— cleanactionlint .github/workflows/*.yml— cleanSelector Smoke, QA Evidence, merge-gate, trufflehog
next buildshowing the releases fix; fresh-context critics found and oneconfirmed bug fixed
run") needs an actual merge to verify, per its own criterion
Powder: linejam-912, linejam-913, linejam-914, linejam-915, linejam-916,
linejam-909, linejam-910, linejam-911 (all done)
🤖 Generated with Claude Code
Summary by CodeRabbit
pnpm doctorreadiness check to the development workflow.