Skip to content

Check-in gated borrowing, deploy cookie/TLS fixes, main sync, and the first CI gate - #1

Open
Shaan-Shoukath wants to merge 6 commits into
mainfrom
dev
Open

Shaan-Shoukath wants to merge 6 commits into
mainfrom
dev

Conversation

@Shaan-Shoukath

Copy link
Copy Markdown
Collaborator

Brings dev level with main and lands three batches that had never been merged.

What's here

Deploy / auth cookies (56b63763)
The staff refresh cookie defaulted to SameSite=None; Secure, which a browser silently drops on a plain-HTTP origin — staff logged in, then every reload signed them out, with nothing in the logs. setup.sh now writes Lax/False for the topology it configures and .env.example declares them. Existing .env files are backfilled only when the file itself proves plain-HTTP same-origin: a split-origin HTTPS deployment legitimately needs None/Secure and must not be rewritten.

The TLS overlay forces Secure back on, serves object storage through a Caddy STORAGE_DOMAIN site, and unpublishes MinIO's plaintext 0.0.0.0:9000 — so STORAGE_DOMAIN is now required rather than defaulted, otherwise a TLS deployment would have no reachable storage at all. Caddy is pinned to a patch release, since update.sh pulls only the application images.

First CI gate (56b63763)
Nothing ran tests before: security-audit.yml only runs pip-audit on PRs, and release.yml builds and publishes on every push to main. A red branch could merge and publish images. ci.yml adds the backend suite, tsc, vitest, npm run build, makemigrations --check, and a diff CLAUDE.md AGENTS.md guard.

Merge of main (4306d17f)
27 commits — events, organizations, report rollups, evidence retention — beside the reinstated check-in programme. Several conflicts could not be resolved by choosing a side; see the merge commit message. The catalog digest was recomputed from the merged graph, because neither branch's pinned value describes the union and validate_catalog() fails closed on a stale one. No app has two migration leaves, so no merge migration is required.

Review findings (f45d9df0)
Three defects the review gate found in the check-in batch, none introduced by the merge: a payment-regime resolution that could roll back a valid machine completion, a handover console that stranded collect-only desk roles behind a pending payment, and a check-in lookup that republished another upstream space's roster entries. Plus two consumable-pool routes that were unreachable from verified tenant custom domains.

Verification

  • Backend: 6113 passed, 22 failed — all 22 are the known ROOT-resolution failures that can only pass from a host checkout. CI runs one, so this PR is the first real test of whether they are genuinely environmental.
  • Frontend: 263/263, tsc -b clean, npm run build succeeds.
  • makemigrations --check clean; diff CLAUDE.md AGENTS.md empty.

Worth a look before merging

legacy:v1 frozen-route count moved 50 → 61. Each new public event/organization route carries an explicit True in _ROUTE_DEFINITIONS, so the extension is declared per route rather than inherited — but it does mean existing legacy API clients gained reach over 11 new endpoints. That is main's shipped decision, preserved rather than reversed.

🤖 Generated with Claude Code

Shaan-Shoukath and others added 5 commits September 9, 2026 16:59
Co-Authored-By: Shaan-Shoukath <shaanshoukath4522@gmail.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Codex GPT-5.6 <noreply@openai.com>
Co-Authored-By: Shaan-Shoukath <shaanshoukath4522@gmail.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Codex GPT-5.6 <noreply@openai.com>
…, CI gate

The refresh cookie defaulted to SameSite=None + Secure, which a browser silently
drops on a plain-HTTP origin: staff logged in, then every reload signed them out
with nothing in the logs. setup.sh now writes Lax/False for the topology it
configures, and .env.example declares them; existing .env files are backfilled
ONLY when the file itself proves plain-HTTP same-origin, because a split-origin
HTTPS deployment legitimately needs None/Secure and must not be rewritten.

The TLS overlay forces Secure back on, serves object storage through a Caddy
STORAGE_DOMAIN site, and unpublishes MinIO's plaintext 0.0.0.0:9000 -- so
STORAGE_DOMAIN is now required rather than defaulted, or a TLS deployment would
have no reachable storage at all. Caddy is pinned to a patch release, since
update.sh pulls only the application images.

Adds the first workflow that runs tests: nothing verified a PR before, so a red
branch could merge and release.yml would publish images from it.

Co-Authored-By: Shaan-Shoukath <shaanshoukath4522@gmail.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Codex GPT-5.6 <noreply@openai.com>
…heck-in

dev carried the reinstated check-in programme; main carried 27 commits of events
(series, station check-in, ICS, certificates), organizations, report rollups and
evidence retention. Neither side's tree described the union, so several conflicts
could not be resolved by choosing a side:

- machines public service/printer views: kept dev's check-in branch but call main's
  require_public_machine_requester inside it, so verified walk-ins still submit AND
  the surface stays reachable on profiles that ship machine_service without
  membership. Taking either side alone silently dropped one of the two.
- lib/api.ts: kept dev's re-export barrel and ported main's tenantPublicRequestBlob
  and memberRequestBlob into apiRequests.ts rather than restoring the monolith.
- PublicRequestPanel: kept BOTH readiness gates; they guard different policies.
- references_json_fields.py was an add/add; took main's JSON_REFERENCE_FIELDS name
  with dev's docstring and dropped the import dev's side left dangling.
- CATALOG_SCHEMA_SHA256 recomputed from the merged graph. Neither branch's pinned
  digest describes the union, and validate_catalog() fails closed on a stale one.
- Count assertions summed rather than chosen (encryption mappers 20 -> 22, claim
  routes -> 86, legacy:v1 frozen 50 -> 61; the last is main's per-route declaration,
  not an inherited default, and the excluded count stays 1).

Two test suites needed porting because main was written against fields dev had
already replaced: test_offstate_identity used anonymous_requests_enabled, removed in
migration 0068 for public_request_mode, and the check-in machine-service fixture
predates main's printing module gate, which runs before authentication.

OpenAPI schema and the generated client are regenerated, not hand-merged. No app has
two migration leaves, so no merge migration is required.

Co-Authored-By: Shaan-Shoukath <shaanshoukath4522@gmail.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ol routes

Three defects the review gate found in the check-in/counter-settlement batch, none
introduced by the merge:

- complete() resolved the payment regime BEFORE saving the completed status and
  outside every error boundary, so a failure reading payment settings rolled back an
  otherwise valid machine completion. Never-block says completion survives payment
  failure. It now runs after the save inside a boundary, and on failure falls through
  to create_for_completed_request(), which repeats the same check inside its own
  try/atomic and no-ops safely.
- The handover console rendered "Record payment" INSTEAD of "Hand over" whenever a
  payment was pending. Settlement carries Payment's own authority, so a handout-only
  desk role may mark a job collected but is refused 403 on recording cash -- that role
  was stranded until another staff member settled. Both actions now render.
- The check-in name lookup returned roster entries bound to a different upstream
  spaceId, republishing another tenant's mid, display name, avatar, purpose and
  project name. They are discarded now, as the space-binding rule requires.

Also: admin-machine-service-printer-pool-detail and -adjustments are keyed only by
pool pk, unlike the sibling list/create route that carries the makerspace in its path,
so without a MODEL_LOOKUPS entry they resolved as global and were unreachable from a
verified tenant custom domain.

CLAUDE.md/AGENTS.md corrected: scripts/dev-local.sh does not exist and has not for
some time, the real infra ports are 5432/9000, the ceiling inventory is six backend
files (not three) plus twelve frontend ones, and the frontend needs Node 20 because
Node 26's native localStorage collides with jsdom.

Co-Authored-By: Shaan-Shoukath <shaanshoukath4522@gmail.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 19, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

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

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: e96346ce-3065-4278-a3c3-2df572470215


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.

… push

requirements.txt auto-merged WITHOUT a conflict and silently kept dev's `pytest>=8,<9`
over main's `pytest>=9`, so this branch would have downgraded main's pytest and
reintroduced PYSEC-2026-1845. The pip-audit gate caught it; the conflict review could
not, because there was no conflict.

The cap's reason is also stale: it was added when pytest-django 4.12 broke on pytest 9's
changed fixture finalizers, but 4.14 declares `pytest>=7.0.0` with no upper bound. The
full suite on 9.1.1 is 22 failed / 6113 passed -- identical to 8.4.2, and all 22 are the
known host-checkout failures. Lifted rather than suppressed with --ignore-vuln.

CI concurrency is keyed on the branch instead of github.ref: a push to `dev` with its PR
open fires both triggers, whose refs differ, so the whole suite ran twice in parallel.

Co-Authored-By: Shaan-Shoukath <shaanshoukath4522@gmail.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant