Skip to content

Open shifts: take back your own request while it waits for approval - #3485

Merged
gitar-bot[bot] merged 19 commits into
mainfrom
claude/open-shifts-withdraw-hj3sp1
Oct 12, 2026
Merged

gitar-bot[bot] merged 19 commits into
mainfrom
claude/open-shifts-withdraw-hj3sp1

Conversation

@BigSimmo

@BigSimmo BigSimmo commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

User description

Summary

Before: once a doctor asked for an open shift, only a roster manager could clear the request. A doctor who asked by mistake, or whose plans changed, had to message the manager.

After: a request that is still waiting for approval shows "Take back my request" on the shift advert. A confirm sheet explains that the shift goes back on the list for everyone, and that the doctor can ask again while it stays open. An approved shift is already on the roster, so it can't be taken back this way. The doctor gives it away through Roster as for any other shift.

This is item 10 of the Open shifts expansion, which the owner approved on 12 Oct 2026 ("Go ahead with your recommendations", then "Go ahead with all improvements and recommendations"). It is kept apart from the UI work in #3483 because it contains a migration.

How: a new SQL function roster_open_withdraw(actor, service, openShiftId) follows the roster_needs_replace pattern. It is security invoker, executable by service_role only, and uses the same lock order as every team write. It resets a claimed row to open only when the caller is the doctor who asked. Anyone else gets roster_not_found, so nobody learns who asked for what. A row that has been approved, cancelled or expired gets roster_conflict. The app sends a new open.withdraw action through the team route to that function. roster_command is unchanged, and there are no table or data changes.

Verification

  • npx tsc --noEmit: clean.
  • npm run test:focused on the changed source: Test Files 173 passed (173), Tests 2419 passed (2419).
  • New tests/open-shifts-withdraw.dom.test.tsx covers the confirm, the success notice, the conflict message, and the button staying hidden on approved shifts and offline (3 tests passed).
  • tests/roster-team-api.test.ts covers routing to roster_open_withdraw and refusing bad payloads (22 tests passed).
  • tests/sql/roster-behaviour.sql section 8 now covers someone else's take-back (not found), a manager's take-back (not found), the doctor's own take-back (shift reopened and claimer cleared), a claim again, and a take-back after approval (conflict). This runs in CI's Migration replay, not locally.
  • npm run check:migration-role: passed. check-migration-immutability --write sealed the new migration.
  • Drift manifest: no Docker daemon was available here. I computed the new function's def_hash with the same expression as schema_drift_snapshot on a local Postgres 16 scratch database. The method reproduced the committed roster_needs_replace hash exactly (c9251a59…). schema_sha256 was recomputed from supabase/schema.sql. CI's freshness step is the authoritative check.

Risk and rollout

  • Risk: low. It adds one function that writes only a single row, and only one the caller themselves claimed. Every refusal path leaves the row untouched.
  • Rollback: revert the app change so the button and action disappear. The function can stay unused, or a follow-up migration can drop it.
  • Provider or production effects: merging this PR applies the migration to the live database within seconds. Merge only inside an approved window, by the owner. Keep this PR draft until then.

Clinical Governance Preflight

  • Source-backed claims still require linked source verification before clinical use: no clinical content changed.
  • No patient-identifiable document workflow was introduced or expanded without explicit governance approval: the change touches roster requests only.
  • Supabase target remains PsychSift Production (sjrfecxgysukkwxsowpy)
  • Service-role keys and private document access remain server-only: the function is granted to service_role only and called from the server route.
  • Demo/synthetic content remains clearly separated from real clinical sources: example mode hides the button, and the demo command returns an example result.
  • Source metadata, review status, and outdated/unknown-source behavior remain conservative: not touched.
  • Deployment classification/TGA SaMD impact was checked when clinical decision-support behavior changed: no decision-support behaviour changed.

Notes

🤖 Generated with Claude Code

https://claude.ai/code/session_01CZwGiwt7eXkD4fwyFM83gY

Generated by Claude Code


Summary by cubic

Lets a doctor take back their own open-shift request while it waits for approval, so a mistaken request or changed plans no longer require messaging a roster manager to clear it.

A waiting request now shows "Take back my request" on the shift advert, with a confirm sheet explaining the shift goes back on the list for everyone. The new roster_open_withdraw function reopens only rows the caller themselves claimed; anyone else gets roster_not_found, and approved, cancelled, expired, or started requests get roster_conflict. The started check reads clock_timestamp() after the locks, so a take-back that waited on a lock is still refused once the start time has passed. If a request is decided before the take-back lands, the page forgets this visit's copy of it and re-reads it, pointing the doctor to My shifts; a take-back only overrides the read it was made against, so a request claimed again from another tab or device shows as waiting, and a "Waiting for approval" state from this visit lapses once a newer read no longer shows the shift as this doctor's. After a take-back, keyboard focus moves to the result, since the button that opened the sheet is gone. Approved shifts are already on the roster, so they're given away through Roster as before.

  • Adds a SQL migration that applies to the live database automatically on merge, plus a Playwright spec that drives the stale take-back race and matches the notice by its text so Next.js's route announcer doesn't trip strict mode.
  • No table, data, or roster_command changes.
  • Once a take-back finds a request already decided, the same sheet can't be retried against the old listing.

Written for commit 179461b. Summary will update on new commits.

View guided diff Turn on auto-fix

Summary by Gitar

  • Safeguards:
    • Refuses a take-back once the shift has started, since a started shift can no longer be claimed
  • Decided requests:
    • A request decided before the take-back lands is forgotten, re-read, and directs the doctor to My shifts
  • Accessibility:
    • Keyboard focus moves once to the take-back result instead of on every re-render

This will update automatically on new commits.


CodeAnt-AI Description

Let doctors take back open-shift requests awaiting approval

What Changed

  • Doctors can confirm taking back their own request while it awaits approval; the shift returns to the list for others, and they can request it again while it remains open.
  • The take-back option is unavailable after approval, once the shift has started, or while offline.
  • If the request is decided before the take-back goes through, the page refreshes and explains that it can no longer be taken back.

Impact

✅ Doctors can correct mistaken shift requests
✅ Open shifts return to the list when requests are withdrawn
✅ Clearer outcomes when a request is already decided

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

A new roster_open_withdraw function lets only the doctor who asked put a
waiting open-shift request back on the list. An approved, cancelled or
expired request is refused with roster_conflict; anyone else's request
reads as missing. The advert page offers "Take back my request" with a
confirm sheet. roster_command is unchanged; no table or data changes.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwGiwt7eXkD4fwyFM83gY
@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 6eb410ee-cf22-4897-b50c-dae179a38bdd

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@supabase

supabase Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Updates to Preview Branch (claude/open-shifts-withdraw-hj3sp1) ↗︎

Deployments Status Updated
Database ✅ Mon, 12 Oct 2026 17:28:36 UTC
Services ✅ Mon, 12 Oct 2026 17:28:36 UTC
APIs ✅ Mon, 12 Oct 2026 17:28:36 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks Status Updated
Configurations ✅ Mon, 12 Oct 2026 17:28:39 UTC
Migrations ✅ Mon, 12 Oct 2026 17:28:41 UTC
Seeding ✅ Mon, 12 Oct 2026 17:28:45 UTC
Edge Functions ✅ Mon, 12 Oct 2026 17:28:49 UTC

View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

@codeant-ai

codeant-ai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

🏁 CodeAnt Quality Gate Results

Commit: 179461b4
Scan Time: 2026-10-12 17:43:01 UTC

✅ Overall Status: PASSED

Quality Gate Details

Quality Gate Status Details
Secrets ✅ PASSED 0 secrets found
Duplicate Code ✅ PASSED 0.0% duplicated
SAST ✅ PASSED No security issues
Bugs ✅ PASSED Rating S: No bugs
IAC ✅ PASSED No IAC issues

View Full Results

@codspeed

codspeed Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

⚠️ 2 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

ℹ️ 1 benchmark has no flame graph

The flamegraph could not be generated because too much time was spent in runtime internals, making the visualization not useful for performance analysis.

✅ 8 untouched benchmarks


Comparing claude/open-shifts-withdraw-hj3sp1 (179461b) with main (2077255)

Open in CodSpeed

gitar-bot[bot]
gitar-bot Bot previously approved these changes Oct 12, 2026

@gitar-bot gitar-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gitar has auto-approved this PR (configure)

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwGiwt7eXkD4fwyFM83gY
gitar-bot[bot]
gitar-bot Bot previously approved these changes Oct 12, 2026

@gitar-bot gitar-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gitar has auto-approved this PR (configure)

@BigSimmo
BigSimmo marked this pull request as ready for review October 12, 2026 12:23
@codeant-ai

codeant-ai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Incremental review completed 9d178eb Oct 12, 2026 · 16:38 16:41
✅ Reviewed your PR 0eb0110 Oct 12, 2026 · 12:24 12:28

@codeant-ai

codeant-ai Bot commented Oct 12, 2026

Copy link
Copy Markdown

Thanks for using CodeAnt! 🎉

We're free for open-source projects. if you're enjoying it, help us grow by sharing.

Share on X ·
Reddit ·
LinkedIn

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BigSimmo has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-12T16:41:39.327105Z 9d178eb Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@codeant-ai codeant-ai Bot added the size:L This PR changes 100-499 lines, ignoring generated files label Oct 12, 2026
@qodo-code-review qodo-code-review Bot added 🕐 40+ Minutes and removed size:L This PR changes 100-499 lines, ignoring generated files labels Oct 12, 2026
@qodo-code-review

qodo-code-review Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Let doctors take back pending open-shift requests

✨ Enhancement 🧪 Tests 🕐 40+ Minutes

Grey Divider

AI Description

• Doctors can take back their own pending requests, returning eligible shifts to the open list.
• A confirmation sheet explains the outcome; stale, approved, started, and offline requests cannot
 be taken back.
• A dedicated SQL function enforces ownership and locking, with API, UI, and SQL coverage.
Diagram

sequenceDiagram
    actor D as Doctor
    participant A as Shift advert
    participant T as Team API
    participant R as Repository
    participant F as Withdraw function
    participant S as Open shifts
    D->>A: Confirm take-back
    A->>T: open.withdraw
    T->>R: Validated action
    R->>F: Call dedicated RPC
    F->>S: Lock and check request
    alt Own pending request before start
        F->>S: Reopen and clear claim
        F-->>R: Open result
        R-->>T: Result
        T-->>A: Success
        A-->>D: Notice and refresh
    else Not owned or no longer eligible
        F-->>R: Not found or conflict
        R-->>T: Error
        T-->>A: Error
        A-->>D: Explanation and refresh
    end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Add open.withdraw to roster_command
  • ➕ Keeps all open-shift writes behind one RPC.
  • ➖ Changes an established function for an ownership-sensitive action.
  • ➖ Broadens the review and migration scope.

Recommendation: Keep the dedicated security-invoker function. It isolates the ownership, timing, and lock checks without changing roster_command or the table schema; the existing team API still provides the common entry point.

Files changed (13) +499 / -21

Enhancement (7) +270 / -16
open-shifts-advert-page.tsxAdd take-back confirmation and advert feedback +167/-6

Add take-back confirmation and advert feedback

• Shows a take-back action for eligible pending requests and explains its effect in a confirmation sheet. Handles success, stale-request errors, reloading, and focus without continuing to display a withdrawn request as pending.

src/components/open-shifts/open-shifts-advert-page.tsx

demo-team-core.tsReturn a demo receipt for open.withdraw +1/-0

Return a demo receipt for open.withdraw

• Makes the demo roster command return an open-shift result for the new action.

src/lib/roster/team/demo-team-core.ts

model.tsValidate the open.withdraw action +2/-0

Validate the open.withdraw action

• Adds a strict action variant requiring an open-shift UUID, so the team route can accept and validate take-back requests.

src/lib/roster/team/model.ts

repository.tsRoute take-backs to their dedicated RPC +17/-10

Route take-backs to their dedicated RPC

• Dispatches open.withdraw to roster_open_withdraw while leaving other actions on their existing RPC paths.

src/lib/roster/team/repository.ts

database.types.tsDeclare the withdrawal RPC signature +4/-0

Declare the withdrawal RPC signature

• Adds the typed arguments and JSON return value for roster_open_withdraw.

src/lib/supabase/database.types.ts

20261012090000_roster_open_withdraw.sqlAdd an owner-only withdrawal function +44/-0

Add an owner-only withdrawal function

• Adds a service-role-only, security-invoker function that locks the team and request, refuses non-owners as not found, and rejects requests that are no longer pending or whose shift has started. An eligible request returns to open with its claimant and claim time cleared.

supabase/migrations/20261012090000_roster_open_withdraw.sql

schema.sqlMirror the withdrawal function in the schema +35/-0

Mirror the withdrawal function in the schema

• Adds the new function and its execution grants to the checked-in schema definition.

supabase/schema.sql

Tests (4) +217 / -2
cross-tenant-roster-probe.tsProbe cross-tenant access for take-back actions +1/-1

Probe cross-tenant access for take-back actions

• Adds open.withdraw to the roster action samples exercised by the cross-tenant probe.

scripts/lib/cross-tenant-roster-probe.ts

open-shifts-withdraw.dom.test.tsxCover the advert take-back experience +175/-0

Cover the advert take-back experience

• Tests confirmation, success and focus, competing claims, stale-request feedback, and the disappearance of locally remembered requests. Also checks that approved, offline, and started shifts do not offer take-back.

tests/open-shifts-withdraw.dom.test.tsx

roster-team-api.test.tsVerify API routing and payload validation +16/-0

Verify API routing and payload validation

• Checks that open.withdraw calls the dedicated RPC with the session actor and rejects invalid or extra payload fields.

tests/roster-team-api.test.ts

roster-behaviour.sqlExercise withdrawal authorization and state transitions +25/-1

Exercise withdrawal authorization and state transitions

• Checks that other doctors and managers cannot withdraw a request, that its owner can reopen and reclaim it, and that started or approved shifts produce conflicts without reopening.

tests/sql/roster-behaviour.sql

Other (2) +12 / -3
applied-migration-hashes.jsonSeal the new migration +3/-2

Seal the new migration

• Records the withdrawal migration's hash in the applied-migration manifest.

supabase/applied-migration-hashes.json

drift-manifest.jsonRecord the new function in the drift snapshot +9/-1

Record the new function in the drift snapshot

• Updates the schema checksum and records the withdrawal function's signature, definition hash, and execution grants.

supabase/drift-manifest.json

@qodo-code-review

Copy link
Copy Markdown
Contributor

Qodo Fixer

No findings are available for this PR yet. Findings appear here once Qodo has reviewed the PR.

@gitar-bot
gitar-bot Bot enabled auto-merge (squash) October 12, 2026 12:25
@claude

claude Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Claude encountered an error after 2s —— View job


I'll analyze this and get back to you.

@codeant-ai

codeant-ai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

CodeAnt PR Risk: Low Risk

  • The PR appears safe to merge; withdrawal is limited to the requesting active member and guarded against decided or started shifts.
  • SQL and UI tests cover successful withdrawal, stale requests, authorization, and started shifts.

Assessed commit: 00923f44fd40

@qodo-code-review

qodo-code-review Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

No findings for this group


Remediation recommended

No findings for this group


Informational

No findings for this group

Grey Divider


Resolved findings

1. New browser test breaks the PR shard check and never runs ✓ Resolved
Description
open-shifts-withdraw was added to productionSpecPattern and testMatch in
playwright.config.ts, but not to the copy of that pattern in scripts/playwright-pr-shards.mjs
(productionSpecFilePattern, which must be kept in sync), and
tests/ui-open-shifts-withdraw.spec.ts was not added to prUiSpecProfiles either. The #159
alignment test in tests/playwright-pr-shards.test.ts will now fail on every PR. Even with that
test fixed, listProductionSpecFiles (used by both the PR shards and the release-matrix shards)
leaves the spec out, so the new browser test for the take-back race never runs in CI.
Code

playwright.config.ts[26]

+  /.*(?:adaptive-answer-ui|api-csrf-proxy|answer-progress-ui-smoke|dsm-ui-smoke|ui-(admin|smoke|stress|accessibility|clinical-ask|cme-phone|dictionary|document-canvas|tools|tools-show-all|overlap|universal-search|specifiers|sources|formulation(?:-result-cards)?|forms-section-nav|chrome-scroll|therapy-nav-scroll|therapy-pathways|mode-nav-density|my-day-calendar|on-call-(?:boards|call|now|service)|teaching|patient-number-field|phone-motion|phone-scroll(?:-[a-z0-9-]+)?|pwa|roster-team|open-shifts-withdraw|route-coverage|style-contract|token-layer-resolution|visual-artifacts|hydration|user-journeys))\.spec\.ts/;
Relevance

●●● Strong

Direct precedent shows missing shard-pattern synchronization and registration are accepted CI
reliability fixes.

PR-#2577
PR-#3444

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The config pattern now matches ui-open-shifts-withdraw.spec.ts. The script copy, which says it is
the same matcher and must be kept in sync, does not include open-shifts-withdraw. The unit test
asserts that both patterns select the same on-disk files, so it fails. The release shards build
their file list from the script's listProductionSpecFiles, so the spec is left out there too. A
past repo record shows the same mistake with ui-token-layer-resolution: it was fixed by updating
productionSpecFilePattern and adding a shard profile entry.

scripts/playwright-pr-shards.mjs[21-23]
tests/playwright-pr-shards.test.ts[45-55]
scripts/playwright-release-shards.mjs[30-30]
docs/branch-review-records/bf249c989a4f4b805bae7156c55640bf94ee51503f9a81c935cc51f04bd36879.record.md[1-1]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`playwright.config.ts` now collects `ui-open-shifts-withdraw.spec.ts`, but `scripts/playwright-pr-shards.mjs` `productionSpecFilePattern` (the copy that must be kept in sync) was not updated, and the spec has no shard profile. The `#159` parity test fails, and the spec is never run in the PR or release shards.

## Fix Focus Areas
- scripts/playwright-pr-shards.mjs[21-23]
- scripts/playwright-pr-shards.mjs[45-300]
- scripts/playwright-release-shards.mjs[39-290]

## Recommended Fix
Add `open-shifts-withdraw` to the `ui-(?:...)` alternation in `productionSpecFilePattern`, next to `roster-team`. Add a `{ file: "tests/ui-open-shifts-withdraw.spec.ts", shard: N, fullSeconds: ~5, criticalSeconds: 0 }` entry to `prUiSpecProfiles`, choosing the shard that keeps the balance limits. Add an estimated per-project entry for `tests/ui-open-shifts-withdraw.spec.ts` in every project table of `releaseSpecSeconds`. Then run `npx vitest run tests/playwright-pr-shards.test.ts tests/playwright-release-shards.test.ts tests/playwright-project-isolation.test.ts`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Stale refresh goes untested ✗ Dismissed
Description
The browser spec returns an unclaimed shift on every requests read and asserts only that the waiting
state disappears. onStale clears that state before calling reload(), so these assertions pass
even if the reload never fetches or applies the post-decision listing.
Code

tests/ui-open-shifts-withdraw.spec.ts[R87-88]

+  await expect(page.getByText("Waiting for approval")).toHaveCount(0);
+  await expect(page.getByRole("button", { name: "Take back my request", exact: true })).toHaveCount(0);
Relevance

●●● Strong

Recent precedents favor assertions proving refreshed, changed UI state rather than local handlers
alone.

PR-#3488
PR-#3362

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test skill calls for a deterministic regression proof, and the testing-review skill requires
Playwright coverage of interactive frontend behavior. The fixture never changes across reads, while
the assertions can pass solely because the existing stale handler clears local state.

tests/ui-open-shifts-withdraw.spec.ts[63-64]
tests/ui-open-shifts-withdraw.spec.ts[83-89]
src/components/open-shifts/open-shifts-advert-page.tsx[718-722]
Skill: test
Skill: testing-review

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The stale-withdrawal browser test cannot distinguish a successful refresh from the immediate local state change, so it does not protect the reload behavior it claims to cover.
## Fix Focus Areas
- tests/ui-open-shifts-withdraw.spec.ts[63-64]
- tests/ui-open-shifts-withdraw.spec.ts[83-89]
## Recommended Fix
Serve a claimed-by-me listing before withdrawal and a distinct post-decision listing only on the subsequent requests read. Assert that the post-decision read occurs and that the advert renders a state unique to its response.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Started shifts blame roster managers ✓ Resolved
Description
WithdrawSheet treats every roster_conflict response as proof that a roster manager decided the
request, although roster_open_withdraw also returns that code when a shift has started. If a
doctor opens the sheet before the start but confirms afterward, the sheet can submit the request and
show a manager-decision explanation even though no decision occurred.
Code

supabase/migrations/20261012090000_roster_open_withdraw.sql[37]

+  if v_open.status <> 'claimed' or v_open.starts_at <= now() then raise exception 'roster_conflict'; end if;
Relevance

●●● Strong

The UI presents a misleading actor explanation for a distinct server state, matching accepted
reader-facing correctness fixes.

PR-#2584
PR-#2978

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The SQL guard raises roster_conflict when starts_at <= now(), and the sheet maps that code to
“Your roster manager decided.” The confirmation button is disabled only for a previously detected
stale request, so a sheet opened before the shift starts can still submit after it starts and
receive that misleading message.

supabase/migrations/20261012090000_roster_open_withdraw.sql[34-37]
src/components/open-shifts/open-shifts-advert-page.tsx[290-299]
src/components/open-shifts/open-shifts-advert-page.tsx[290-300]
src/components/open-shifts/open-shifts-advert-page.tsx[313-320]
Skill: api-review

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A started-shift refusal is presented as a roster manager's decision because both a decided request and a started shift return `roster_conflict`.

## Fix Focus Areas
- supabase/migrations/20261012090000_roster_open_withdraw.sql[34-37]
- src/components/open-shifts/open-shifts-advert-page.tsx[290-300]

## Recommended Fix
Use conflict guidance that accurately covers both a decided request and a shift that has started, without claiming the manager acted unless the response distinguishes those cases. Add a focused test for the started-shift response.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View resolved (9)
4. Doctors can retry a stale take-back ✗ Dismissed
Description
The stale-action handler clears the local request marker and starts a reload, but it leaves the
withdrawal sheet open and reusable after its busy state resets. When the server has already decided
or removed the request, the old listing remains in memory until the asynchronous reload finishes,
allowing another withdrawal attempt and, for requests sent during this visit, potentially exposing a
new request action before refreshed data arrives.
Code

src/components/open-shifts/open-shifts-advert-page.tsx[R652-656]

+          onStale={() => {
+            // The request was decided elsewhere, so this visit's own record of sending it no longer holds.
+            setSent(null);
+            state.reload();
+          }}
Relevance

●●● Strong

Stale-action handling should close the reusable sheet and suppress stale local state before
asynchronous reload completes.

PR-#3463

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The failure handler calls onStale for both roster_conflict and roster_not_found, then resets
busy without closing the sheet. state.reload() only starts asynchronous loading and does not
synchronously replace the current listings, while setSent(null) can make alreadyMine false for a
request created during the current visit; the added test replaces the mocked state only after
observing the reload call and therefore does not cover this interim state.

src/components/open-shifts/open-shifts-advert-page.tsx[284-293]
src/components/open-shifts/open-shifts-advert-page.tsx[305-306]
src/components/open-shifts/use-open-shifts.ts[75-80]
src/components/open-shifts/use-open-shifts.ts[93-117]
tests/open-shifts-withdraw.dom.test.tsx[102-110]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The `onStale` handler clears `sent` and starts an asynchronous reload, but does not close or invalidate the withdrawal sheet. The old listing remains available during the reload, so the user can retry the take-back or see a new request action based on stale data.

### Fix Focus Areas
- src/components/open-shifts/open-shifts-advert-page.tsx[652-656]
- src/components/open-shifts/open-shifts-advert-page.tsx[276-296]

### Recommended Fix
When a stale result is received, close the withdrawal sheet and mark the local action as stale or otherwise disable its controls until the refreshed listing is available. Ensure clearing the visit-local `sent` state cannot expose a request action while the old listing is still being displayed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Started requests reopen but cannot be claimed ✓ Resolved
Description
roster_open_withdraw checks that a request is claimed but not whether starts_at has passed
before setting it back to open. Because claimed rows do not automatically expire, withdrawing an
in-progress shift makes it visible in request reads until its end time even though the claim action
rejects it once it has started.
Code

supabase/migrations/20261012090000_roster_open_withdraw.sql[R35-37]

+  if v_open.status <> 'claimed' then raise exception 'roster_conflict'; end if;
+
+  update public.roster_open_shifts set status = 'open', claimed_by = null, claimed_at = null where id = v_open.id;
Relevance

●●● Strong

Migration reviews accept missing lifecycle guards that permit invalid state transitions and require
explicit boundary checks.

PR-#2187
PR-#3111
PR-#2547

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The function reopens an owned claimed row without a start-time condition, and claimed rows are not
automatically changed to expired. Request reads include rows until their end time, while the claim
path rejects shifts once they start; the advert predicate also lacks a start-time condition.

supabase/migrations/20261012090000_roster_open_withdraw.sql[30-38]
supabase/migrations/20260926225409_roster_mode.sql[1241-1245]
supabase/migrations/20260926225409_roster_mode.sql[693-710]
supabase/migrations/20260926225409_roster_mode.sql[1243-1246]
src/components/open-shifts/open-shifts-advert-page.tsx[394-396]
Skill: api

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A claimed request can remain claimed after its shift starts. Withdrawal then reopens an in-progress shift that appears in reads but cannot be claimed.

## Fix Focus Areas
- supabase/migrations/20261012090000_roster_open_withdraw.sql[30-38]
- supabase/schema.sql[27307-27315]
- src/components/open-shifts/open-shifts-advert-page.tsx[394-396]
- tests/sql/roster-behaviour.sql[166-185]

## Recommended Fix
Reject withdrawal when `starts_at <= now()` before updating the row in the database function and its schema copy. Hide the withdrawal action after the shift starts, and add a SQL regression case for an already-started claimed shift.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. A newer request can look withdrawn ✗ Dismissed
Description
withdrawnReadAt and state.readAt are compared using Date.getTime(), so two separate successful
reads in the same millisecond are treated as the same read. If the doctor requests the shift again
before that newer read receives a distinct timestamp, the advert can continue showing the take-back
notice and request controls instead of the newer waiting state.
Code

src/components/open-shifts/open-shifts-advert-page.tsx[R432-433]

+  const withdrawnHere = withdrawnKey === `${listing.serviceId}:${listing.id}`;
+  const withdrawn = withdrawnHere && (state.readAt?.getTime() ?? null) === withdrawnReadAt;
Relevance

●●● Strong

Recent open-shifts precedents accept read-freshness guards and tests for retry races; millisecond
collisions are a concrete correctness bug.

PR-#3483

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The withdrawal handler stores state.readAt.getTime() as the override identity, and rendering keeps
the override whenever the current read has the same millisecond value. useOpenShifts assigns each
successful reload readAt: new Date() without guaranteeing distinct millisecond values, while the
added DOM test only exercises a read 60 seconds later.

src/components/open-shifts/open-shifts-advert-page.tsx[432-433]
src/components/open-shifts/open-shifts-advert-page.tsx[727-732]
src/components/open-shifts/use-open-shifts.ts[117-127]
tests/open-shifts-withdraw.dom.test.tsx[84-99]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The withdrawal override identifies a read by `Date.getTime()`. A reload can complete within the same millisecond and receive the same timestamp, causing the page to retain the withdrawal override after a newer request is present.

## Fix Focus Areas
- src/components/open-shifts/open-shifts-advert-page.tsx[432-433]
- src/components/open-shifts/open-shifts-advert-page.tsx[727-732]
- src/components/open-shifts/use-open-shifts.ts[117-127]

## Recommended Fix
Expose a monotonic read identity from `useOpenShifts`, such as an incrementing generation or unique read token created for each successful listing read. Store that identity when the withdrawal succeeds and compare identities rather than millisecond timestamps, while preserving the existing per-shift key check.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


7. Waiting withdrawals reopen started shifts ✓ Resolved
Description
roster_open_withdraw checks starts_at against now(), which is fixed when the transaction
begins rather than when the locked row is checked. If the call starts before the shift and waits for
a service or row lock until after it starts, it can set the shift to open even though a subsequent
claim will reject it.
Code

supabase/migrations/20261012090000_roster_open_withdraw.sql[37]

+  if v_open.status <> 'claimed' or v_open.starts_at <= now() then raise exception 'roster_conflict'; end if;
Relevance

●●● Strong

Lock-wait races and stale time checks are accepted correctness issues; evaluate the current clock
after locks.

PR-#3111
PR-#3216

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new function acquires an advisory lock and a row lock before evaluating the start-time guard,
then reopens the row. The claim path rejects a shift whose start time has passed, so a withdrawal
delayed past that time can leave an open shift that a later transaction cannot claim.

supabase/migrations/20261012090000_roster_open_withdraw.sql[28-39]
supabase/migrations/20260926225409_roster_mode.sql[1243-1246]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A withdrawal waiting on a lock can reopen a shift after its start because `now()` retains the transaction-start time.
## Fix Focus Areas
- supabase/migrations/20261012090000_roster_open_withdraw.sql[28-39]
- supabase/schema.sql[27305-27316]
## Recommended Fix
Use a current wall-clock timestamp for the start-time guard after acquiring the locks, and keep the migration and schema definitions in sync.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


8. Another shift can appear open to its requester ✓ Resolved
Description
OpenShiftsAdvertPage keeps withdrawn as component state without tying it to serviceId and
openShiftId, then uses it to suppress listing.claimedByMe. If the same page instance receives
another shift after a withdrawal, that doctor's pending request can be displayed as an open shift
with a request action instead of its waiting state.
Code

src/components/open-shifts/open-shifts-advert-page.tsx[394]

+  const alreadyMine = (listing.claimedByMe && !withdrawn) || sent !== null;
Relevance

●●● Strong

Recent history accepts stale state across entity changes and requests identity synchronization.

PR-#3463
PR-#3480

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The route supplies shift identity as props, while the new boolean remains unchanged across prop
updates. That boolean overrides the current listing's ownership flag and can make a future claimed
listing pass the page's open-shift branch.

src/app/(search-app)/open-shifts/shift/[serviceId]/[openShiftId]/page.tsx[11-18]
src/components/open-shifts/open-shifts-advert-page.tsx[334-346]
src/components/open-shifts/open-shifts-advert-page.tsx[394-403]
src/components/open-shifts/open-shifts-advert-page.tsx[653-658]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A successful withdrawal can leave local state that misrepresents a different shift shown by the same page instance.
## Fix Focus Areas
- src/components/open-shifts/open-shifts-advert-page.tsx[342-345]
- src/components/open-shifts/open-shifts-advert-page.tsx[394-403]
- src/components/open-shifts/open-shifts-advert-page.tsx[647-659]
## Recommended Fix
Store the withdrawn shift's service and shift IDs rather than a page-wide boolean, and apply the optimistic state only when they match the current listing. Reset or close shift-specific sheets when the route identity changes.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


9. Advert says a claimed shift is open ✓ Resolved
Description
The withdrawal success notice checks withdrawn && !alreadyMine without checking the refreshed
listing's status. If another doctor claims the shift before reload() finishes, the page displays
“The shift is open again” alongside “Someone else has it now.”
Code

src/components/open-shifts/open-shifts-advert-page.tsx[R485-488]

+      {withdrawn && !alreadyMine ? (
+        <p role="status" className="mx-3 mt-4 text-sm font-medium text-[color:var(--text-heading)]">
+          Request taken back. The shift is open again.
+        </p>
Relevance

●●● Strong

Recent reviews accept race-condition fixes preventing success messaging from contradicting refreshed
state.

PR-#3200
PR-#3469

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The review skill calls for realistic failure-path checks. Withdrawal sets the persistent flag before
reloading, while the new notice ignores the listing status that the closed-state branch uses.

src/components/open-shifts/open-shifts-advert-page.tsx[394-417]
src/components/open-shifts/open-shifts-advert-page.tsx[485-488]
src/components/open-shifts/open-shifts-advert-page.tsx[653-657]
Skill: review

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A competing claim during refresh leaves the withdrawal success notice visible beside the advert's closed state.

## Fix Focus Areas
- src/components/open-shifts/open-shifts-advert-page.tsx[394-417]
- src/components/open-shifts/open-shifts-advert-page.tsx[485-489]
- tests/open-shifts-withdraw.dom.test.tsx[63-78]

## Recommended Fix
Keep the confirmation that the request was withdrawn, but say the shift is open only while the current listing is open. Add a test that refreshes the listing with another doctor's claim.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


10. Keyboard focus disappears after take-back ✓ Resolved
Description
onWithdrawn sets the advert to withdrawn and closes WithdrawSheet, removing the conditionally
rendered “Take back my request” button without providing another focus-return target. After keyboard
confirmation, Sheet cannot restore focus to the disconnected opener, leaving the user without a
focused control at their prior position.
Code

src/components/open-shifts/open-shifts-advert-page.tsx[R653-656]

+          onWithdrawn={() => {
+            setSent(null);
+            setWithdrawn(true);
+            setWithdrawOpen(false);
Relevance

●●● Strong

Accessibility history consistently accepts focus restoration when closing removes or unmounts the
triggering control.

PR-#3355
PR-#2211
PR-#2589

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The withdrawal button is rendered only while canWithdraw is true; setting withdrawn makes
alreadyMine, and therefore canWithdraw, false. The success handler closes the sheet as that
button is removed, while the shared sheet restores focus only when its target remains connected.
Because no replacement target is supplied, successful withdrawal leaves focus unrestored.

src/components/open-shifts/open-shifts-advert-page.tsx[576-587]
src/components/open-shifts/open-shifts-advert-page.tsx[647-659]
src/components/ui/sheet.tsx[349-370]
src/components/open-shifts/open-shifts-advert-page.tsx[394-396]
src/components/open-shifts/open-shifts-advert-page.tsx[653-657]
src/components/ui/sheet.tsx[367-371]
src/components/ui/sheet.tsx[384-390]
Skill: accessibility-review

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Successful withdrawal removes the button that opened `WithdrawSheet`, so the sheet cannot restore keyboard focus to its default target when it closes.

## Fix Focus Areas
- src/components/open-shifts/open-shifts-advert-page.tsx[485-489]
- src/components/open-shifts/open-shifts-advert-page.tsx[576-587]
- src/components/open-shifts/open-shifts-advert-page.tsx[647-659]

## Recommended Fix
Provide a focus-return target that remains mounted after successful withdrawal, such as a programmatically focusable success notice, the My shifts link, or an appropriate page heading. Preserve restoration to the opener when the doctor cancels, and test keyboard focus after a successful withdrawal.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


11. Browser test only passes because the clock is frozen ✓ Resolved
Description
After open.claim, the stub returns claimedByMe: false on the reload. The advert keeps showing
"Waiting for approval" and the take-back button only because page.clock.setFixedTime makes the
reload's readAt (new Date()) equal the sentReadAt recorded at send time, so sent stays set.
With a real clock, that same reload clears sent and hides the button before line 80 clicks it. So
the test checks a screen production never shows (the button next to a read that says the request
isn't yours) rather than the race it is meant to cover (a request decided between two reads), and it
breaks if the clock setup changes.
Code

tests/ui-open-shifts-withdraw.spec.ts[R23-24]

+    // The request is deliberately absent on the read after the stale withdrawal.
+    claimedByMe: false,
Relevance

●●● Strong

Team accepts deterministic browser-test fixes when frozen timing masks the behavior under test.

PR-#2305
PR-#2463

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
sent in the advert keeps the local send state only when listing.claimedByMe is true or
state.readAt equals sentReadAt. readAt comes from new Date() in use-open-shifts.ts, which
the fixed fake time keeps constant across reloads. So whether the button is visible depends on the
frozen clock, not on the listing data.

src/components/open-shifts/open-shifts-advert-page.tsx[429-435]
src/components/open-shifts/use-open-shifts.ts[120-127]
tests/ui-open-shifts-withdraw.spec.ts[8-26]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The stale take-back spec reaches the take-back button only because `page.clock.setFixedTime` gives every read the same `readAt`, while the stubbed reload already reports `claimedByMe: false`.

## Fix Focus Areas
- tests/ui-open-shifts-withdraw.spec.ts[8-89]

## Recommended Fix
Make the stub keep state: return `status: "claimed", claimedByMe: true` from `what=requests` after `open.claim`. When `open.withdraw` arrives, switch to `claimedByMe: false` (the manager declined) and respond with `roster_not_found`. Then the visible-button state matches what production shows, and the test does not depend on the reads sharing a timestamp.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


12. Take-back race lacks browser coverage ✓ Resolved
Description
The new stale-request test exercises the take-back race only through a mocked jsdom render and
manual state replacement, so it does not verify the real route, dialog interaction, or reload-driven
rendering in a browser. Changes to focus handling, portals, or the actual open-shift route could
therefore break this flow while the added test still passes.
Code

tests/open-shifts-withdraw.dom.test.tsx[R92-111]

+  it("drops a request sent on this visit once a take-back finds it already declined", async () => {
+    const open = waiting({ status: "open", claimedByMe: false });
+    mocks.state = stateWith(open);
+    const view = renderFor(open);
+    mocks.post.mockResolvedValueOnce({ ok: true, result: { ok: true, openShiftId: open.id, status: "claimed" } });
+    fireEvent.click(screen.getAllByRole("button", { name: "Request this shift" })[0]!);
+    fireEvent.click(screen.getByRole("checkbox"));
+    fireEvent.click(screen.getByRole("button", { name: "Yes, send request" }));
+    await waitFor(() => expect(screen.getByText("Waiting for approval")).toBeTruthy());
+
+    // The manager declines before the take-back lands, which clears the request.
+    mocks.post.mockResolvedValueOnce({ ok: false, code: "roster_not_found", message: "Not found." });
+    fireEvent.click(screen.getByRole("button", { name: "Take back my request" }));
+    fireEvent.click(screen.getByRole("button", { name: "Yes, take it back" }));
+    await waitFor(() => expect(mocks.state.reload).toHaveBeenCalledTimes(2));
+    mocks.state = stateWith(open);
+    view.rerender(<OpenShiftsAdvertPage serviceId={open.serviceId} openShiftId={open.id} />);
+    expect(screen.queryByText("Waiting for approval")).toBeNull();
+  });
+
Relevance

● Weak

Recent reviews rejected adding Playwright coverage when focused mocked DOM tests already covered the
behavior.

PR-#3419
PR-#3436

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added regression coverage is a jsdom test that replaces the mocked hook state manually after
reload. The testing-review skill requires interactive frontend behavior to be covered by Playwright
specs as well, so this change does not verify the real browser route and dialog lifecycle.

tests/open-shifts-withdraw.dom.test.tsx[92-111]
Skill: testing-review

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The stale take-back race is covered only by a mocked jsdom component test, which cannot verify the real route, modal behavior, focus handling, or reload-driven UI update.

## Fix Focus Areas
- tests/open-shifts-withdraw.dom.test.tsx[92-111]

## Recommended Fix
Add or extend a Playwright `ui-*.spec.ts` test that opens the real open-shift advert, submits a request, simulates the request becoming unavailable before take-back completes, confirms the take-back, and verifies the stale error plus refreshed request state. Keep the existing unit-style test for fast local coverage, but ensure the browser test uses deterministic mocked backend responses rather than live external services.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 9 rules
✅ Cross-repo context — repo relationships
✅ REVIEW.md
Review mode: Auto: 🚀 Fast: Localized test-only stabilization changes with no production behavior or high-risk paths.

Grey Divider

Tip of the day
💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

gitar-bot[bot]
gitar-bot Bot previously approved these changes Oct 12, 2026

@gitar-bot gitar-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gitar has auto-approved this PR (configure)

Comment thread playwright.config.ts Resolved
Comment thread tests/ui-open-shifts-withdraw.spec.ts Resolved
@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 9127b34

…-back spec

The take-back override now applies only to the read it was made against, so a
request made again from another tab or device shows as waiting rather than
offering Request this shift. The new take-back browser spec joins the PR and
release shard tables.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CZwGiwt7eXkD4fwyFM83gY
gitar-bot[bot]
gitar-bot Bot previously approved these changes Oct 12, 2026

@gitar-bot gitar-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gitar has auto-approved this PR (configure)

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 08c5678

@gitar-bot gitar-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gitar has auto-approved this PR (configure)

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code review by qodo was updated up to the latest commit 9fb65dc

@qodo-code-review

qodo-code-review Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

No code changes since the last review — review skipped

Qodo Logo

@gitar-bot

gitar-bot Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor
CI failed: The PR policy check failed because the added Supabase migration reuses version 20261012090000, which already exists on main; applied migration versions never re-run on live, so the migration needs a new, newer version.

Overview

One unique error template across 12 analyzed logs. The failure is a single blocking PR policy error from the migration-history check, and it is directly related to this change. Warnings (UI verification, governance preflight, operational-risk bundling) are advisory and omitted from the failure analysis.

Failures

Migration version reused from main (confidence: high)

  • Type: configuration
  • Affected jobs: 114934433883
  • Related to change: yes
  • Root cause: The added file supabase/migrations/20261012090000_roster_open_withdraw.sql reuses migration version 20261012090000, which already exists on main. Supabase records each version once, so an edit under an applied version would reach the repository but never the live database, causing drift.
  • Suggested fix: Rename the migration to a new file with a unique version newer than 20261012090000, update any migration-history metadata as needed, and rerun the PR policy check.

Summary

  • Change-related failures: 1 (migration version reuse blocking the PR policy check).
  • Infrastructure/flaky failures: None identified.
  • Recommended action: Create a new migration with a unique, newer version instead of reusing 20261012090000, then rerun the workflow.
Code Review ✅ Approved 2 closed / 2 findings

🔴 High risk · The production migration adds a function that clears a doctor's claim and reopens shifts, potentially letting others claim a slot if its checks misfire.

Lets a doctor take back their own open-shift request while it awaits approval, via a new roster_open_withdraw function and a confirm sheet on the shift advert. Resolved the stale 'Waiting for approval' page after a failed take-back and the inline focus ref that stole focus on every re-render. No issues remain open.

Auto-approved: No blocking issues found.
Please see Auto-approve Docs for details on setting custom approval criteria.

✅ 2 closed
✅ Edge Case: Failed take-back leaves a stale 'Waiting for approval' page

📄 src/components/open-shifts/open-shifts-advert-page.tsx:281-290
A take-back can fail with roster_conflict (the manager approved or cancelled the request) or roster_not_found (the manager declined it, which clears claimed_by). In both cases the request has already been decided, but withdraw() only sets an error and returns. It never calls state.reload(), so after the sheet is closed the advert still says "Waiting for approval" and still shows "Take back my request". Every retry fails the same way until the doctor leaves the page. The fix is to reload the listing on these two error codes (for example by giving WithdrawSheet an onStale callback that calls state.reload()), so the page shows the request's real state.

✅ Bug: Inline focus ref steals focus back on every page re-render

📄 src/components/open-shifts/open-shifts-advert-page.tsx:504-514 📄 src/components/open-shifts/open-shifts-advert-page.tsx:352
ref={(node) => node?.focus()} creates a new callback on every render. In React 19 (the repo uses 19.3.0), a changed callback ref is detached and attached again on each commit, so focus() runs on every re-render while the take-back notice is showing, not only once. The page re-renders when state.reload() lands, and useRosterNow ticks every 60 s. That means a keyboard or screen-reader user who has moved on, for example into the Request sheet opened from "Request this shift" or to any other control, has focus pulled back to the notice. When a dialog is open, focus is pulled out of it. Move focus only once, using a stable ref and an effect keyed on withdrawnKey, or a useCallback ref.

Review coverage

🤖 Auto-approval Approved

📋 Rules No rules evaluated

🧪 Functional validation Not enabled · Set up

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

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

Labels

🔴 Large blast radius size:L This PR changes 100-499 lines, ignoring generated files 🕐 40+ Minutes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants