Repository navigation
fix(onboarding): disable submit while the job title eligibility check runs - #1413
Conversation
Splits the flag declaration out of #1395 so it can land on its own: adds 'job_title_eligibility' to the OnboardingFeatures union (with its JSDoc) and enables it in the example app. No behavior wired up yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds the useJobTitleEligibilityCheck hook and jobTitleEligibilityCheckOptions query, and wires OnboardingForm's onBlur to call the flowBag's new checkJobTitleEligibility(values). The check only runs on the contract_details step, when 'job_title_eligibility' is enabled, and when the role fields (role_description, role_is_onsite, role_requires_license) are filled and valid. Dedup: params are compared against the previous check (fast-deep-equal) before firing a request, and jobTitleEligibilityCheckOptions sets staleTime: Infinity since the query key already encodes employmentId + params. Without both, react-query's built-in dedup only covers truly concurrent calls with an identical key — an imperative queryClient.query() call is not a mounted observer, so it treats every call as a fresh mount and refetches under the default staleTime: 0. The result isn't consumed yet (no UI reacts to eligibility yet) - follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Removes the console.log statements left over from development. Also fixes a real bug surfaced while writing tests for this: paramsChanged was read immediately after calling setParams(updaterFn), assuming the functional updater runs synchronously. It doesn't reliably do that, so paramsChanged came back false almost every time - meaning the eligibility check never actually fired a request, for any input. Fixed by comparing against the params value already in the closure before calling setParams, instead of depending on a side effect inside the updater. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
.claude/settings.json is a personal/local permission-allowlist file, not meant for this PR - it got swept into the previous commit because it was already staged from earlier local work. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The JSDoc claimed the check also runs on entering the contract details step and before submitting it, but nothing calls it that way - only OnboardingForm's onBlur does. Since useOnboarding is reachable through the public ./flows/* entry point, this overstated the contract for anyone building a custom UI around the headless hook. Reworded to describe only what's actually wired up, and pointed custom UIs at calling checkJobTitleEligibility themselves for other trigger points instead of implying it happens automatically. Also drops stepValues, initialContractDetailsValues, and fieldValues from useJobTitleEligibilityCheck's params - they were threaded through from hooks.tsx but never used inside the hook, leftover scaffolding for the same unimplemented triggers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
check() compared nextParams against params captured from the render closure after two awaits (handleValidation, parseFormValues). If a second blur's check() started before the first one's setParams had committed a re-render, both calls would compare against the same stale params - depending on timing, that can either fire a duplicate request or wrongly treat a later complete result as unchanged and skip it. Flagged by Bugbot on PR #1406, independently of the same race I'd already called out in review. Fixed by tracking params in a ref alongside the state (state still drives the reactive useQuery's enabled/key). Ref writes are synchronous and immediately visible to any concurrently-resolving check() call, so the comparison always sees the latest known params regardless of render timing, closing the window entirely rather than narrowing it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds two integration tests against the rendered OnboardingFlow (not the headless hook) since that's the surface most consumers actually hit: blurring the role fields once they're complete fires the check, and a later blur with unchanged values doesn't fire it again. Also adds unit tests for the pure getJobTitleEligibilityParams (slug field gate, incomplete/invalid fields, a hidden param field still allowing the others through, job_title inclusion). Adds a minimal, purpose-built contract_details fixture (following the v1-italy-apl.ts "trimmed down" convention) with just the four fields the check cares about, rather than driving the full multi-hundred-field country schemas through this scenario. The real dedup bug this uncovered (paramsChanged never actually true) is fixed upstream in the parent branch, not in this commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bugbot flagged that the second test's final assertion ran right after user.tab() resolves, but checkJobTitleEligibility is fire-and-forget from OnboardingForm's onBlur - it awaits handleValidation, parseFormValues, and the request itself, none of which user.tab() waits on. A regression that fires a second request wouldn't have reached the spy by assertion time, so the test would have passed even with a broken dedup. Verified by temporarily disabling both dedup layers (forcing paramsChanged true and setting staleTime: 0): without this fix the test still passed; with it, it correctly fails and catches the regression within the 200ms window. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The waitFor + .catch pattern for asserting an async side effect does not eventually happen is unusual enough to warrant a why-comment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…r a failure A failed check must be retried on the next blur even when the role answers are unchanged. Depends on the fix in #1412; fails until it lands on the base branch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…uest Same change as #1412, so this PR's retry test passes on its own. The hunk is identical, so merging both is clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… runs Track in-flight checks with a pending counter so the button is disabled from validation through the request settling, including the blur fired by clicking submit itself. Exposed as isCheckingJobTitleEligibility on the bag for custom UIs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
📦 Bundle Size Report
Size Limits
Largest Files (Top 5)
View All Files (291 total)
✅ Bundle size check passed |
|
Deploy preview for remote-flows ready!
Deployed with vercel-action |
|
Deploy preview for adp-cost-calculator ready!
Deployed with vercel-action |
Only disable submit while the eligibility request is actually in flight, not while the form validates. A previous request is already aborted when params change, so the query for the current key is all we need to watch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… tests The mocked bag lacked isCheckingJobTitleEligibility, so disabled resolved to undefined instead of false. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
📊 Coverage Report✅ Coverage increased! 🎉
Detailed BreakdownLines Coverage
Statements Coverage
Functions Coverage
Branches Coverage
✅ Coverage check passed |
…oader warning (#1394) The example package is "type": "commonjs" (dev_server.js and api/ use require), so Vite flagged ESM syntax in vite.config.ts as unsupported by the upcoming native config loader. The .mts extension marks just the config as ESM without flipping the whole package. Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…ng (#1389) fakeValueFor() hardcoded [true] for checkbox fields, mimicking a jsonType-array shape that doesn't match how the real form submits const-based checkboxes (e.g. FRA's ack_non_eligible_job_titles, a jsonType string with const: "acknowledged"). parseFormValuesToAPI swaps the RHF boolean for field.const at submit time; mirror that here instead of sending the raw fill value, or the API 422s with a type mismatch. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* feat(scripts): seed onboarding directly against a gateway env
Lets seed-onboarding.mjs bypass the local example dev server via --env=<name>,
reusing example/api/{utils,get_token,proxy}.js for auth against .env.<env> at
the repo root. Prints a ready-to-click app link (?employmentId= prefilled,
via VITE_APP_URL) when set. Onboarding.tsx now reads employmentId from the
URL query string so that link actually prefills the intro form.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs(skills): add verify-sandbox-deployed-app skill
Captures the manual sandbox-verification loop (seed via --env=sandbox, log
past the Vercel password gate with VITE_APP_PASSWORD, drive the flow, check
the claim) as a repeatable skill instead of re-deriving it each time.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* style: fix oxfmt formatting in verify-sandbox-deployed-app skill
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs(skills): remove verify-sandbox-deployed-app skill
Split into its own PR (#1379) so it can be reviewed independently of
the seed-onboarding env flag.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* test(e2e): give sandbox onboarding specs room for slow backend writes
The basic and Germany onboarding specs create real employments against
the sandbox and ran against Playwright's 30s test / 5s expect defaults.
Traces from PR #1371 and main show nothing hanging: Germany reached the
final ILA sign click at 27.2s, leaving <3s for the sign POST, and Spain
failed step-title assertions while ~3s employment writes were in flight.
- test.slow() on both onboarding specs (90s budget)
- expectOnboardingStep helper with a 15s timeout for step transitions,
which are gated on sandbox writes rather than rendering
- wait on the ILA sign POST and assert it succeeded before checking the
dialog closed, so a backend rejection reports as such
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
* test(e2e): gate onboarding steps on save responses, set timeouts globally
Replaces the per-spec test.slow() and the 15s step-title timeout with:
- submitAndWaitForSave / clickAndWaitForSave helpers that wait for the
request each step's submit fires (POST /v1/employments, POST
engagement-agreement-details, PATCH /v1/employments/{id}, PUT
benefit-offers, POST .../sign) and assert it succeeded, instead of
waiting for the "Loading..." text to disappear. A backend failure now
reports method, path and status.
- Global timeout (60s) and expect timeout (10s) in playwright.config.ts,
matching Dragon's defaults. Response waits make failures precise but do
not make the sandbox faster; the Germany flow alone takes ~30s on a
good day, so the 30s default budget has to go up regardless.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* docs: surface the inline-comment convention in CLAUDE.md The comment-density rule (zero by default, one line max for a genuinely non-obvious why) only lived in the untracked CLAUDE.local.md and in .cursor/BUGBOT.md, which Cursor's Bugbot reads for its own reviews but Claude Code doesn't load as project instructions. Add a pointer in CLAUDE.md's Conventions section so it's visible up front instead of surfacing after the fact via a bot comment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: describe what a good inline comment looks like Extends the opt-in comment rule with what "good" actually means when a comment does get written (the rare non-obvious-why case, or on explicit request): plain English a newcomer can follow, not jargon-dense shorthand - even if that takes a couple of sentences. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs: exempt human-requested comments from the one-line cap The one-line cap exists to stop unprompted, jargon-dense comments - it's not meant to force a human-requested explanation into an unreadable single line. Carves out that exception in both CLAUDE.md and .cursor/BUGBOT.md: a comment explicitly asked for can take as many plain sentences as it needs; one added on the AI's own initiative for the non-obvious-why case stays capped at one line, and shouldn't exist at all if the code it's next to is actually simple. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
React first, then external packages, then @remoteoss/* packages, then absolute (@/) imports, then relative imports. Not enforced by oxlint or oxfmt today (verified: import/order is not a supported oxlint rule and oxfmt does not reorder imports) — this documents the existing manual convention so it's explicit for humans and Claude Code. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
… to v1.1.2 (#1409) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…#1403) Dragon hard-gates its Invite button on employment.status !== 'job_title_review' (SendInvitation.jsx); we didn't. Tiger already rejects the invite call server-side while in this status (409 'Can't invite employee in status job_title_review' on the partner API, 422 on the internal one), so this closes the client-side gap rather than introducing new blocking behavior. job_title_review is independent of the (still unmerged) job title eligibility check feature flag: it's part of the generated Employment['status'] type already, and Tiger can put an employment into this status any time its stage-2 AI check needs human review, so the SDK should respect it regardless of whether a partner has opted into that feature. Addresses checklist item 1 in JOB_TITLE_ELIGIBILITY_CHECKLIST.md (on feat/onboarding-job-title-eligibility-check). Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
* chore: ignore CLAUDE.local.md Adds an explicit .gitignore entry for CLAUDE.local.md, a local-only companion to CLAUDE.md for per-developer notes that shouldn't be checked in. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * chore: copy CLAUDE.local.md into new worktrees CLAUDE.local.md is gitignored, so a plain `git worktree add` doesn't carry it over and Claude Code loses that context in worktree sessions. Copy it the same way example/.env is already seeded. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(scripts): copy root .env.<env> files into new worktrees npm run seed:onboarding -- --env=<name> reads .env.<name> from the repo root, but create-worktree.ts only ever copied example/.env - a fresh worktree silently lost access to gateway-targeting credentials like .env.sandbox even though it's a root-level, not example/-level, file. Mirrors the CLAUDE.local.md copy step just above it: glob root for any .env.* file and copy each one into the new worktree. Also gitignores .env.* (previously only .env itself was ignored), since these files carry real credentials and weren't excluded from `git add -A`. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…e-job' into found-1869-job-title-eligibility-tests
…-disable-submit-during-eligibility-check
…e-job' into found-1869-job-title-eligibility-tests
…-disable-submit-during-eligibility-check
…e-job' into found-1869-job-title-eligibility-tests
…-disable-submit-during-eligibility-check
…ract details The eligibility request can still be in flight after the user leaves contract_details, which kept isCheckingJobTitleEligibility true and disabled Continue on the next step until the request settled. Gate it on the current step, matching how the query itself is enabled. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…bmit-during-eligibility-check # Conflicts: # src/flows/Onboarding/hooks.tsx # src/flows/Onboarding/hooks/useJobTitleEligibilityCheck.tsx # src/flows/Onboarding/tests/OnboardingFlowJobTitleEligibility.test.tsx
…k is superseded (#1415) * fix(onboarding): don't log an error when a job title eligibility check is superseded When the role fields change while a check is in flight, React Query aborts the previous request. The imperative query in check() rejected with a CancelledError, which the catch reported as "Failed to fetch job title eligibility check" even though nothing failed. Skip the log for cancellations. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(onboarding): replace deprecated isCancelledError with instanceof CancelledError Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat(onboarding): set job title eligibility result into form (#1417) * wip set values in form * fix(onboarding): skip legacy contract details schema fetch for jsf v1 countries When an employment id is present, isContractDetailsEnabled is true regardless of the country's schema version, so jsf v1 countries fetched contract_details twice: once through the v0 useJSONSchema path (result unused) and once through useJsfV1ContractDetails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(onboarding): drop a job title eligibility result superseded by newer values When two checks fire before React re-renders (reachable from a headless consumer calling checkJobTitleEligibility back to back), the older query never gets an observer, so React Query doesn't abort it and its result used to overwrite the newer, cleared eligibility values. check() now resolves with undefined when the params it sent are no longer the latest. In the prebuilt form the re-render between blurs already aborts the stale request; the new UI test pins that behaviour. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(onboarding): ignore a failed job title eligibility check superseded by newer values The stale-result guard only covered the success path. A superseded request that failed still resolved with cleared values, wiping the newer check's verdict, and logged an error for a result nobody was waiting on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * revert(onboarding): move the superseded job title eligibility guard to its own PR checkJobTitleEligibility is only called from the prebuilt form's onBlur, where React Query already aborts a superseded check. The guard for back-to-back calls now lives in a separate PR to decide on later. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs(onboarding): scope checkJobTitleEligibility to the contract details form's blur handler Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(onboarding): let the eligibility verdict reveal employer_acknowledges_risk The contract details schema already hides employer_acknowledges_risk unless additional_job_title_eligibility_check_result is yes_with_ack, so the SDK-level hidden override only ever took effect in that one state, which is exactly when the employer has to see and accept it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(onboarding): hide employer_acknowledges_risk until the check returns yes_with_ack The gateway serves employer_acknowledges_risk to API partners as a plain optional checkbox and withholds the server-owned additional_job_title_eligibility_check_result field along with its yes_with_ack conditional, leaving it to the client to decide when to show the acknowledgement (see tiger's AdditionalJobTitleEligibilityCheck component). Confirmed against local tiger for DEU, ITA and PRT with employment-scoped contract_details requests. The contract details schema now gets the result field and that conditional added before the form is built, whenever it carries the check slug and the acknowledgement but not the result. The checkbox stays hidden when no check has run (including with the job_title_eligibility feature off) and appears only once the verdict is eligible_with_risk_acknowledgement. The job title eligibility fixture now mirrors the served shape (no result field, no conditional, Role Requirements fieldset, additionalProperties false), so the tests exercise the SDK's injection rather than a schema the backend never serves. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(onboarding): run job title eligibility check on submit (#1469) * fix(onboarding): run job title eligibility check on submit Submitting without a blur (pressing Enter, or a click that lands before the blur-triggered check flips isCheckingJobTitleEligibility) sent contract details without the eligibility slug and result. The prebuilt form now re-runs the check right before submitting and merges its values into the payload; the query cache makes this free when the blur already fetched the same params. If the verdict changed, the values are written and the submit is held so the user sees any risk acknowledgement it now requires. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(onboarding): only hold submit when the new verdict invalidates the form Holding every submit whose verdict changed made users click Continue twice even when the new verdict required nothing new (e.g. needs_review). Revalidate after writing the new verdict instead, so the submit is only stopped when the new verdict makes fields required that aren't filled, such as the risk acknowledgement for yes_with_ack. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * chore(onboarding): remove debug log Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * refactor(onboarding): name the revalidation after a verdict change Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * trim comments --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 9dce6e3. Configure here.
| return; | ||
| } | ||
| } | ||
| await onSubmit({ ...values, ...eligibilityValues }); |
There was a problem hiding this comment.
Stale ack blocks later submit
Medium Severity
A submit that re-runs the eligibility check can keep employer_acknowledges_risk in form state after the verdict leaves yes_with_ack. handleSubmit then re-validates those leftover values against a schema that forbids the field (additionalProperties: false plus the injected else branch), so the native Enter submit path can fail closed even though the new verdict does not need an acknowledgement.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 9dce6e3. Configure here.


Summary
On the contract details step, the Continue button stays disabled while the job title eligibility check is running, so users can't submit before the check finishes.
Why
The check runs on blur, and until now the submit button stayed clickable while it was in flight. That let users submit before the result came back. The button now stays disabled from the moment a check starts until it settles, whether it succeeds or fails.
What changed
Toggle details
isFetching, so only while the eligibility request is actually in flight. Blurs with incomplete or invalid role fields never disable it. Overlapping checks need no extra tracking: when the values change, React Query aborts the previous request (the query function passessignalthrough), so the query for the current key is the only one that matters.onboardingBag.isCheckingJobTitleEligibility, so headless/custom-UI consumers can do the same thing. I deliberately kept it out ofisSubmitting, which stays "a mutation is pending".OnboardingSubmitis disabled whenisCheckingJobTitleEligibilityis true.SaveDraftButtonis unchanged: saving a draft doesn't depend on the eligibility result.OnboardingFlowJobTitleEligibility.test.tsx. One holds the MSW response open, asserts the button is disabled, releases it, and asserts it's enabled again. The other checks the button re-enables after a 500. I confirmed the first test fails without theOnboardingSubmitchange.OnboardingSubmit.test.tsxalso getsisCheckingJobTitleEligibilityin its mocked bag and a unit test for the new disabled case.Screenshots
N/A
Related Resources
Testing
example/app in a browsernpm test— 1023 passedjob_title_eligibility🤖 Generated with Claude Code
Note
Medium Risk
Changes contract-details validation and PATCH payloads when
job_title_eligibilityis enabled, including conditional required fields and submit timing; mistakes could block legitimate submits or omit check metadata.Overview
Strengthens the job title eligibility flow on contract details so submissions reflect the latest check and users cannot continue while a request is in flight.
Submit UX:
OnboardingSubmitstays disabled whenonboardingBag.isCheckingJobTitleEligibilityis true (new public flag on the bag). The eligibility hook now returns mapped slug/result values, treatsCancelledErroras benign, and only reports fetching on the contract details step.Schema & form: Contract details schemas are passed through
withJobTitleEligibilityRiskAcknowledgement, which injects the hiddenadditional_job_title_eligibility_check_resultfield and JSON Schema conditionals soemployer_acknowledges_riskis required only when the result isyes_with_ack(replacing the previous always-hidden override inhooks.tsx).OnboardingFormsyncs check results into react-hook-form on blur and again before submit; if the verdict changes, it re-triggers validation and can block submit until the risk acknowledgement is satisfied. Payloads include the mapped check slug/result viagetJobTitleEligibilityValues.Tests: Integration coverage expands for JSF v0/v1, in-flight disable/abort, submit-without-blur, and risk-ack show/hide/submit behavior.
Reviewed by Cursor Bugbot for commit 9dce6e3. Bugbot is set up for automated code reviews on this repo. Configure here.