Cleanup of SaaS code (#6669)

# Description of Changes
De-AI comments and fix ridiculously indented code
This commit is contained in:
James Brunton
2026-06-16 11:49:13 +01:00
committed by GitHub
parent 42c1cce56d
commit 9a883be697
20 changed files with 79 additions and 99 deletions
@@ -24,6 +24,7 @@ import org.springframework.security.oauth2.jwt.Jwt;
import org.springframework.security.oauth2.jwt.JwtDecoder;
import org.springframework.security.oauth2.jwt.JwtValidators;
import org.springframework.security.oauth2.jwt.NimbusJwtDecoder;
import org.springframework.security.oauth2.server.resource.OAuth2ProtectedResourceMetadata;
import org.springframework.security.oauth2.server.resource.authentication.JwtAuthenticationConverter;
import org.springframework.security.oauth2.server.resource.authentication.JwtGrantedAuthoritiesConverter;
import org.springframework.security.oauth2.server.resource.web.authentication.BearerTokenAuthenticationFilter;
@@ -201,40 +202,9 @@ public class McpSecurityConfig {
.protectedResourceMetadata(
prm ->
prm.protectedResourceMetadataCustomizer(
builder -> {
if (!auth.getResourceId()
.isBlank()) {
builder.resource(
auth
.getResourceId());
}
if (!auth.getIssuerUri()
.isBlank()) {
builder.authorizationServer(
auth
.getIssuerUri());
}
// Only advertise the granular
// tool scopes when we actually
// enforce them. When scopes are
// disabled (e.g. the IdP only
// mints coarse tokens, like
// Supabase), advertising scopes
// the authorization server
// can't
// issue makes spec-compliant
// clients request them and get
// rejected with
// invalid_request.
if (applicationProperties
.getMcp()
.isScopesEnabled()) {
builder.scope(
"mcp.tools.read");
builder.scope(
"mcp.tools.write");
}
}))
builder ->
buildResourceMetadata(
builder, auth)))
.jwt(
jwt ->
jwt.decoder(mcpJwtDecoder)
@@ -243,6 +213,25 @@ public class McpSecurityConfig {
return http.build();
}
/** Populate the RFC 9728 protected-resource metadata document from the configured auth. */
private void buildResourceMetadata(
OAuth2ProtectedResourceMetadata.Builder builder, ApplicationProperties.Mcp.Auth auth) {
if (!auth.getResourceId().isBlank()) {
builder.resource(auth.getResourceId());
}
if (!auth.getIssuerUri().isBlank()) {
builder.authorizationServer(auth.getIssuerUri());
}
// Only advertise the granular tool scopes when we actually enforce them. When scopes are
// disabled (e.g. the IdP only mints coarse tokens, like Supabase), advertising scopes the
// authorization server can't issue makes spec-compliant clients request them and get
// rejected with invalid_request.
if (applicationProperties.getMcp().isScopesEnabled()) {
builder.scope("mcp.tools.read");
builder.scope("mcp.tools.write");
}
}
@Bean
JwtDecoder mcpJwtDecoder() {
ApplicationProperties.Mcp.Auth auth = applicationProperties.getMcp().getAuth();
@@ -303,9 +303,9 @@ public class PaygWalletController {
} else {
long capMinor = CapMoneyUnits.usdToCents(req.capUsd());
policy.setCapSourceMoney(capMinor);
// Derived document allowance (design §10: store both the money intent and the unit
// translation). The live snapshot recomputes from cap_source_money + current rate;
// this stored value is the enforcement fallback when the rate is unreachable.
// Derived document allowance: store both the money intent and the unit translation.
// The live snapshot recomputes from cap_source_money + current rate; this stored value
// is the enforcement fallback when the rate is unreachable.
TeamBillingContext billing = billingService.forTeam(teamId);
Optional<Long> docCap = billingService.docCapForMoney(billing, capMinor);
if (docCap.isPresent()) {
@@ -82,8 +82,7 @@ public record WalletSnapshotResponse(
/**
* One row of the team-members table on the leader's Plan page — display-only per-member usage.
* (Per-member sub-caps aren't enforced yet; see the follow-ups note. When they ship, a cap
* field returns here.)
* (Per-member sub-caps aren't enforced yet. When they ship, a cap field returns here.)
*/
public record MemberRow(String userId, String name, String email, int spendUnits) {}
@@ -12,7 +12,7 @@ import stirling.software.saas.payg.model.FeatureSet;
* effect, and the corresponding enabled {@link FeatureGate}s. No DB access, no caches — the caller
* (entitlement service) supplies the inputs.
*
* <p>State transitions (matching {@code notes/PAYG_DESIGN.md} §3.6):
* <p>State transitions:
*
* <ul>
* <li>{@code capUnits == null} → {@code FULL} / {@link FeatureSet#FULL} unconditionally.
@@ -78,10 +78,7 @@ public final class CapEvaluator {
return full();
}
/**
* Default enabled gates for a given feature set. Kept in sync with the design doc §3.7 mapping
* table.
*/
/** Default enabled gates for a given feature set. */
public static List<FeatureGate> gatesFor(FeatureSet set) {
if (set == null) {
return List.of();
@@ -20,17 +20,17 @@ import lombok.extern.slf4j.Slf4j;
/**
* Read-only accessor for the Stripe Sync Engine schema ({@code stripe.*}). Gives the PAYG layer the
* team's real billing window and the per-document rate of the Price its subscription bills against
* both live in Stripe, mirrored into Postgres by the sync engine, and are NOT duplicated in
* {@code stirling_pdf} (design §10: money lives in Stripe).
* - both live in Stripe, mirrored into Postgres by the sync engine, and are NOT duplicated in
* {@code stirling_pdf} (money lives in Stripe).
*
* <p>PAYG prices are plain {@code per_unit} metered prices, so {@code stripe.prices.unit_amount}
* carries the rate directly. The free grant is deliberately NOT in Stripe it's the one-time
* carries the rate directly. The free grant is deliberately NOT in Stripe - it's the one-time
* {@code pricing_policy.free_tier_units} pool, applied app-side (free units are never metered),
* because un-subscribed teams get the same grant and have no Stripe Price at all.
*
* <p>Defensive by construction: the {@code stripe} schema only exists where the sync engine has run
* (dev + prod Supabase, not unit-test H2). Any {@link DataAccessException} missing schema,
* missing row, connectivity blip degrades to {@link Optional#empty()} with a WARN so callers fall
* (dev + prod Supabase, not unit-test H2). Any {@link DataAccessException} - missing schema,
* missing row, connectivity blip - degrades to {@link Optional#empty()} with a WARN so callers fall
* back to calendar-month windows rather than 500ing the wallet endpoint.
*/
@Slf4j
@@ -60,13 +60,13 @@ public class StripeSubscriptionDao {
BigDecimal perDocMinor) {}
/**
* The per-document rate of a Price looked up directly (not via a subscription) used to price
* The per-document rate of a Price looked up directly (not via a subscription) - used to price
* the cap estimate for un-subscribed teams, whose default policy points at Stripe Prices that
* carry the same {@code unit_amount} they'd be billed at on subscribing.
*
* @param priceId the resolved Stripe Price id
* @param currency lower-case ISO 4217 of that Price
* @param perDocMinor per-document rate in minor units (may be fractional); never null a row
* @param perDocMinor per-document rate in minor units (may be fractional); never null - a row
* with no usable amount is filtered out rather than returned with a null rate
*/
public record PriceRate(String priceId, String currency, BigDecimal perDocMinor) {}
@@ -130,12 +130,12 @@ public class StripeSubscriptionDao {
/**
* Per-document rate of the active {@code stripe.prices} row with the given {@code lookupKey} in
* {@code currency} the elegant mirror of {@link #findBilling}, reading the same synced table
* {@code currency} - the elegant mirror of {@link #findBilling}, reading the same synced table
* by Stripe Price {@code lookup_key} instead of via a subscription. The PAYG layer uses this to
* price the cap estimate for an un-subscribed team: there's no subscription to read a rate off,
* but the PAYG Price (lookup key {@code plan:processor}) carries the very rate they'd be billed
* at. We resolve by lookup_key rather than the default policy's price ids because those aren't
* seeded the lookup key is the stable, env-agnostic handle (same one the price-lookup edge
* seeded - the lookup key is the stable, env-agnostic handle (same one the price-lookup edge
* function uses).
*
* <p>Empty when the {@code stripe} schema is absent (H2 unit tests), no active matching row
@@ -9,8 +9,6 @@
-- restored on a first-step refund). Because the counter is authoritative, the wallet_ledger is
-- no longer the source of truth for the grant and its old rows can be pruned after a retention
-- window (separate future job).
--
-- See notes/payg-lifetime-free-grant-plan-2026-06-11.html.
-- ---------------------------------------------------------------------------------------------
-- 1. Rename the policy column — it is no longer "per cycle", it's the one-time grant size.
@@ -119,8 +119,11 @@ test.describe("1. Authentication and Login", () => {
// Full page reload forces the SPA to re-check auth with the backend
await page.reload({ waitUntil: "domcontentloaded" });
// Step 3: Verify the user is redirected to the login page
await expect(page).toHaveURL(/\/login/, { timeout: 15000 });
// Step 3: Verify the user is redirected to the login page. The redirect
// is driven client-side after the SPA re-bootstraps (session check +
// config fetch + backend probe), which can outrun a 15s budget on a
// loaded CI runner, so allow longer here.
await expect(page).toHaveURL(/\/login/, { timeout: 30000 });
// Step 5: Log in with valid credentials
await page.locator("#email").fill("admin");
@@ -21,9 +21,8 @@ describe("scoreMatch", () => {
});
it("rejects unrelated words that share half their letters", () => {
// Regression: the old fallback accepted 50% relative Levenshtein
// similarity, so longer queries matched more junk (rotate vs update is
// edit distance 3 of 6)
// Unrelated words can share many letters (rotate vs update is edit
// distance 3 of 6) but must not be treated as a fuzzy match.
expect(isFuzzyMatch("rotate", "update")).toBe(false);
expect(isFuzzyMatch("rotate", "create")).toBe(false);
expect(isFuzzyMatch("rotate", "private")).toBe(false);
@@ -20,8 +20,8 @@ function makeEntry(name: string, tags: string): ToolRegistryEntry {
};
}
// Real names and tags from the en-GB translations; these tools previously
// polluted results for partial queries like "rotat" and "rotate"
// Real names and tags from the en-GB translations, chosen because they share
// letters with "rotat"/"rotate" and stress the partial-query filtering.
const registry: Partial<ToolRegistry> = {
rotate: makeEntry(
"Rotate",
@@ -72,9 +72,8 @@ describe("filterToolRegistryByQuery", () => {
});
it("only returns Rotate for every prefix of 'rotate'", () => {
// Regression: "rotat" used to also pull in Redact, Annotate, Compare and
// Adjust Colours/Contrast, and "rotate" additionally Scanner Effect,
// Change Metadata, Add Password and Air-gapped Setup
// Prefixes of "rotate" must not leak tools that merely share letters
// (Redact, Annotate, Compare, Adjust Colours/Contrast, etc.).
expect(idsFor("rota")).toEqual(["rotate"]);
expect(idsFor("rotat")).toEqual(["rotate"]);
expect(idsFor("rotate")).toEqual(["rotate"]);
@@ -158,6 +158,6 @@ export const ListSection: Story = {
),
};
// Note: the setup + edit wizard now embeds the Watch Folders automation builder
// (its Workflow step), which needs the ToolWorkflow context so the wizard is
// The setup + edit wizard embeds the Watch Folders automation builder (its
// Workflow step), which needs the ToolWorkflow context - so the wizard is
// exercised in-app, not via an isolated story here.
@@ -39,8 +39,8 @@ interface PolicyToolConfigStepProps {
initialOperations: AutomationOperation[];
/**
* The preset's default operations. Their params seed any tool whose saved
* value is still at the tool's own default — so e.g. a policy saved before the
* PII patterns existed inherits them rather than running with an empty list.
* value is still at the tool's own default — so e.g. a policy saved with the
* PII list at its default inherits the preset patterns rather than running empty.
*/
presetOperations: AutomationOperation[];
/** Used to name the built pipeline definition. */
@@ -1,5 +1,5 @@
/**
* External store for real backend policy runs (Phase B: auto-run on upload).
* External store for real backend policy runs (auto-run on upload).
*
* The auto-run controller fires a backend run for each enabled policy × each
* newly-uploaded file and records it here; the detail view's activity feed reads
@@ -3,8 +3,6 @@
* config page shows (one section per tool). Tools can be configured + toggled
* on/off, but not added or removed. Each id is a frontend tool-registry key (and
* maps to that tool's backend endpoint via the registry's operationConfig).
*
* Only Security is wired today; other categories follow.
*/
export const POLICY_TOOL_CHAINS: Record<string, string[]> = {
// Security: redact PII + watermark + sanitize (strips JS). Which are enabled
@@ -1,5 +1,5 @@
/**
* Auto-run controller (Phase B): every enabled policy enforces on every uploaded
* Auto-run controller: every enabled policy enforces on every uploaded
* file. Watches the session's files and, for each (active policy × not-yet-run
* file), fires a real backend run (`POST /api/v1/policies/{id}/run`) and polls it
* to completion, recording progress in {@link policyRunStore} for the activity
@@ -5,8 +5,8 @@ describe("policy definitions integrity", () => {
it("every category has a matching config entry", () => {
for (const cat of POLICY_CATEGORIES) {
expect(POLICY_CONFIG[cat.id], `config for ${cat.id}`).toBeDefined();
// A category may have no policy-level setting fields (e.g. Security's were
// all unwired and removed); fields is required but can be empty.
// A category may have no policy-level setting fields; fields is required
// but can be empty.
expect(Array.isArray(POLICY_CONFIG[cat.id].fields)).toBe(true);
expect(POLICY_CONFIG[cat.id].rules.length).toBeGreaterThan(0);
// Every preset seeds a real, non-empty pipeline (the category→steps map).
@@ -24,7 +24,7 @@ import type {
const ICON_SX = { fontSize: "1rem" } as const;
/** The 5 policy categories, in the prototype's narrative order. */
/** The 5 policy categories, in display order. */
export const POLICY_CATEGORIES: PolicyCategory[] = [
{
id: "ingestion",
@@ -69,7 +69,7 @@ export const POLICY_CATEGORIES: PolicyCategory[] = [
* PII presets for the redact step: a label + the regex the /auto-redact endpoint
* matches (via `wordsToRedact` + `useRegex`). Patterns are precise — validated
* (SSN areas, card IINs, ABA prefixes), context- or separator-anchored — to keep
* false positives down and avoid the backtracking the old broad patterns risked.
* false positives down and avoid catastrophic backtracking.
*/
export const PII_PRESETS: { value: string; label: string; pattern: string }[] =
[
@@ -137,7 +137,7 @@ export const DEFAULT_PII_PATTERNS: string[] = [
PII_PRESETS[1].pattern, // cards
];
/** Per-category narrative + editable fields (from the prototype's POLICY_CONFIG). */
/** Per-category narrative + editable fields. */
export const POLICY_CONFIG: Record<string, PolicyConfigDef> = {
ingestion: {
summary:
@@ -149,7 +149,7 @@ export const POLICY_CONFIG: Record<string, PolicyConfigDef> = {
],
scopeLabel: "All PDFs on this device",
// Policy-level controls only — the per-tool params (OCR level, extract
// tables, naming, normalize, rotate…) now live in the Workflow step.
// tables, naming, normalize, rotate...) live in the Workflow step.
fields: [
{
label: "Min confidence",
@@ -29,13 +29,13 @@ function defaultState(): PolicyState {
// Enforce on upload by default; export enforcement is the alternative.
runOn: "upload",
// Every catalog category is a shipped, built-in policy → default (not
// deletable). User-created policies (later) will set this false.
// deletable).
isDefault: true,
};
}
/** An obsolete reviewer email from earlier development, scrubbed from persisted
* state on read so it can re-default to the real signed-in user. */
/** An obsolete reviewer email scrubbed from persisted state on read so it can
* re-default to the real signed-in user. */
const STALE_REVIEWER_EMAIL = "[email protected]";
/** Read the full policy state, seeding + healing any missing categories. */
@@ -4,9 +4,8 @@
* A Policy is conceptually like a Watch Folder (a configured automation that
* runs over documents) but backend-driven and triggered by *sources/events*
* (editor save/export, device sweeps, cloud connectors) rather than just a
* folder. Per-policy state is persisted locally (localStorage) today; server
* persistence and real enforcement land in a follow-up. Activity + stats shown
* in the detail view are derived live from the user's real uploaded files.
* folder. Per-policy state is persisted locally (localStorage). Activity + stats
* shown in the detail view are derived live from the user's real uploaded files.
*/
import type { ReactNode } from "react";
@@ -10,8 +10,8 @@
* - the per-category breakdown comes from {@code categoryBreakdown}
* (wallet_category_summary view)
* - money-equivalent display and the units↔money cap preview need
* stripe.prices via Sync Engine (design §13 / PR-C2) and are deliberately
* absent until that ships
* stripe.prices via Sync Engine and are deliberately absent until that
* ships
* - the activity feed renders {@code wallet.recent}, which is {@code []} in
* V1 — so it shows a real empty state, not fabricated rows
*/
@@ -437,10 +437,9 @@ const GATE_CAP_BEHAVIOR: { gate: Gate; staysAtCap: boolean }[] = [
];
/**
* Folded-in "what happens at the cap" — previously its own GatesCard, now a
* collapsed disclosure rendered inside the cap card(s) to save vertical space.
* Mirrors the {@link DocHelp} toggle pattern; default-collapsed so it costs no
* height until the user opens it.
* Collapsed "what happens at the cap" disclosure rendered inside the cap
* card(s) to save vertical space. Mirrors the {@link DocHelp} toggle pattern;
* default-collapsed so it costs no height until the user opens it.
*/
function CapReachedHelp() {
const { t } = useTranslation();
@@ -488,7 +487,7 @@ function CapReachedHelp() {
/**
* Leader-only roster of each teammate's billable usage this period. Display-only — per-member
* sub-cap enforcement isn't shipped (see the follow-ups note), so there's no cap control here.
* sub-cap enforcement isn't shipped, so there's no cap control here.
*/
function MemberUsage({ members }: { members: Wallet["members"] }) {
const { t } = useTranslation();
@@ -29,12 +29,12 @@
*
* Stripe-touching code lives in Supabase edge functions, not the Java
* backend. This panel invokes {@code create-checkout-session}
* directly via {@code supabase.functions.invoke()} same pattern {@link
* directly via {@code supabase.functions.invoke()} - same pattern {@link
* usePlans} already uses for {@code stripe-price-lookup}. The auth JWT is
* attached automatically by the Supabase client.
*
* <p>The edge function (SaaS PR #300) is the canonical place Stripe Checkout
* Sessions get created it uses the Stripe Sync Engine tables, has dedicated
* <p>The edge function is the canonical place Stripe Checkout
* Sessions get created - it uses the Stripe Sync Engine tables, has dedicated
* unit tests, and shares Stripe SDK / secret-key plumbing with the metering +
* webhook edge functions. Routing through Java would have meant a useless
* proxy hop + a second Stripe SDK to maintain.
@@ -170,10 +170,10 @@ const StripeCheckoutPanel: React.FC<StripeCheckoutPanelProps> = ({
// React 18 strict-mode dev mounts effects twice. We use `cancelled` to
// discard the first mount's response (its setState calls become no-ops),
// and the second mount's response wins. We deliberately do NOT short-
// circuit the second mount with a ref — earlier versions did that and
// ran into the trap where mount 1 was cancelled but the live mount
// never re-fetched, leaving `loading` stuck at true. Two network calls
// in dev is an acceptable cost; prod has no strict mode → single fetch.
// circuit the second mount with a ref: that traps mount 1 as cancelled
// while the live mount never re-fetches, leaving `loading` stuck at true.
// Two network calls in dev is an acceptable cost; prod has no strict mode
// -> single fetch.
let cancelled = false;
async function createSession() {
try {