Release: main → production (2026-09-02) - #135
Open
corrin wants to merge 20 commits into
Open
Conversation
scripts/ops/extract_v1_credentials.py and its test import cryptography for the v1 Fernet decryption, but only xero-python was pulling it in, so deptry fails CI on main (DEP003) since #112 merged. Main rather than dev group: the extract runs on the production host during cutover phase 0, whose venv is installed with `uv sync --frozen --no-dev`. Claude-Session: https://claude.ai/code/session_01H4YMRsFzJQrHHzBkXHfYFC Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
#113) * Beat's schedule shelve lives in the writable instance root, and the verifier sees crash loops celery beat's PersistentScheduler writes its last-run shelve file to CWD by default — the app symlink into the immutable release dir — so every rendered beat unit crash-looped on permission denied (msm-uat, NRestarts>1900). The unit template now passes --schedule=<instance root>/celerybeat-schedule, the path already live on msm-prod, and the template test pins it. verify-instance.sh reported that broken beat healthy: with Restart=always / RestartSec=10 a crash-looping unit is "active" for a slice of every cycle, so bare is-active passes intermittently. The three runtime-unit checks now also require NRestarts unchanged across a 12s window (longer than every unit's RestartSec), pinned by the template test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H4YMRsFzJQrHHzBkXHfYFC * Stability rechecks span the whole verifier run, and the test pin matches code, not comments The 12s window catches only a fast crash loop, so the runtime units are rechecked against the same NRestarts baselines at the end of verification, stretching the observed window to the whole run; celery has no sd_notify, so a readiness signal was not available. The template test now pins the executable NRestarts comparison rather than any mention of the word, which a comment alone would have satisfied. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H4YMRsFzJQrHHzBkXHfYFC * The stability window and the templates' RestartSec are pinned as a pair The 12s window is sound only while it exceeds every unit's RestartSec, and nothing coupled them: a template could raise RestartSec past the window (or the sleep could vanish) with every gate green. Both sides are now pinned so a change to either forces revisiting the pair together. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H4YMRsFzJQrHHzBkXHfYFC * The beat schedule and verifier crash-loop findings are in the history ledger Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H4YMRsFzJQrHHzBkXHfYFC --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…viction (#120) * Password validators are configured; weak values are a 400 at every set-password surface Django's four standard validators, with the similarity attributes named explicitly because Staff has no username/email attribute for the defaults to find. _set_staff_password was already wired to enforce them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H4YMRsFzJQrHHzBkXHfYFC * Repo rename docketworks_v2 -> docketworks: v1 path and clone URL follow The v1 repo now sits at ../docketworks_v1, so gen_v1_operations.py's V1_REPO and every live doc pointing at ../docketworks (which now resolves to this repo itself) move with it. initial_install.md clones the renamed GitHub repo. DB names stay docketworks_v2 - the database was not renamed. Historical records (cutover checklist, plan docs) untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0164MHjth7CvHoYQDEcZHahA * Self-service password change: POST /api/accounts/me/password/ Verifies the current password (400 on mismatch, ADR 0038 transparent post-auth), routes the new value through _set_staff_password so the validators judge it and password_needs_reset clears. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWTJcCbpQdzWLMVNg1QaLb * password_needs_reset gates at the auth layer, typed 403 as the exit sign While the flag is set, CookieJWTAuth refuses every path outside the /me/ and /me/password/ allowlist with code "password_change_required" (error_id null, no AppError row — expected security outcome, ADR 0013). A frontend redirect the API does not back would leave every endpoint serving a session whose credential we have decided not to trust. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWTJcCbpQdzWLMVNg1QaLb * Admin force-reset control: password_needs_reset on the staff write surfaces StaffCreateIn/StaffUpdateIn carry the "must change at next login" flag; an explicit flag outlives _set_staff_password's clear, so an admin can issue a known temporary password already flagged. UserProfile and StaffListItemOut expose the stored value — the route guard reads /me/, and the edit modal must render the real state or clear it by accident. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWTJcCbpQdzWLMVNg1QaLb * Change-password screen, flagged-session navigation, typed-403 interceptor Login response and /me both carry password_needs_reset; the login page and the authed layout route flagged sessions to /change-password (its own top-level route — nesting under _authed would loop that guard). The transport interceptor hard-navigates on the auth layer's typed 403 so a session flagged mid-flight gets walked to the exit too; the server gate stays the control, all of this is navigation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWTJcCbpQdzWLMVNg1QaLb * Gmail delegated sender in apps/core; credential builders move with it The password-reset email gives application code its first Google call, and apps must never import from scripts — so the one credential builder moves to apps/core/gauth.py (scripts/gdocs re-imports it) and apps/core/gmail.py becomes the one application email sender: plain-text send as the instance's Workspace user via domain-wide delegation with the gmail.send scope, proven by the 2026-08-31 delegation probe. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWTJcCbpQdzWLMVNg1QaLb * Forgot-password flow: anonymous reset request and confirm endpoints POST /password-reset/ answers a fixed 200 whether or not the address has an active account, and emails a uid+token link via the delegated Gmail sender. POST /password-reset/confirm/ exchanges the link for a new password through _set_staff_password (validators run, flag clears); refusals are declared 400 bodies because the envelope masks anonymous exception text and the validator's reason is the response. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWTJcCbpQdzWLMVNg1QaLb * Forgot/reset password screens and the login page's way into them /forgot-password always lands on the same sent-confirmation copy — the server's fixed 200 must not be undone by a chattier client. The emailed link lands on /reset-password, whose uid/token search params normalise to the invalid-link state rather than crashing; a dead link surfaces as the server's 400 detail on submit, since the token is deliberately unverifiable without attempting the change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWTJcCbpQdzWLMVNg1QaLb * E2E spec for the weak-password path; real-send Gmail integration test The spec locks a flagged login to /change-password until a strong password lands, and walks the anonymous forgot/reset paths — the request step submits an account-less address (the fixed 200 sends nothing), so the E2E stack needs no Gmail configuration. The real delegated send is the integration gate's job (ADR 0050), addressed to the delegated subject itself so the probe stays in the instance's own inbox. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWTJcCbpQdzWLMVNg1QaLb * E2E gate green: spec's console allowance is one pattern, status bullet falls Playwright parses a two-element test.use() array whose second entry is an object (a RegExp qualifies) as a [value, options] fixture tuple, so the console filter received a bare string and threw after every step had passed; one combined 400|401 pattern is unambiguous. Both specs are green under run_e2e.sh (12 passed), which is what deletes the weak-password bullet from rewrite-status. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWTJcCbpQdzWLMVNg1QaLb * Review findings: session eviction, either-mailbox reset, queued send, dialog un-flag bug Adversarial-review batch on the branch diff. Every issued token now carries a fingerprint of the password hash (issue_refresh_token is the one mint; stubs/ninja_jwt grows Token.get), checked at authentication and refresh — a change or reset evicts the attacker who knew the old password and holds cookies, this slice's own threat model. The change endpoint re-mints the caller's cookies so changing your own password keeps you signed in; claimless tokens are refused by the same comparison, never grandfathered (ADR 0017). The reset request matches either email column exactly as login does (payroll-only staff could otherwise never reset, silently) and queues the Gmail send — synchronous sending ran only for matched addresses, making latency and a Gmail outage's 500 an account-existence oracle. SSE streams render the typed 403 instead of 500-looping a flagged EventSource. Frontend: a patch carrying a password now always carries the checkbox state — the dirty-only diff silently un-flagged an already-flagged account on a temp-password reissue; the auth screens share one AuthCard/PasswordField/FormAlert instead of three drifting copies, and the typed-403 predicate lives with its siblings in error-message.ts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWTJcCbpQdzWLMVNg1QaLb * Second review round: dev seed defused, same-password refused, pinned link host The high-effort review pass on the full branch plus CodeRabbit, verified finding by finding. The dev/E2E seed no longer flags its staff (the flag now locks sessions, and those logins exist to act AS someone with the printed password); flag_weak_passwords documents that it flags EVERYONE and confines live sessions immediately. Re-entering the current password no longer satisfies a forced change. The change endpoint's refusals are declared 400s (PasswordErrorOut, shared with confirm) so the wire contract carries their shape and no AppError row records an expected refusal. The reset link pins its host to settings.APP_DOMAIN — ALLOWED_HOSTS accepts localhost and USE_X_FORWARDED_HOST let an anonymous caller poison a victim's genuine reset email with a dead link. One definition each for concepts that had grown twins: the login-email match (Staff.objects.sole_login_match, shared by the login backend and the reset request), the fingerprint comparison, and the typed-403 body. The stale-fingerprint refresh no longer clears cookies (a racing 401 could delete the session the changer was just re-minted). /reset-password joins route_reachability's entry list (reached only by the emailed link); the forced change screen gains sign-out and forgot-password exits; the deep link that started a flagged session survives through the forced change. The fleet-wide one-time re-login at deploy is recorded in rewrite-history. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWTJcCbpQdzWLMVNg1QaLb * Reset tests assert the enqueue, not eager Celery's accidental execution CI failed the outbox assertions with the request logging QUEUED and a 200: on the runner, .delay() published to the live redis service instead of running inline, while the identical invocation runs eagerly on a dev box. The repo already knew better — eager .delay() is "a property of the test settings, not of the product" (test_job_files_api.py) — so the endpoint tests now capture .delay's arguments (the queued job IS what the endpoint owes), and the task's own contract (recipient, subject, link, body) gets a direct-call test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWTJcCbpQdzWLMVNg1QaLb * The dropbox importer uses the one credential builder, not its own Written when it was "v2's only Google client" (its own comment), the command carried a private --credentials path and its own delegation checks. That premise ended when the Gmail sender landed: credentials now come from apps/core/gauth.py like every other Google entry point (GCP_CREDENTIALS key file, delegated subject with its fail-loud resolution), leaving from_service_account_file with exactly one call site in apps/. The Drive client construction stays in the command — clients are domain-owned the way gmail.py builds its own; credentials are what must never fork (ADR 0039). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PWTJcCbpQdzWLMVNg1QaLb --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…older root (#122) * Task: attachment thumbnails/click-to-view never ported (prod report 2026-08-31) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wbe63GrSY11WxJm6t2T5sS * Instance setup survives and gates the workflow-folder root The 2026-08-31 incident class: DROPBOX_WORKFLOW_FOLDER pointed one directory above the Job-* tree, so every attachment 404d and new job folders spawned at the Dropbox top level while every daemon reported healthy. Three legs: reconfigure reads the operator's value back instead of reverting it to the empty instance dir, verify-instance.sh gates on every JobFile row resolving on disk, and check_jobfiles names the wrong-root cause first when nothing resolves. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wbe63GrSY11WxJm6t2T5sS * Task: alert on a paused Maestral, not a dead one (22-23 Aug outage) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wbe63GrSY11WxJm6t2T5sS * Review round: quoted env value, one path resolver, ADR 0051 attribution dw-run.sh sources the instance .env, so an unquoted space-containing workflow path (the real MSM value) aborts the source — the template now renders the value quoted, which read_env_value already strips on the reconfigure read-back. check_jobfiles resolves through job_file_full_path so a row the endpoint refuses cannot pass the check, and counts root-escaping rows as failures (the try/except movement in code-quality.md is this catch). Comment adjacency in verify-instance.sh restored, rationale comments carry their Fable: provenance, and the template's .env.example sync rule is honoured. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wbe63GrSY11WxJm6t2T5sS
Bumps [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) from 26.2.0 to 26.4.0. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node) --- updated-dependencies: - dependency-name: "@types/node" dependency-version: 26.4.0 dependency-type: direct:development update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…116) Bumps [@testing-library/user-event](https://github.com/testing-library/user-event) from 14.6.3 to 14.6.6. - [Release notes](https://github.com/testing-library/user-event/releases) - [Changelog](https://github.com/testing-library/user-event/blob/main/CHANGELOG.md) - [Commits](testing-library/user-event@v14.6.3...v14.6.6) --- updated-dependencies: - dependency-name: "@testing-library/user-event" dependency-version: 14.6.6 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [@types/react-dom](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/react-dom) from 19.2.4 to 19.2.5. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/react-dom) --- updated-dependencies: - dependency-name: "@types/react-dom" dependency-version: 19.2.5 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [lucide-react](https://github.com/lucide-icons/lucide/tree/HEAD/packages/lucide-react) from 1.30.0 to 1.34.0. - [Release notes](https://github.com/lucide-icons/lucide/releases) - [Commits](https://github.com/lucide-icons/lucide/commits/1.34.0/packages/lucide-react) --- updated-dependencies: - dependency-name: lucide-react dependency-version: 1.34.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…124) google-api-python-client and google-auth move from the dev group to [project]: apps/core/gmail.py (imported by apps/accounts/tasks.py, so by the Celery worker at boot) and the per-client onboarding command import_dropbox_hs_documents both run in the production uv sync --frozen --no-dev venv — without this the next deploy cannot even start the worker. The DEP004 ignore entries that silenced deptry's gate are removed and the comment now states the true split (only httplib2 remains dev, for the link probe's tests). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Wbe63GrSY11WxJm6t2T5sS
* KAN-346 backend: copy estimate to quote, blank seeds replaced, real quotes archived POST /cost_sets/quote/copy_from_estimate/ copies the estimate's lines onto the quote. A blank quote (every line $0 — the creation seed) is replaced with no revision recorded; a priced quote answers 409 unless archive_existing, which archives-then-copies in one transaction. A quote already matching the estimate answers as a no-op so a double press never stacks identical archives. The creation-time seeding now shares the one _copy_cost_lines implementation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015yGJr7WS9uYzCVWaoBRNrX * KAN-346 frontend: Copy from Estimate button with archive-and-replace dialog One press copies; the server's 409 (priced quote) opens the dialog rather than a toast, so the client never re-derives blank-vs-priced from possibly stale lines. Confirming re-posts with archive_existing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015yGJr7WS9uYzCVWaoBRNrX * KAN-346 E2E gate: the spec, and blank means zero line totals, not zero unit prices The creation seed's time lines carry real wage/charge-out rates at quantity 0, so the unit-price blank test called an untouched seed "priced" and the first copy 409'd — caught only by the spec, since the unit fixture had recreated the seed wrong. Blank is now per-line total_cost/total_rev, which still archives offsetting adjustments. The spec covers all three server answers and whitelists the deliberate 409's browser resource error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015yGJr7WS9uYzCVWaoBRNrX * KAN-349: one cost-set summary panel, on the Estimate, Quote and Actual tabs v1 showed totals on every costing tab; v2's lean rebuild kept them only on Quote. The Quote tab's panel is now the one CostSetSummaryPanel implementation, rendered on all three tabs (Actual keeps its Time & Expenses chip — specs assert on it). Spec gains the estimate-summary and actual-summary assertions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015yGJr7WS9uYzCVWaoBRNrX * KAN-346: old quotes are viewable — the Revisions history dialog Archive-and-replace promised "a revision you can still see"; v2 had no caller of the revise GET, so archives were API-only (v1 had the modal). The Quote tab gains a Revisions button opening the read-only history, and the revise GET's revision entries cross the wire typed instead of as loose dicts, so the generated client carries the real shape (ADR 0028). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015yGJr7WS9uYzCVWaoBRNrX * Review round: quote locked before deciding, copy and comparison share one field list CodeRabbit's finding: blank/equality were judged outside the transaction, so a line created between decision and replace could be bulk-deleted without reaching the archive — reads now happen after select_for_update on the quote CostSet, serializing concurrent copies. The house review's lockstep finding: _copy_cost_lines and _cost_line_contents each named the copied fields, so they could drift — both now derive from _COPIED_LINE_FIELDS, with a named canonical serializer per field. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015yGJr7WS9uYzCVWaoBRNrX --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ialog (#126) Strict mode rejects the unscoped getByText: the tab header also says "Revision 1" (the live CostSet rev, a different number than the archive's). The merged spec fails on this; the green gate run (4/4, run ot5qglkc) already included this scoping. Claude-Session: https://claude.ai/code/session_015yGJr7WS9uYzCVWaoBRNrX Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…sync fix (#127) * Review round 2: acceptance reset, T&M guard, Xero warning, typed conflict, one-pass copy Nine findings from the /code-review pass and its verifiers, on merged main: - The blank path replaced an accepted quote's content without clearing quote_acceptance_date (the archive path always did) — acceptance now clears on any replace, and the dialog says so. - No pricing_methodology guard: a direct API call populated a T&M job's quote, which no UI shows but kanban/pipeline/aging all read. Refused. - Replacing a quote already exported to Xero warned nobody; the archive dialog now names the Xero quote it will leave stale. - The dialog totalled revenue only, so an offsetting-adjustments quote read "$0.00" while inviting a discard. It states both totals. - QuoteNotBlankError subclasses core ConflictError, so the envelope answers 409 from any boundary and the hand-mapped api catch is gone. - _copy_cost_lines was O(n^2) (a full summary recompute per line, inside the row lock): validate all, bulk_create, recompute once. The old docstring's entry_seq rationale was false — entry_seq is actual-kind only. - The copy tests ran on the shared T&M job fixture, a state the endpoint now refuses; the class overrides it with a fixed-price job. - The row-scan/tab-open/add-row E2E helpers existed in three specs — hoisted into helpers.ts and both estimate specs migrated. create-estimate-entry keeps its own addAdjustmentEntry: it asserts keyboard focus order that the shared helper does not. - ADR 0051 prefixes on the AI-originated rationales, and the 409 check goes through the existing isApiErrorStatus helper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015yGJr7WS9uYzCVWaoBRNrX * Instances point .env at the Google key they already hold instance.sh requires GCP_CREDENTIALS, refuses a missing file, and copies the key to <instance>/gcp-credentials.json at chmod 600 — but never wrote that path into the rendered .env, and there was no __GCP_CREDENTIALS__ in the substitution list. Every instance has its service-account key on disk, correctly permissioned, with nothing pointing at it. The template hardcoded it blank under a comment claiming only the scripts/gdocs/ toolchain reads it, "never the app". That stopped being true at 9ede91e: apps/core/gauth.py is app code, and both the Gmail password-reset email and the health-and-safety Drive import go through it. gauth fails loud on an unset key, so the blank value takes those two features out on any instance. Same false claim corrected in .env.example and the server README. The guard is the point. test_server_templates.sh already rendered the template and asserted no __TOKEN__ survived, but it renders through its own substitution list, which can agree with the template while instance.sh does not — and instance.sh is what ships. It now checks every token the template declares against instance.sh's own sed list, so a placeholder the shipping script cannot resolve fails the commit tier rather than the instance. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZsmtNaMn88gWoBvzTNMnc * The hourly pay-run sync persists again, and the tenant comes from the run transform_pay_run takes a keyword-only tenant_id; sync_entities called transform_func(item, xero_id). So every pay run raised TypeError into the per-item handler, was filed as an AppError and skipped, and the entity still yielded "Completed sync of pay_runs" having written nothing. Production logged one per pay run per hour under a green sync. The realistic cost was small — the mirror was already correct and self-repairs on the next posting run — but the safety net that other code cites ("the mirror converges on the next hourly sync anyway") did not exist. The engine's ENTITY_CONFIGS entry was a second implementation of the mirror refresh_pay_runs already owned: same fetch, same orphan delete, same transform. It is collapsed onto refresh_pay_runs' extracted half, sync_pay_runs, so there is one implementation (ADR 0039) — and the engine inherits the tenant-scoped delete. Its own delete was exclude(xero_id__in=fetched) across the whole table, so syncing one organisation dropped another's rows. transform_func: Any becomes the XeroTransform Protocol. That Any is why an arity mismatch reached production through strict mypy: with the contract declared, transform_pay_run's keyword-only argument is a type error at the call site rather than a TypeError swallowed at runtime. sync_entities also loses delete_orphans — its one caller has moved off it, and the docstring records why it must not come back: the function is handed one page and cannot answer whether that page is the whole set, which is the question a delete depends on. Two more sites stop re-reading the tenant singleton. transform_pay_slip and sync_accounts called get_tenant_id() per row, which is exactly what transform_pay_run's docstring forbids: a posting run threads its dispatched organisation through, and a fresh read during the five-minute swap window stamps rows with the other tenant's id. The rule now holds for every synced row: the tenant is the one the run resolved. Tests drive the engine, not the persist functions — the functions were never broken, only the wiring between them, so a test that does not cross that seam cannot fail when it breaks again. Each was mutation-tested: reinstating the generic call reproduces the production TypeError and fails three of them, unscoping the delete fails the isolation test, and restoring the singleton read fails the pay-slip test. Full suite: 2810 passed. The integration suite (ADR 0050) has NOT been run — it needs sandbox credentials. apps/xero/tests/test_payroll_integration.py exercises refresh_pay_runs, which now routes through sync_pay_runs, so that suite covers this change and must pass before merge. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CZsmtNaMn88gWoBvzTNMnc --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
… explicit (#128) Dependabot's #119 bumped @fullcalendar/react alone and could not do more: @fullcalendar/interaction and @fullcalendar/timegrid have no stable v7 on npm (they stop at 7.0.0-rc.0), because v7 folded them into subpath exports of @fullcalendar/react. @fullcalendar/core@7 is now a stub whose entry point logs "should not be imported directly". So the four v6 packages become one, plus temporal-polyfill, which v7 requires as a peer. Two renames the compiler catches (EventContentArg -> EventDisplayInfo, PluginDef -> PluginInput) and one it does not: slotLabelFormat is now slotHeaderFormat. v7 warns on unknown options rather than failing, so a missed rename would have shipped a 12-hour clock face. v7 also injects no styles and picks no theme. The classic theme is the v6-alike of the five on offer; its plugin and CSS are imported in the component rather than main.css so only the one route that mounts a calendar pays for them. Claude-Session: https://claude.ai/code/session_01EooU4XgEvG1cfSem9B5vBC Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Rename annual leave loading to labour cost loading * Name the wage loading spec after labour cost * Migrate the E2E database before cleanup
…131) * fix: guard labour loading recomputation * perf: skip unchanged Xero employee writes * fix: make the Xero employee checksum see the fields it guards The skip was wrong in both directions. Xero quotes rates to four decimals while every money column here is numeric(_, 2), so an employee on 24.0385 could never satisfy the three-way equality: rewritten, with a fresh payroll-term history row, on every hourly sync forever. Rounding now happens at the write in one helper, ROUND_HALF_UP to agree with Postgres rather than Decimal's banker's default, which would disagree with the column at exactly .xx5. wage_rate was in neither projection, so a rate written from a stale loading still matched and the sync skipped the one write that re-derives it, making the KAN-350 damage permanent instead of hour-long. Both sides now carry it, which also makes a loading change reach every employee. Deriving it needed the loading arithmetic in a third place, so the formula that had already diverged between Staff._compute_wage_rate and _recompute_all_staff_wage_rates becomes one core.models.loaded_wage_rate. Naming the projections caught Staff.pay_basis being nullable on the stored side alone. CompanyDefaults.save now freezes update_fields before testing membership: a generator caller was consumed by the test, leaving super().save() an empty update and a silent no-op write. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012KsKjSzg9umSt7ad4NHEg7 * chore: fold in the KAN-351 review findings Six findings landed with #129's squash. The one with teeth is run_e2e.sh. Cleanup imports the current models, so the migrate has to precede Playwright -- which means it also precedes global-setup's pre-test pg_dump, and teardown restores it rather than reverting it. Leaving the dev database at head is the right outcome; the comment now says so, and says the cost: a branch behind head afterwards meets a schema newer than its code. migrate core 0004 was redundant. The script's final no-app migrate reaches head, and nothing between the two lines touches workflow_companydefaults, so it goes and the phone-ciphertext ordering guard re-anchors to core 0003 where it was. The August plan and spec are dated records carrying unchecked steps; renaming the field inside them made them claim they referred to a column that did not exist until yesterday. development_session.md is the forward-looking home and keeps the current name. Also a half-done edit that read "Loaded loaded rate allocates". The labour_cost_loading comment went with the previous commit: its lost 8/6/4/2 breakdown returns with the bereavement leave KAN-351 asked for, cited to the ticket rather than prefixed "Owner:", which is not an ADR 0051 form. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012KsKjSzg9umSt7ad4NHEg7 * refactor: name the checksum projections for the system that owns them The types exist to decide whether Xero's copy of an employee matches ours, and they feed Staff.xero_fields_checksum, so the field name and the type name now agree. _PayrollEmployeeProjection was the alternative, matching the neighbouring PayrollEmployeeSnapshot; those snapshots are named for the domain because they flow through the generic sync registry, which these do not. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012KsKjSzg9umSt7ad4NHEg7 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* The KPI calendar honours the weekend flag, and says so in its payload The calendar skipped Saturday and Sunday unconditionally — v1 did the same, consulting no setting, while `weekend_timesheets_enabled` governed only the timesheet grids. Shops that work weekends had a report that could not see their weekends. The flag now reaches the calendar too. It governs the money, not just the columns. With weekends off a Saturday's cost lines are absent from the month rather than hidden, so gross profit omits them — kept from v1 deliberately, because aggregating them always would restate the gross profit of every month already reported. Enabling the flag is the only thing that ever moves a past month's numbers, and that is a decision someone makes rather than one a deploy makes for them. `weekend_enabled` rides in the response beside the data it describes. A client reading CompanyDefaults on its own request could draw seven columns over a five-day payload whenever the two straddle a settings change. Every working-day counter already derives from the loop, so `working_days`, `elapsed_workdays`, `remaining_workdays` and `elapsed_target` follow with no further change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EooU4XgEvG1cfSem9B5vBC * The month serves its labour profit, like it already served the other two `material_profit` and `adjustment_profit` were on the monthly totals and their labour twin was not, so the one place a page wanted the month's labour profit it would have had to subtract `time_revenue - staff_cost` for itself. That is the shape v1 rotted into: cards reading the server, modals recomputing, and the two free to disagree. Cheaper to serve the third than to trust every future consumer to derive it the same way. Also guards the weekend flag against an N+1. The existing flat-across-a-month guard runs with weekends off, so a lookup added inside the weekend branch would cost nothing where it is measured — and deciding whether a day counts is exactly where someone reaches for get_solo() a second time. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EooU4XgEvG1cfSem9B5vBC * The target variance stops calling itself net profit `net_profit` was gross profit less the daily GP target across the elapsed working days — a variance against target. Nothing in this response measures operating expenses, so no net profit can be computed here at all. The name cost v1 a page. Believing the field, its KPI report added a "Projected Expenses" row to close the arithmetic, built from a different threshold (`gp_green`, not `gp_target`) and a different day count (`working_days`, not `elapsed_workdays`) than the field it was explaining, so the subtraction on screen never balanced. A reader who trusts the name will reach for that row again. Renamed while the field has no consumers: the v2 page that will read it is still unwritten, so this is the cheapest this change will ever be. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EooU4XgEvG1cfSem9B5vBC * Hours display joins the other formatters, and months learn to be formatted `formatHoursDisplay` sat in features/timesheet/hours.ts, which was right while timesheets were its only caller. The KPI calendar shows billable hours in every cell, and a deep import across features would have been the first step to a second copy — the exact divergence lib/format.ts exists to prevent. Parsing stays behind in hours.ts: "1 1/4" is a workshop entry habit, not a display concern, and lib/ has no business knowing about it. The round-trip between the two now runs through both modules, asserted where the parser lives. Neither file's own tests would notice a display change the parser cannot read back, and that pairing is what the hours input depends on. `formatWholeCurrency` is a second named export rather than an option on formatCurrency, so a call site cannot quietly pick a precision its neighbours did not; a report choosing it uses it everywhere it shows money. `formatMonth` takes en-NZ's own abbreviation rather than a hardcoded table, so 'Sept 2026' agrees with formatDate's '02 Sept 2026' by construction. The test asserts that agreement instead of the literal, which is the property that actually matters. `isIsoMonthString` bounds the year where the server does, so a hand-edited URL falls back to this month instead of rendering a 422. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EooU4XgEvG1cfSem9B5vBC * Weekends earn without being charged, and net profit gets its name back The daily GP target is the month's operating expense amortised over its weekdays — about $20,000 a month over ~20 weekdays, so ~$1,000 a weekday and nothing at the weekend, because working a weekend is abnormal. The overhead is already fully spread across the weekdays; a Saturday is owed no share of it. So the target and the average GP now count weekdays, never the days the calendar happens to show. Weekend gross profit still counts toward the month in full — it is simply bonus, beating the target rather than raising it. Before this, turning on weekend columns raised a shop's monthly overhead by a third and cut its average GP by a quarter, purely for displaying empty cells. A test now pins every money figure across the flag. `net_profit` is restored: gross profit less the overhead incurred IS the net profit, and the earlier rename rested on my mistaken claim that no expense figure was in the response. The target is that figure. v1's error was never the name — it was the "Projected Expenses" row built to explain the field from the wrong threshold and the wrong day count, so the arithmetic on screen never reached the number beneath it. Averages now name their divisor: avg_weekday_gp over weekdays, avg_active_day_gp and avg_active_day_billable_hours over days that carried hours. "So far" said neither. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EooU4XgEvG1cfSem9B5vBC * Each day ships both ladders, so the report can offer the choice The calendar tinted days by billable hours only. That is the right default and v1 chose it deliberately — dollars are the goal but a noisy daily signal, while billed hours lead them; bill enough hours and the money follows. It is not, however, the only view worth having, and the report is getting a target selector at the top. So a day now carries `color_hours` and `color_gp`, named to match the monthly totals that already used both. The gross-profit ladder was computed here already and thrown away, kept only as a tally — serving it costs nothing and means the browser never re-derives a ladder that could drift from the one the tallies use. Both ladders are now single implementations that the day entry and the month counters share. A day with few hours and a fat material margin is red by hours and green by dollars. That case is now a test, because it is the entire reason both ship. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EooU4XgEvG1cfSem9B5vBC * Review fixes: one spelling, two guards, three doubled imports `labour_profit` on the monthly totals had put both spellings of one concept in a single response body, beside v1's `labor_profit` in the daily breakdown. A client reconciling the day against the month needed two names for the same thing. Aligned while nothing consumes either. `shiftMonth` accepted a full YYYY-MM-DD and silently dropped the day, which `shiftDate` eleven lines below does take — so the likely mistake returned a plausible wrong answer instead of failing. `formatMonth` threw Intl's bare "Invalid time value" from inside a render. Both now run isIsoMonthString, which existed for exactly this and was not being used. Promoting formatHoursDisplay left three files importing @/lib/format twice; oxlint does not flag it, so nothing would have. Rationale carries its author per ADR 0051: `Opus:` where the judgement is mine, "owner ruling" with its date where it is not. The GP-target and weekend reasoning stays in the module docstring, at the code it constrains. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EooU4XgEvG1cfSem9B5vBC * A weekend is a category, not a red day and not a missing one An untouched Saturday was graded against the weekday overhead and came back red — a failure to clear a bar it was never set behind. $0 earned against $0 owed is neither good nor bad. Weekends are now ungraded: they report the fourth category, `weekend`, and the cell draws blank. A category rather than a null, so a consumer's switch is exhaustive and no branch has to ask whether a colour went missing. `gp_target_achievement` IS null there, because a weekend is owed no target and the percentage genuinely has no denominator — a real absence beside a fake one. The monthly colours stay the three-rung ladder: a month is never "weekend". The day tallies follow. `days_red`, `labour_red_days` and `profit_red_days` counted every empty weekend cell, so turning the columns on moved a shop with twenty good weekdays from two red days to ten with no change in work done — the same display-setting-must-not-raise-the-bar reasoning that already keeps the target on weekdays. Scoring and money aggregation are now separate passes: `_score_day` grades weekdays, `_tally_money` counts all seven days. Which is the other half. Weekend cost lines now always reach the monthly totals, whatever the flag says. A Saturday stock issue is real money that counts in WIP and job costing, so a KPI month that dropped it disagreed with those reports about the same job. The flag governs which cells are drawn, never which money is counted — with weekends off the day simply has no cell. This restates historical KPI months, which is the intended correction. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EooU4XgEvG1cfSem9B5vBC * Ledger the three KPI behaviour changes Weekend money reaching the monthly totals is the one that restates history, so it says so. The other two record what a reader would otherwise rediscover and "fix" back: that weekends are ungraded on purpose, and that a day carries two ladders because hours lead dollars. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EooU4XgEvG1cfSem9B5vBC * fix: the KPI month's averages count the days their money came from Six review findings on the KPI backend. The major one: _tally_money folds all seven days into gross_profit and billable_hours, but active_workdays was incremented inside _score_day, which weekends never reach. avg_active_day_gp and avg_active_day_billable_hours therefore put a seven-day numerator over a five-day count, and the month's own colour is read off those averages -- twenty amber weekdays plus four busy Saturdays graded the month green with no green day in it. The count moves to the day loop beside the money, ungated by the display flag for the same reason the money is, and becomes active_days: it is the divisor of avg_active_day_*, and a worked Saturday is a day that carried hours. working_days and elapsed_workdays count drawn cells, so remaining_workdays swings eight days when the flag flips and overstates the overhead still to cover by about a third. remaining_weekdays is its weekday twin and the one target arithmetic may multiply. The module docstring claimed weekend cost lines are absent from the month with the flag off -- the opposite of _tally_money and of the behaviour ledger -- and that the day colours count every working day, when they count weekdays and so sum to weekdays rather than working_days. Both corrected. gp_target_achievement's null is not the weekend signal: an install with no target configured gets one too. The comment now says so and points at color_hours/color_gp, which carry weekend as an explicit category. shiftMonth guarded its input against isIsoMonthString's 2000-2100 bound but not its output, so a prev-month control one click past the boundary handed formatMonth a value it throws on. Refused rather than clamped: returning the same month would leave the control looking enabled and doing nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012KsKjSzg9umSt7ad4NHEg7 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ns (#132) * feat: look the shop's address up through Places, which knows its region Address Validation does not return a region for New Zealand. Measured against six real addresses across four regions on 2026-09-02: no administrative_area_level_1 comes back for any of them, so the entry mapping it to `state` in _COMPONENT_FIELDS has never fired here -- which is why 513 of 522 SupplierPickupAddress rows have a NULL state and the few that do not hold v1 residue like 'Address changed 16/01/2012'. So the shop's own address gets a second product. Places (New) returns the region, and takes the key in a header: the classic Geocoding API carries the region too but is GET-only with the key in the query string, which is exactly what the credential-in-URL fable on geocode_address forbids. The supplier autocomplete is untouched and still calls Address Validation, which is the right product there -- it grades what a person typed. look_up_place keeps the whole reply, not the six fields read today. Re-fetching a field already paid for is the failure that guards against. The region maps through the holidays package's own alias table rather than a second copy of it here, after stripping an optional " Region" suffix: Google says "Canterbury Region" but plainly "Auckland". South Canterbury cannot be derived at all -- Timaru answers "Canterbury Region" like Christchurch, while holidays carries South Canterbury separately with its own anniversary day. That limit is a test, not a comment. The unit test's fixture is a captured response, pasted verbatim. The existing hand-written mock has no administrative_area_level_1 in it and was right by accident; nobody noticed for a year. The integration test (ADR 0050) closes the task rewrite-status recorded, and asserts the absent region as a live check, so if Google ever adds it we are told rather than carrying a second call forever. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012KsKjSzg9umSt7ad4NHEg7 * refactor: one address lookup, and it is the one that knows the region Address Validation and Places were two implementations of "look up an address" living side by side, which is the sibling shape the prime rule exists to stop. The Address Validation path is deleted; every caller now goes through Places. That is not a wash, because the two products do not answer the same question. Address Validation returns no administrative_area_level_1 for any New Zealand address, so its administrative_area_level_1 -> state mapping had never fired and 513 of 522 supplier rows carry a NULL state. Moving the shared endpoint to Places means supplier addresses start getting a real region too -- the sweep fills it on rows it has already visited, since there was never a value to preserve. AddressCandidate.state becomes .region, which is what New Zealand calls it and what Google actually returns. The SupplierPickupAddress column keeps its name; renaming a column is not this change. search_places returns a list because the caller is a picker: PO entry offers candidates and a person confirms which address Google found, which is how they see "Mt Wellington" become "St Johns" rather than having it substituted underneath them. fetch_place re-reads one by the id that person picked, so a save can store what was chosen without trusting coordinates posted by a browser. Places components carry a LIST of types where Address Validation carried one string, so the parser indexes by every type a component claims. Reading it as a single value is exactly how a parser ported from the older product returns empty strings in silence. The street line comes from postalAddress.addressLines rather than street_number plus route, which drops the unit: Google writes "3/41 Elizabeth Knox Place" where rebuilding gives "41 Elizabeth Knox Place". The integration test covers both calls live, including that a subpremise's long synthetic id re-fetches like a real place id -- undocumented, so checked rather than assumed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012KsKjSzg9umSt7ad4NHEg7 * feat: give CompanyDefaults the geocode of its own address Six columns: the raw Google reply plus the facts denormalised out of it that something actually reads. Names follow SupplierPickupAddress, which already stores these for supplier addresses -- one vocabulary for one concept, not two. The holidays subdivision is deliberately NOT a column. It is a pure function of `region`, and the mapping belongs to the holidays package: frozen in a column it would keep answering with last year's table after an upgrade renamed a code or added an alias. `region` is Google's fact and is stored; the code is derived at the point of use. The service moves from apps/company to apps/core, because core is where its second consumer lives and core cannot import a domain app. It was only under company because suppliers were its only caller; it depends on no domain app and never did. The contract already carries this exact precedent for apps.ai -- infrastructure used across the app belongs beside core, and putting it above the domains "would force every consumer through a registry seam and invite per-domain LLM clients, which is exactly how v1 ended up with four". A second geocoder in core is the same mistake, and we deleted one of those an hour ago. formatted_address and region are read-only on the settings screen: they record what Google confirmed, not a preference. Nobody should be able to type a region the holiday calendar then disagrees with. The outbound-link probe stopped both new and existing place-id columns being excused as "no verifier yet" -- untrue since fetch_place landed, because re-reading a place by its id IS the check. Classified honestly and the missing probe adapter recorded as a task. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012KsKjSzg9umSt7ad4NHEg7 * feat: pick the shop's address on the settings screen, geocode on save The picker PO entry already uses, mounted on the company address block. The operator sees Google's cleaned version and accepts it -- "Mt Wellington" becomes "St Johns" in front of them rather than underneath them -- and one pick fills street, suburb, city, post code, country and the place id together. The save re-reads the geocode from that id rather than believing coordinates posted back by a browser. The id is the only part of a chosen candidate a client cannot quietly swap for somewhere else, and the re-read is what puts the whole reply in our hands to store. Coordinates in the body are overwritten, which is a test. A place Google no longer knows is a 400 rather than a save that keeps the address and drops its geocode: the two are one fact, and a half-saved pair is exactly the drift that had a stored wage rate disagreeing with the setting it came from. A PATCH touching no address never calls Google at all, so an outage cannot block editing a phone number. google_place_id sits in the company section but renders as nothing. It has to be a section field because buildPatch only sends keys the schema names, and there is no id a person should be typing. formatted_address and region are read-only beside it: they record what Google confirmed. SettingsFieldInputProps gains onApplyMany, required rather than optional so a caller that forgot it fails to compile instead of rendering an address box that silently discards the pick. The three single-field controls take the narrower SettingsFieldControlProps, which is what they always used. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012KsKjSzg9umSt7ad4NHEg7 * fix: CodeRabbit round 1 — JSON boundary, the Any ban, and an empty caption Three accepted, one rejected. _places_call now parses the body itself, so every failure shape leaves as a GeocodingError. response.json() sat outside the try, so malformed JSON escaped raw; and `parsed: dict[str, object]` was a claim nothing checked, so a top-level array would have reached the parsers and surfaced as AttributeError on .get several frames from the thing that went wrong. Both are now refused at the boundary, named, with a test each. The test fixtures dropped their `Any`. CLAUDE.md and ADR 0028 are explicit and I put it there anyway. mypy infers the captured response fine, so no TypedDict: the fixture's whole value is that it is a verbatim capture, and a hand-kept shape beside it would be one more thing to drift from what Google sent. google_place_id drew an empty labelled box. Returning null from the control was not enough -- CompanyDefaultsPage renders the caption span for every field in the grid, so the Company section showed "Google Place Id" with nothing under it. Filtered where the precedent already is, beside WORKING_HOURS_TIME_KEYS, and the control's null branch is gone so one place decides whether a field renders. snapshot.test pins that the id still survives buildPatch, which is the direction that would actually hurt: drop it from the section and the address silently stops being geocoded. Rejected: the rewrite-history entry dated 2026-09-02. CodeRabbit compared against the UTC timestamp of its own review; local is +1200, and every commit on this branch is dated 2026-09-02 local. The date is right. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012KsKjSzg9umSt7ad4NHEg7 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…d gets a column (#133) Two independent defects on one journey, "find job 97537". The search box read the RAW query string (location.searchStr) but wrote through navigate(), i.e. TanStack's stringifier. The default stringifier JSON-quotes any value that parses as JSON, so a job number went out as ?q=%2297537%22 and came back as the literal `"97537"`. Typing 9, pausing past the 300ms debounce, then typing the rest therefore produced `"9"7537` in the box and searched for a string no job carries. Both sides now read the parsed value through one normaliser; a number is coerced rather than dropped, so a hand-typed or shared ?q=97537 filters the board instead of silently rendering an unfiltered one. Archived was never a column in v2 — omitted on a planning brief's unexamined premise, recorded in no ADR — which hid ~95% of the jobs the board knows about (2287 archived against 117 elsewhere), a set that grows nightly as auto_archive_service moves completed jobs into it. Adding it to OFFICE_COLUMN_IDS is the whole lever: the grid, the reconciliation feed and every cache writer already derive from that array. The three special cases built on the old premise go with it, notably updateStatus, which called removeJob and so DELETED a card the user asked to archive. The backend needed no change: it has served the archived column, its label and its diffs all along. Decisions taken with the owner: Archived obeys the staff filter like every other column (an exempt column that stayed full while its neighbours emptied would read as the filter having failed), and it uses the same COLUMN_MAX_JOBS as everything else rather than a second ceiling to reason about. Perf posture: one extra parallel column query on mount, index-served by Index(status, priority), ~31KB gzipped against the 100KB E2E wire guard. The real cost is DOM — 117 to ~317 mounted JobCards, with no virtualization on the board. Nothing on the drag or reconciliation hot path changed. Tests: the unit specs were confirmed to FAIL against the pre-fix code and pass after (the box round-trip, the numeric link, and the archive-from-drawer move). The E2E spec types with pressSequentially rather than fill() — fill() is one atomic change event, which is exactly why the existing kanban specs never saw this. Claude-Session: https://claude.ai/code/session_012KsKjSzg9umSt7ad4NHEg7 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
…story #125 was squash-merged straight to production and never back-merged, and #136 promoted main to production by squash, so the branches carry identical trees (this merge changes no file) while sharing only bf82955 as a merge base. Left alone, every future release PR would diff from bf82955 and re-propose the whole history. Merging rather than resetting production: production is what the prod hosts pull and what prod-2026-08-31-ccb3eefc tags, so its published history stays put and main absorbs it instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AiYiuX5xF8wMXF5gzNCVNZ
…quash docs/release-process.md already required the hotfix back-merge; it said nothing about how the release PR itself is merged, which is what broke the link. #136 was squash-merged, so production carried main's tree with no parent pointing at main and the merge base stayed at bf82955. Stated as a check rather than a warning: merge-base --is-ancestor is the one-command test for whether a promotion was squashed, and the repair named is the back-merge, so nobody reaches for a force-push of a branch the prod hosts pull. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AiYiuX5xF8wMXF5gzNCVNZ
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 freeto 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.
Brings
productionup tomain. After this merges, the two branches are identical.This is a release, not a hotfix — it requires a deliberate
deploy.shrun with downtime. No workflow deploys on a push toproduction;deploy-uat.ymlonly updates a git mirror.Contents — 17 commits, ~103 files, 4 migrations
Production's
ccb3eefwas a squash of #113–#126, which exist on main as individual commits. The genuinely new work is #127–#132:annual_leave_loading→labour_cost_loadingOperator actions REQUIRED before deploying
GCP_CREDENTIALSis blank in existing instance.envfiles anddeploy.shnever re-renders them. Runinstance.sh reconfigure, or hand-edit, or Gmail password-reset and the Drive H&S import stay broken.<instance>.company-defaults.jsonfor the oldannual_leave_loadingkey — it will failloaddataon the next reconfigure.Migration safety
Four migrations: a
RenameField, a help-textAlterField, and nullableAddFields with blank-check constraints. NoRunPython, no backfill, no drops. The rename preserves data.Code and schema must move together. Old code selects
annual_leave_loading, new code selectslabour_cost_loading; either half alone is a hard 500 on nearly every page.deploy.shalready does backup → stop units → switch release → migrate → start, so there is no window of mismatch. Do not attempt a code-only rollback — userollback.sh, which reverses the migrations.Verification
test_live_unchanged_employee_resync_is_a_local_nooppasses — the new Xero employee checksum settles after one sync rather than churning hourly. The first sync after deploy rewrites every Staff row once (checksum is NULL for existing rows); it is inbound-only, no Xero writes. The one failure is missing local phone-provider credentials, unrelated to this release.Known conflicts
This PR conflicts in 15 files, an artifact of the squash: production holds the older version of #123's files and main supersedes them via #127. I verified production is strictly behind main on every conflicted file — the
ConflictErrorrefactor, thebulk_createfix, the T&M guard, the Xero-staleness warning, theGCP_CREDENTIALSsubstitution and its template gates. Resolving every conflict in main's favour loses nothing.Open question for the owner
The release renames the loading field but does not change the stored percentage or reprice the ~19k cost lines the model comment says were mispriced. Deliberate, or a follow-up?
Separately filed and NOT in this release: KAN-354 (pay-run mirror truncation), KAN-355 (the flaky sales-forecast assertion).
🤖 Generated with Claude Code
https://claude.ai/code/session_012KsKjSzg9umSt7ad4NHEg7