feat(issues): allow reassigning issues to a different machine (PP-3hb)#1274
Open
timothyfroehlich wants to merge 17 commits intofeat/issue-detail-rework-PP-yxw.9from
Open
feat(issues): allow reassigning issues to a different machine (PP-3hb)#1274timothyfroehlich wants to merge 17 commits intofeat/issue-detail-rework-PP-yxw.9from
timothyfroehlich wants to merge 17 commits intofeat/issue-detail-rework-PP-yxw.9from
Conversation
…ite for PP-3hb)
Adds a "Move to another machine…" kebab action on the issue detail page
so technicians, admins, and the current machine's owner can fix issues
filed against the wrong game (a common APC pain point — games sit close
together and reporters are guests-on-the-fly).
- New permission `issues.reassign` in the matrix:
unauthenticated/guest = no, member = "owner", technician/admin = yes.
Resolved via `OwnershipContext.machineOwnerId` in the existing helper.
- Service `reassignIssueMachine` atomically reserves a fresh
`issueNumber` on the destination using the same `nextIssueNumber + 1`
pattern as `createIssue`. Old number on the source becomes a permanent
gap — only safe option given `unique_issue_number` on
`(machineInitials, issueNumber)` and external references to the old
URL.
- New timeline event variant `machine_reassigned` records both formatted
IDs and machine names. `assertUnreachableEvent` makes the format-case
mandatory.
- Server action returns `{ issueId, newUrl }` so the client navigates via
`router.push` after success — keeps the action result-driven and
composable with `useActionState` rather than throwing a Next redirect
error.
- Kebab lives in the new `PageHeader` `actions` slot from PR #1270;
trigger is hidden when the viewer has no eligible actions.
- E2E test (NN #11) clicks kebab → Move → confirms; verifies URL change
and timeline event. Negative case verifies the kebab is hidden for
members who don't own the machine.
Plan: ~/.claude/plans/i-need-to-allow-pure-parnas.md
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Updates to Preview Branch (feat/issue-reassign-machine-PP-3hb) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an “issue reassignment” capability so an issue can be moved from one machine to another, including permission gating, atomic renumbering on the destination machine, and a timeline entry recording the move.
Changes:
- Introduces
issues.reassignpermission (member: conditional machine-owner; technician/admin: allowed) and wires it into the issue detail page UI. - Implements
reassignIssueMachine(transactional machine counter increment + issue update) and adds a newmachine_reassignedtimeline event + formatter. - Adds unit tests for permissions/service/timeline and an E2E spec for the reassignment flow.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/lib/permissions/matrix.ts |
Adds new issues.reassign permission row. |
src/test/unit/permissions-matrix.test.ts |
Tests the new permission behavior + ownership resolution. |
src/services/issues.ts |
Adds transactional reassignIssueMachine service and result shape. |
src/services/issues.test.ts |
Adds service-level unit tests for reassignment scenarios. |
src/lib/timeline/types.ts |
Extends the timeline event union with machine_reassigned and formats it. |
src/lib/timeline/format.test.ts |
Adds formatter unit test for machine_reassigned. |
src/app/(app)/issues/schemas.ts |
Adds Zod schema for reassignment inputs. |
src/app/(app)/issues/actions.ts |
Adds reassignIssueMachineAction server action with permission enforcement + revalidation. |
src/app/(app)/m/[initials]/i/[issueNumber]/page.tsx |
Fetches machines and conditionally renders a header actions menu for reassignment. |
src/app/(app)/m/[initials]/i/[issueNumber]/issue-actions-menu.tsx |
Adds client-side kebab menu entry to open reassignment dialog. |
src/app/(app)/m/[initials]/i/[issueNumber]/reassign-machine-form.tsx |
Implements the client dialog + command picker + navigation on success. |
e2e/full/issues-reassign-machine.spec.ts |
Adds E2E coverage for technician reassignment + member-without-ownership hidden menu. |
.husky/pre-push |
Adds a best-effort bd dolt push on git push when bd is installed. |
.claude/hooks/worktree-remove.sh |
Adds a best-effort bd dolt push before worktree teardown when bd is installed. |
Comment on lines
+150
to
+155
| // Fetch all machines for the reassign-machine picker. Cheap given the org's | ||
| // machine count and rendered only when the viewer has permission. | ||
| db.query.machines.findMany({ | ||
| columns: { initials: true, name: true }, | ||
| orderBy: asc(machines.name), | ||
| }), |
Comment on lines
+19
to
+23
| const rememberIssueId = (page: Page): void => { | ||
| const issueId = extractIdFromUrl(page.url()); | ||
| if (issueId) { | ||
| createdIssueIds.add(issueId); | ||
| } |
Comment on lines
+1061
to
+1065
| const result = await reassignIssueMachine({ | ||
| issueId, | ||
| newMachineInitials, | ||
| userId: user.id, | ||
| }); |
Comment on lines
+157
to
+158
| .min(1, "Pick a machine to move this issue to") | ||
| .max(20, "Invalid machine"), |
…ge (PP-5px) (#1273) Three coordinated CAPTCHA changes: 1. Skip Turnstile CAPTCHA on /report when the user is logged in. The action reads auth.getUser() before calling verifyTurnstileToken(); the form omits the widget entirely when userAuthenticated. Anonymous reporters still get CAPTCHA + rate limit. 2. Normalize the report form's CAPTCHA wiring to match the auth forms: rename hidden field cf-turnstile-response -> captchaToken, reuse the shared extractCaptchaToken() helper, and gate the submit button on token presence (matching login/signup/forgot-password). 3. Add Turnstile CAPTCHA to /reset-password. Clones the forgot-password widget pattern client-side; verifies the token via verifyTurnstileToken() server-side because auth.updateUser() does not accept a captchaToken option in @supabase/supabase-js@2.103.3. Adds CAPTCHA to ResetPasswordResult. Tests: 4 new unit tests in actions-security.test.ts (reset-password CAPTCHA branches) and actions.test.ts (submitPublicIssueAction CAPTCHA branching); all 1013 unit tests pass; smoke + targeted full E2E for reset-password and public-reporting all green. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…#1277) Bumps the minor-patch-updates group with 10 updates: | Package | From | To | | --- | --- | --- | | [@tiptap/extension-link](https://github.com/ueberdosis/tiptap/tree/HEAD/packages/extension-link) | `3.22.3` | `3.22.4` | | [@tiptap/extension-mention](https://github.com/ueberdosis/tiptap/tree/HEAD/packages/extension-mention) | `3.22.3` | `3.22.4` | | [@tiptap/extension-placeholder](https://github.com/ueberdosis/tiptap/tree/HEAD/packages-deprecated/extension-placeholder) | `3.22.3` | `3.22.4` | | [@tiptap/html](https://github.com/ueberdosis/tiptap/tree/HEAD/packages/html) | `3.22.3` | `3.22.4` | | [@tiptap/pm](https://github.com/ueberdosis/tiptap/tree/HEAD/packages/pm) | `3.22.3` | `3.22.4` | | [@tiptap/react](https://github.com/ueberdosis/tiptap/tree/HEAD/packages/react) | `3.22.3` | `3.22.4` | | [@tiptap/starter-kit](https://github.com/ueberdosis/tiptap/tree/HEAD/packages/starter-kit) | `3.22.3` | `3.22.4` | | [@tiptap/suggestion](https://github.com/ueberdosis/tiptap/tree/HEAD/packages/suggestion) | `3.22.3` | `3.22.4` | | [baseline-browser-mapping](https://github.com/web-platform-dx/baseline-browser-mapping) | `2.10.19` | `2.10.20` | | [eslint](https://github.com/eslint/eslint) | `10.2.0` | `10.2.1` | Updates `@tiptap/extension-link` from 3.22.3 to 3.22.4 - [Release notes](https://github.com/ueberdosis/tiptap/releases) - [Changelog](https://github.com/ueberdosis/tiptap/blob/main/packages/extension-link/CHANGELOG.md) - [Commits](https://github.com/ueberdosis/tiptap/commits/v3.22.4/packages/extension-link) Updates `@tiptap/extension-mention` from 3.22.3 to 3.22.4 - [Release notes](https://github.com/ueberdosis/tiptap/releases) - [Changelog](https://github.com/ueberdosis/tiptap/blob/main/packages/extension-mention/CHANGELOG.md) - [Commits](https://github.com/ueberdosis/tiptap/commits/v3.22.4/packages/extension-mention) Updates `@tiptap/extension-placeholder` from 3.22.3 to 3.22.4 - [Release notes](https://github.com/ueberdosis/tiptap/releases) - [Changelog](https://github.com/ueberdosis/tiptap/blob/main/packages-deprecated/extension-placeholder/CHANGELOG.md) - [Commits](https://github.com/ueberdosis/tiptap/commits/v3.22.4/packages-deprecated/extension-placeholder) Updates `@tiptap/html` from 3.22.3 to 3.22.4 - [Release notes](https://github.com/ueberdosis/tiptap/releases) - [Changelog](https://github.com/ueberdosis/tiptap/blob/main/packages/html/CHANGELOG.md) - [Commits](https://github.com/ueberdosis/tiptap/commits/v3.22.4/packages/html) Updates `@tiptap/pm` from 3.22.3 to 3.22.4 - [Release notes](https://github.com/ueberdosis/tiptap/releases) - [Changelog](https://github.com/ueberdosis/tiptap/blob/main/packages/pm/CHANGELOG.md) - [Commits](https://github.com/ueberdosis/tiptap/commits/v3.22.4/packages/pm) Updates `@tiptap/react` from 3.22.3 to 3.22.4 - [Release notes](https://github.com/ueberdosis/tiptap/releases) - [Changelog](https://github.com/ueberdosis/tiptap/blob/main/packages/react/CHANGELOG.md) - [Commits](https://github.com/ueberdosis/tiptap/commits/v3.22.4/packages/react) Updates `@tiptap/starter-kit` from 3.22.3 to 3.22.4 - [Release notes](https://github.com/ueberdosis/tiptap/releases) - [Changelog](https://github.com/ueberdosis/tiptap/blob/main/packages/starter-kit/CHANGELOG.md) - [Commits](https://github.com/ueberdosis/tiptap/commits/v3.22.4/packages/starter-kit) Updates `@tiptap/suggestion` from 3.22.3 to 3.22.4 - [Release notes](https://github.com/ueberdosis/tiptap/releases) - [Changelog](https://github.com/ueberdosis/tiptap/blob/main/packages/suggestion/CHANGELOG.md) - [Commits](https://github.com/ueberdosis/tiptap/commits/v3.22.4/packages/suggestion) Updates `baseline-browser-mapping` from 2.10.19 to 2.10.20 - [Release notes](https://github.com/web-platform-dx/baseline-browser-mapping/releases) - [Commits](web-platform-dx/baseline-browser-mapping@v2.10.19...v2.10.20) Updates `eslint` from 10.2.0 to 10.2.1 - [Release notes](https://github.com/eslint/eslint/releases) - [Commits](eslint/eslint@v10.2.0...v10.2.1) --- updated-dependencies: - dependency-name: "@tiptap/extension-link" dependency-version: 3.22.4 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: minor-patch-updates - dependency-name: "@tiptap/extension-mention" dependency-version: 3.22.4 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: minor-patch-updates - dependency-name: "@tiptap/extension-placeholder" dependency-version: 3.22.4 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: minor-patch-updates - dependency-name: "@tiptap/html" dependency-version: 3.22.4 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: minor-patch-updates - dependency-name: "@tiptap/pm" dependency-version: 3.22.4 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: minor-patch-updates - dependency-name: "@tiptap/react" dependency-version: 3.22.4 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: minor-patch-updates - dependency-name: "@tiptap/starter-kit" dependency-version: 3.22.4 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: minor-patch-updates - dependency-name: "@tiptap/suggestion" dependency-version: 3.22.4 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: minor-patch-updates - dependency-name: baseline-browser-mapping dependency-version: 2.10.20 dependency-type: direct:development update-type: version-update:semver-patch dependency-group: minor-patch-updates - dependency-name: eslint dependency-version: 10.2.1 dependency-type: direct:development update-type: version-update:semver-patch dependency-group: minor-patch-updates ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* chore(hooks): adopt bd v1.0.3 section markers in husky (PP-gc0) Wrap the bd integration in each husky hook with `--- BEGIN BEADS INTEGRATION v1.0.3 ---` markers so `bd doctor` recognizes them as Dolt-compatible and version-stamped. Custom PinPoint logic (lint-staged, main-branch block, dolt push, worktree setup) lives outside the markers so future `bd hooks install` upgrades preserve it. Also drop the project-level `bd prime` SessionStart hook from .claude/settings.json — the beads plugin's own plugin.json already declares this hook, so we were firing it twice per session. bd doctor before: 1 error, 2 warnings (Dolt incompatibility, hooks at v0.0.0) bd doctor after: 0 errors, gitignore patterns refreshed via --fix Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hooks): capture bd exit code without set -e short-circuit (PP-gc0) Switch from `cmd; _bd_exit=$?` to `cmd || _bd_exit=$?` (with explicit `_bd_exit=0` initialization) so the special-case handling for exit 124 (timeout) and exit 3 (Dolt database not initialized) actually runs. Under `set -e`, `timeout ... bd hooks run X` returning non-zero aborts the hook before the next line; the graceful-skip branches were unreachable. Copilot caught this on PR #1275. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hooks): quote $_bd_exit in numeric tests (PP-gc0) Defensive quoting in `[ "$_bd_exit" -eq N ]` so the test parses safely even if the variable is ever empty. Copilot caught this on PR #1275. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…1278) * chore(e2e): remove unused screenshots tests and workflows (PP-ekq) The on-demand screenshot capture system (e2e/screenshots/, separate Playwright config, and the pr-screenshots/cleanup-screenshots GitHub Actions) is no longer in use. Drop the test, config, workflows, and package.json entries (including the ratchet-lint pins for the deleted workflow files). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(e2e): scrub remaining screenshots refs from ci.yml and vercel.json Two more references to the removed screenshots feature: - ci.yml `Ratchet Check` step listed the deleted workflow files (would fail on next CI run) - vercel.json `git.ignoredBuildStep` was added in PR #979 with the screenshots feature to skip Vercel builds on the gh-pages branch the cleanup workflow pushed to. Now obsolete. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Owner
Author
|
Raise the Playwright actionTimeout from 10 s (flat) to 30 s in CI / 5 s locally. Radix portal mounts — used by DropdownMenu and Select — can lag on loaded CI runners; the previous 10 s budget was tight enough to cause intermittent failures (PP-jsh, PP-v7g). The CI-only bump absorbs that variance without slowing local iteration. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…l (PP-5ra) (#1290) Marks 5 known-broken E2E tests as expected-to-fail with test.fixme() so main CI flips from red to green and PR signal becomes trustworthy again. Each fixme references its tracking bead and bug summary. The fixme form (rather than skip) ensures: - Tests still appear in CI reports as expected-to-fail (visibility) - CI fails if a test ever passes accidentally (forces unskip) - The reason string is self-documenting in source Silenced specs: - email-and-notifications.spec.ts (password reset) → PP-q9r - oauth-connected-accounts.spec.ts (Discord OAuth) → PP-e20 - machine-details-extended.spec.ts (logout DropdownMenu) → PP-jsh - status-overhaul.spec.ts (Status Select) → PP-v7g - issues-crud-extended.spec.ts (Unwatch strict-mode) → PP-49m BINDING RULE: feature PRs do not merge while any test.fixme(true, "PP-...") exists in the codebase. Each fixme is removed by the PR that fixes its bead. Tracking: PP-5ra (meta), PP-q9r, PP-e20, PP-jsh, PP-v7g, PP-49m. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…PP-278) (#1286) * docs(agents): document e2e:all and warn against raw playwright test (PP-278) Updates AGENTS.md section 4 "Which Tests to Run": - Reframes item 6 from "NEVER run e2e:full" to "NEVER run e2e:full directly during iteration" — the new e2e:all wraps it as part of a legitimate pre-review pass. - Adds item 7 documenting `pnpm run e2e:all` (introduced in PP-6dp) for the "want everything locally before review" case, with explicit ~10-15 min runtime and a note that each suite runs in its own Playwright invocation with a fresh global-setup between them. - Adds item 8 warning against `pnpm exec playwright test` without --config= — picks up all specs at once and cross-contaminates state. Pointers to e2e:all as the safe alternative. No code changes. Depends on PP-6dp (PR #1285) landing first so the referenced script exists. Refs PP-2on (epic). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(agents): scope item 8 to bare playwright invocation only (PP-278) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…PP-7kc) (#1276) Convert Checkbox from defaultChecked to controlled useState so an unchecked choice is not discarded when the form re-renders after a failed login attempt.
Bumps [@types/nodemailer](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/nodemailer) from 7.0.11 to 8.0.0. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/nodemailer) --- updated-dependencies: - dependency-name: "@types/nodemailer" dependency-version: 8.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Tim Froehlich <timothyfroehlich@gmail.com>
…00s) (#1281) * docs(agents): edit canonical specs in place, no divergence notes (PP-00s) Adds a "Keeping specs aligned" subsection to AGENTS.md section 5 instructing agents to edit canonical spec docs in place rather than appending divergence notes at the bottom of files. Codifies the lesson from PR #1270 review churn where spec/implementation drift produced repeated Copilot back-and-forth. Also tells agents that finding an existing divergence note is a signal to fold the content back into the canonical text, not preserve the annotation. No automation — process guidance only. Single source of truth = the spec docs themselves. Refs PP-2on (epic). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(agents): scope spec-alignment to canonical, drop PR-specific rationale (PP-00s) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(agents): fix archetypes reference to point to SKILL.md sections (PP-00s) The previous text referenced a non-existent .agent/skills/pinpoint-design-bible/archetypes/ directory. Page and Modal Archetypes are sections inside SKILL.md (§5 and §17). --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…zw) (#1282) * feat(worktree): auto-configure branch tracking on post-checkout (PP-ozw) `git worktree add /path -b new-branch origin/main` creates new-branch but leaves its upstream as origin/main, so later `git pull`/`git push` operate against main. AGENTS.md documents the manual `git push -u origin <branch>` fix but worktree_setup.py never automated it. Adds configure_branch_tracking(branch, worktree_path) called from main() right after get_branch(): - If origin/<branch> exists: `git branch --set-upstream-to=origin/<branch>` and confirm. - If not: print stderr reminder to run `git push -u origin <branch>` after first commit. - Skip for main/master/HEAD (detached state). - Never auto-push — that would create remote branches for throwaway local experiments. - Failures are non-fatal: warn and continue. Setup must not block on transient git issues. Verified scenarios manually: - New local-only branch → reminder printed. - Branch with existing remote → upstream set, confirmation printed. - main / HEAD → silently skipped. Refs PP-2on (epic). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(worktree): respect existing upstream config + add coverage (PP-ozw) Addresses Copilot review on #1282: - Post-checkout hook now checks current upstream before reconfiguring, so deliberately-set non-origin/main upstreams aren't clobbered on every branch switch. - Adds 9-scenario test coverage for upstream resolution + git stderr capture edge cases. * chore(worktree): remove unused imports + ruff format (PP-ozw) Drop unused subprocess and call imports from upstream-tracking tests, apply ruff format. Surfaced by pnpm run check after merging origin/main. * refactor(worktree): tighten branch-tracking + drop heavy unit tests (PP-ozw) Trim configure_branch_tracking and get_current_upstream to ~38 lines total (was ~93). Behavior unchanged: skip main/master/HEAD, preserve custom upstreams, print reminder when no remote, idempotent set-upstream. Drop the 184-line TestConfigureBranchTracking class and add a top-of-file comment explaining the testing philosophy: worktree setup is infrastructure code we test by running. Mock-heavy unit tests for git interactions test the mocks more than real behavior; the existing parsing/port/manifest tests remain (they cover hard-to-verify-by-usage logic). * fix(worktree): quote branch in reminder + clear stale upstream when no remote (PP-ozw) - shlex.quote the branch name in the push-reminder shell command (Copilot: branch names can legally contain shell metacharacters) - When origin/<branch> doesn't exist, also unset the stale origin/main upstream that `git worktree add -b new origin/main` leaves behind, so `git pull` doesn't operate against main Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#1283) * fix(e2e): pass empty Turnstile keys to Playwright webServer (PP-uc8) playwright.config.ts webServer.env only forwarded PORT and MOCK_BLOB_STORAGE to the dev server it spawns. The dev server therefore inherited whatever Turnstile keys .env.local had, which made the login form's enforceCaptcha check (`hasTurnstile && NODE_ENV !== "test"`) evaluate true and required agents to manually clear the env vars before running E2E. Now passing empty NEXT_PUBLIC_TURNSTILE_SITE_KEY and TURNSTILE_SECRET_KEY through webServer.env activates the existing graceful-bypass logic: - Client: TurnstileWidget returns null when site key is empty, so the form doesn't enforce a token. - Server: verifyTurnstileToken returns true in non-prod when the secret key is missing, so submission succeeds. No new logic — only plumbing the existing bypass through to the spawned dev server. Production unaffected (real keys still apply outside Playwright). Refs PP-2on (epic). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(e2e): use dev:e2e script that overrides Turnstile after sourcing .env.local (PP-uc8) - Add dev:e2e script that sources .env.local then re-exports empty NEXT_PUBLIC_TURNSTILE_SITE_KEY and TURNSTILE_SECRET_KEY, so real keys from .env.local cannot clobber the bypass (Copilot #1 on playwright.config.ts:122) - Change playwright.config.ts webServer.command from dev to dev:e2e - Remove now-redundant Turnstile vars from webServer.env (the script handles it) - Update PR description with reuseExistingServer caveat for local dev workflows (Copilot #2 on playwright.config.ts:122) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(e2e): comment out Turnstile keys for local dev (PP-uc8 redesign) The previous approach added a dev:e2e npm script that overrode .env.local after sourcing it. This solved E2E but left local dev with the wrong default — the Turnstile widget rendered on every form (with a Cloudflare round-trip) for no real value. New approach: scripts/worktree_setup.py writes the Turnstile keys commented-out by default, so .env.local has them visible as opt-in documentation but inactive. The application code already handles unset/empty correctly: - TurnstileWidget renders nothing if siteKey is falsy - verifyTurnstileToken returns true (skip) in non-prod if secretKey is falsy To test CAPTCHA UI locally: chmod 644 .env.local # uncomment the two TURNSTILE_* lines chmod 444 .env.local The TURNSTILE_TEST_SECRET carveout in turnstile.ts is preserved — it now protects the explicit opt-in path (developer uncomments keys to debug CAPTCHA in headless mode where the widget JS may not complete). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(e2e): browser+Docker preflight in global-setup (PP-cao)
Adds two fail-fast checks at the top of e2e/global-setup.ts before the
existing Supabase/Postgres pre-flight:
1. checkBrowserBinaries(config): inspects config.projects, derives the
set of browsers needed (chromium/firefox/webkit), and verifies each
binary exists via existsSync(executablePath()). On miss, throws with
the actionable hint "Install with: pnpm exec playwright install
--with-deps <names>". Only checks browsers the active config needs,
so CI's chromium-only install isn't penalized.
2. checkDocker(): runs `docker info` via spawnSync (no shell, fixed argv)
with a 5s timeout. On failure, throws with "Start OrbStack: open -a
OrbStack" hint.
Both checks run regardless of SKIP_SUPABASE_RESET — that flag only skips
DB reset/migration/seed; browsers and the Docker daemon are still
required to launch Playwright.
Replaces cryptic mid-run failures like
browserType.launch: Executable doesn't exist
with clear actionable messages before any spec runs.
Also updates the global-setup unit test to pass a mock FullConfig (empty
projects array disables the browser check for tests focused on DB
orchestration) and mocks spawnSync for the Docker check.
Refs PP-2on (epic).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(e2e): branch ENOENT vs daemon-down + add failure-path coverage (PP-cao)
Branch checkDocker() error handling: ENOENT (not installed → install
hint) vs other errors (daemon down → start hint). Add spawnSync
mock-call assertion and 4 failure-path tests: Docker not running,
Docker not installed via ENOENT, missing browser binary, and ordering
check (Docker before Supabase).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* docs(e2e): make Docker error messages platform-neutral (PP-cao follow-up)
The previous error text recommended `brew install --cask orbstack` and
`open -a OrbStack` which is mac-only guidance. Replace with platform-neutral
messages naming OrbStack/Docker Desktop/Colima/systemctl alternatives, with
the mac brew hint preserved as a sub-line for the most common platform.
- ENOENT (not installed): Lists OrbStack, Docker Desktop, Docker Engine,
with Mac brew hint as sub-line
- Daemon down: Lists OrbStack, Docker Desktop, Colima, systemctl alternatives
with "start your Docker daemon" as the lead instruction
- Test assertions still pass: substring matching still matches new messages
("Docker is not installed" and "Docker daemon is not running" preserved)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…1272) * feat(forms): apply consistent CREATE-form reset pattern (PP-rvv) All four CREATE forms now use a single canonical reset pattern: useEffect on success → formRef.reset() → explicit setState clears for every controlled field → client-side redirect last. This makes resets deterministic and adds defense in depth — if redirect fails or the component stays mounted, the user sees an empty form rather than stale values. - createMachineAction: returns ok({machineId, redirectTo}) instead of calling redirect() server-side. Server-side redirect throws before useActionState reaches the client, so the form's cleanup effect never fired (already documented in submitPublicIssueAction). - create-machine-form.tsx: adds formRef + dual-pass reset + Clear button with AlertDialog confirmation. OwnerSelect remounts via key bump. - unified-report-form.tsx: reset effect now runs before window.location.assign, preserves defaultMachineId from URL param, bumps RichTextEditor key, clears localStorage, plus a Clear button. - AddCommentForm and InviteUserDialog kept as canonical references (already clean). - Integration tests updated to assert result.ok instead of catching NEXT_REDIRECT. - e2e/full/form-resets.spec.ts covers all four forms. - docs/patterns/create-form-reset.md captures the pattern + edge cases (URL-derived defaults, localStorage drafts, shadcn Select, RichTextEditor). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(report): drop stale machineId from localStorage; pin select-fallback risk (PP-lql) ROOT CAUSE Two production reports of /report creating issues against the alphabetically-first machine instead of the user's selection. Both came from the same mechanism: 1. localStorage carries a machineId from a previous draft / tenant / since- deleted machine. 2. The restoration effect set `selectedMachineId` from localStorage WITHOUT checking the UUID against the current machinesList. 3. The native `<select value="<stale-uuid>">` then renders — and submits — the first non-disabled `<option>` (HTML-spec behavior). FormData reads that first option's value, so the action receives a valid UUID for an unrelated machine. Zod uuid() and the server's existence check both pass, the issue is created on the wrong machine, and the user sees no error. FIX - Restoration: only call `setSelectedMachineId(parsed.machineId)` when the ID is in the current machinesList. Otherwise the draft's other fields still restore, but the user must pick a machine. - Defense in depth: useEffect that resets selectedMachineId to "" if it ever ends up referencing a machine not in the list (catches any other path: BFCache, prop swaps, etc.). REGRESSION COVERAGE - src/app/(app)/report/select-fallback.test.tsx — 3 JSDOM tests pinning the underlying HTML behavior so any future "fix" of the silent-fallback at the harness level is intentional. - e2e/full/report-stale-machine.spec.ts — seeds a stale UUID in localStorage, loads /report, asserts the dropdown is empty and submission is blocked. - docs/patterns/create-form-reset.md — documents the silent-fallback footgun and the validation-on-restore mitigation pattern. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(e2e): scope InviteUserDialog locators to dialog (PP-rvv) The /m/new page contains another element matching /Email/i (likely the AppHeader user menu), causing Playwright strict-mode to fail when the test selected by-label from the page root. Scope every dialog interaction to page.getByRole("dialog") so the locator can only ever resolve inside the dialog. First Name / Last Name happened to be dialog-only so they passed before; the fix is uniform across all three. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(e2e): use role+name for InviteUserDialog Email field (PP-rvv) Previous fix scoped to getByRole("dialog") to avoid a page-level collision. CI revealed the dialog ITSELF contains two /Email/i matches: the email <input> and a "Send Invitation Email" switch. Switch to getByRole("textbox", { name: "Email" }) so the switch can't match. NOTE: This commit is not yet pushed — verifying locally before push to avoid another CI round-trip on a still-broken fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(report): strip ?machine= on Clear; cleanup test machines (PP-rvv) Two issues from Copilot review: 1. UnifiedReportForm Clear button reset selectedMachineId="" but left the ?machine= URL param intact when the user had picked a machine from the dropdown (no URL-derived default). A reload would silently re-select that machine. Strip the param via history.replaceState when defaultMachineId is absent. 2. The new "CreateMachineForm clears fields after successful submit" E2E test created a real machine each run but afterEach only cleaned issues. Track created initials in a per-test array and pass them to cleanupTestEntities to prevent pollution and initials collisions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(e2e): assert ?machine= is stripped on Clear (PP-rvv) Adds explicit URL assertions to the existing "Clear button wipes fields" test so the strip-on-clear behavior fixed in 7ef693f has regression coverage. Without this assertion, a future change could silently revert that fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(report): remount Turnstile widget on form reset (PP-rvv) After Clear or success-reset, the hidden captchaToken cleared but the Turnstile widget still showed "solved", desyncing the visible state from the form. The Submit button's disabled gate now blocks stale submissions, but the user gets stuck with no clear remediation. Add a turnstileWidgetKey state that bumps on both reset paths, forcing a fresh widget mount that visibly matches the cleared token. Resolves Copilot review concern about Turnstile desync after reset. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(report)+test: address Copilot review feedback (PP-rvv) Three Copilot concerns fixed together: 1. unified-report-form: success-effect didn't strip ?machine= for user-picked selections. Mirror the Clear-button cleanup so a failed redirect or back-nav doesn't leave a stale URL pointing at the old machine. Both paths now read window.location.search (not the useSearchParams hook), since history.replaceState mutations from the dropdown's onChange don't update the React hook. 2. e2e/form-resets: the CreateMachineForm submit test used a 3-digit timestamp suffix for initials, which collides easily across parallel workers and quick reruns. Switch to getTestMachineInitials() from test-isolation.ts (worker index + random char). 3. machine-actions unit tests: 5 success-path tests still wrapped the call in try/catch NEXT_REDIRECT — that pattern is now dead code (the action returns ok({redirectTo}) instead of throwing), so the tests passed without asserting anything about success. Replace with explicit expect(result.ok).toBe(true) + redirectTo + machineId assertions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(e2e): scope form-reset cleanup per-worker + drop persistent comment (PP-rvv) - Worker-unique title prefix prevents cross-worker afterEach interference. - Comment-creation test now verifies form-reset behavior without persisting a comment to a seeded issue (avoids timeline-noise accumulation across runs). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
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
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
Adds a "Move to another machine…" kebab action on the issue detail page so issues filed against the wrong machine can be reassigned without losing comments / timeline / number-history. Permissions: members can reassign their own machines' issues, technicians/admins can reassign anywhere, guests blocked.
Status (2026-05-06)
Not yet ready for merge. Two reasons:
1. Gated on PR #1270 (issue-detail rework)
This PR is currently based on
feat/issue-detail-rework-PP-yxw.9. The kebab menu lives in the newPageHeaderactions slot introduced in #1270. After #1270 merges to main:git fetch origin && git checkout feat/issue-reassign-machine-PP-3hb && git merge origin/mainsrc/app/(app)/m/[initials]/i/[issueNumber]/page.tsxfeat/issue-detail-rework-PP-yxw.9→mainviagh pr edit 1274 --base main2. Four valid Copilot comments to address (post-rebase)
src/app/(app)/issues/actions.tsreassignIssueMachineActionreturns genericSERVERerror instead of validatingnewMachineInitials; domain errors become opaquesrc/app/(app)/issues/schemas.tsreassignIssueMachineSchemaonly checks length, ignores^[A-Z0-9]{2,6}$regex; invalid initials bypass schema validatione2e/full/issues-reassign-machine.spec.tsextractIdFromUrl()doesn't match new/m/<initials>/i/<number>URL pattern; test cleanup leaks datasrc/app/(app)/m/[initials]/i/[issueNumber]/page.tsxallMachinesqueried unconditionally despite permission gate in code; extra DB query for ineligible viewersWhat's already shipped here
reassignIssueMachine()server action with atomicnextIssueNumberreservation viaUPDATE … RETURNINGcheckPermission())issue-actions-menu.tsx(kebab),reassign-machine-form.tsx(combobox dialog)Test plan
Tracking