Compare commits
3 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| ed51a59c80 | |||
| bedeb05a67 | |||
| 58305d7a89 |
@@ -0,0 +1,54 @@
|
||||
# AGENTS.md
|
||||
|
||||
This repository (`groombook/api`) is part of the GroomBook application stack. The
|
||||
authoritative process, quality bar, and safety rules live in the shared
|
||||
[`groombook/org`](https://git.farh.net/groombook/org) skills repository. Read
|
||||
those first; this file is only a pointer.
|
||||
|
||||
## Authoritative skills
|
||||
|
||||
- **SDLC (branching, PRs, phases, handoffs):**
|
||||
[`groombook/org/skills/sdlc/SKILL.md`](https://git.farh.net/groombook/org/src/branch/main/skills/sdlc/SKILL.md)
|
||||
- **Coding standards (priority ordering, PR discipline, tests, no-hardcoded-values, CalVer):**
|
||||
[`groombook/org/skills/coding-standards/SKILL.md`](https://git.farh.net/groombook/org/src/branch/main/skills/coding-standards/SKILL.md)
|
||||
- **Safety (no plaintext secrets, no direct `kubectl apply` to `groombook`, no self-merge, board approval for destructive actions):**
|
||||
[`groombook/org/skills/safety/SKILL.md`](https://git.farh.net/groombook/org/src/branch/main/skills/safety/SKILL.md)
|
||||
|
||||
For human contributors and humans reviewing agent work, see
|
||||
[`CONTRIBUTING.md`](./CONTRIBUTING.md) in this repo for the phase-by-phase PR
|
||||
flow and the `uat→main` merge-gate policy summary.
|
||||
|
||||
## Non-negotiable operational rules
|
||||
|
||||
These mirror the org skills; they are restated here so any agent landing in
|
||||
this repo sees them without a cross-repo fetch.
|
||||
|
||||
- **All changes go through a PR.** Never push directly to `dev`, `uat`, or `main`.
|
||||
- **Branch strategy:** `feature/<name>` → `dev` → `uat` → `main`. Engineers
|
||||
always target `dev` first.
|
||||
- **No self-merge contract.** The engineer who opened a PR clicks merge only
|
||||
after the named reviewer (CI / QA / UAT / Security / CTO per phase)
|
||||
approves. Issue-thread QA / UAT / security approvals do **not** clear the
|
||||
Gitea `required_approvals` gate on `uat→main` — only a Gitea **Approve**
|
||||
click from a member of the `approvals_whitelist_username` does. On this
|
||||
repo that whitelist is `["gb_flea", "gb_dogfather"]` (engineer team).
|
||||
Board-level accounts cannot give the Approve click by policy.
|
||||
- **Always include `cc @cpfarhood`** at the bottom of every PR body for
|
||||
board visibility (not as a reviewer).
|
||||
- **Secrets in code are forbidden.** Use Bitnami Sealed Secrets; never commit
|
||||
plaintext. See the `safety` skill.
|
||||
- **Production (`groombook` namespace) is Flux-managed.** Never
|
||||
`kubectl apply` directly. Infrastructure changes go through PRs in
|
||||
`groombook/infra`.
|
||||
|
||||
## Local development
|
||||
|
||||
See the repo's own README, package scripts, and CI workflow. The
|
||||
authoritative pipeline (Gitea Actions, image build, deploy hooks) is the
|
||||
shared `groombook/infra` overlay; do not reimplement it here.
|
||||
|
||||
## When uncertain
|
||||
|
||||
If a task conflicts with the org skills, **the org skills win**. Open an
|
||||
issue in `groombook/org` to propose a change rather than encoding a local
|
||||
exception.
|
||||
+117
@@ -0,0 +1,117 @@
|
||||
# Contributing to `groombook/api`
|
||||
|
||||
Thanks for contributing. This document is the human-facing companion to
|
||||
[`AGENTS.md`](./AGENTS.md) and the authoritative
|
||||
[`groombook/org`](https://git.farh.net/groombook/org) skills. The org skills
|
||||
govern; this file is a quick-reference for the human/agent PR flow in this
|
||||
repo.
|
||||
|
||||
## Branch strategy
|
||||
|
||||
Three long-lived branches; one PR per promotion step.
|
||||
|
||||
| Branch | Environment | Who merges | Prerequisites for merge |
|
||||
|---------|-------------|-----------|-------------------------|
|
||||
| `dev` | Dev | Engineer | CI passes |
|
||||
| `uat` | UAT | Engineer | QA code review approval |
|
||||
| `main` | Production | Engineer | UAT validation + CTO Gitea Approve when the `uat→main` merge-gate policy applies (see below) |
|
||||
|
||||
Engineers always target `dev` first. Feature branches: `<agent-name>/<short-description>`.
|
||||
|
||||
## Phase-by-phase PR flow
|
||||
|
||||
### Phase 1 — Dev
|
||||
|
||||
1. Branch from `dev`: `git checkout -b <name>/<short-description> origin/dev`.
|
||||
2. Write code + tests. Run unit tests, type check, and lint locally (or rely on CI).
|
||||
3. Open a PR against `dev`:
|
||||
```bash
|
||||
tea pr create --base dev --title "..." --body "..."
|
||||
```
|
||||
Include `cc @cpfarhood` at the bottom of the body for board visibility.
|
||||
4. CI must pass. CI green → engineer self-merges.
|
||||
5. CI builds and deploys to Dev automatically.
|
||||
|
||||
### Phase 2 — UAT promotion
|
||||
|
||||
1. Open a PR from `dev` to `uat`.
|
||||
2. CI must pass.
|
||||
3. **QA (Lint Roller)** reviews and approves on the Gitea PR.
|
||||
4. QA approved → engineer self-merges.
|
||||
5. CI builds and deploys to UAT automatically.
|
||||
|
||||
### Phase 3 — UAT regression + Security review
|
||||
|
||||
1. **UAT (Shedward Scissorhands)** runs full regression against UAT — every
|
||||
feature, old and new, no exceptions.
|
||||
2. **Security (Barkley Trimsworth)** reviews the changes.
|
||||
3. Failures in either gate bounce back to Phase 1.
|
||||
|
||||
### Phase 4 — Production promotion (`uat → main`)
|
||||
|
||||
This is the gate the org PR
|
||||
[`groombook/org#13`](https://git.farh.net/groombook/org/pulls/13) defines.
|
||||
The full rule is in
|
||||
[`groombook/org/skills/sdlc/SKILL.md`](https://git.farh.net/groombook/org/src/branch/main/skills/sdlc/SKILL.md)
|
||||
and
|
||||
[`groombook/org/skills/coding-standards/SKILL.md`](https://git.farh.net/groombook/org/src/branch/main/skills/coding-standards/SKILL.md);
|
||||
the summary is below.
|
||||
|
||||
**The CTO Gitea Approve click is NOT the default gate.** Once the four
|
||||
pre-gates (QA, UAT deploy, UAT regression, security) are green, the engineer
|
||||
self-merges.
|
||||
|
||||
**A CTO Gitea Approve click IS required** only for PRs in one of three
|
||||
categories:
|
||||
|
||||
1. **Novel auth / session paths** — login, OIDC, OOBE, session middleware,
|
||||
token issuance, password reset, MFA, new auth provider integrations.
|
||||
Routine auth-gated UI (button styling, error messages, form layout) is
|
||||
**not** in this category.
|
||||
2. **Infra / prod-affecting merges** — deploys, infra manifests, secrets,
|
||||
GitOps overlays, CI/CD, `main` branch protection, production
|
||||
routing/ingress, prod state mutations. All Phase 5 infra overlay PRs in
|
||||
`groombook/infra` require CTO Gitea Approve without exception.
|
||||
3. **Risk-flagged merges** — `risk:cto-approve` label, or explicit CTO/CEO
|
||||
sign-off request in the PR or issue thread.
|
||||
|
||||
The engineer opens the `uat→main` PR, classifies it against the three
|
||||
categories above, and adds `cc @cpfarhood`. If the PR is in scope, the CTO
|
||||
clicks Approve; once approved (and the four pre-gates are green), the
|
||||
engineer merges.
|
||||
|
||||
### Phase 5 — Production deployment
|
||||
|
||||
A separate PR in `groombook/infra` bumps the overlay image tag for prod.
|
||||
Handed to QA (Lint Roller) for review, then self-merged by the engineer.
|
||||
|
||||
## The four pre-gates (uat→main)
|
||||
|
||||
A `uat→main` PR is mergeable when **all four** are green:
|
||||
|
||||
1. **QA code review** — done on the dev→uat promotion PR.
|
||||
2. **UAT deploy** — the UAT image built from the uat tip is live in UAT.
|
||||
3. **UAT regression** — Shedward's full-feature UAT pass is green (no
|
||||
pre-existing defects, no new defects).
|
||||
4. **Security review** — Barkley's security code review is green.
|
||||
|
||||
Issue-thread QA / UAT / security approvals do **not** clear the Gitea
|
||||
`required_approvals` gate. Only a Gitea **Approve** click from a member of
|
||||
the `approvals_whitelist_username` for `main` clears it. In this repo that
|
||||
whitelist is the engineer team (`gb_flea`, `gb_dogfather`).
|
||||
|
||||
## Style, tests, and quality bar
|
||||
|
||||
See
|
||||
[`groombook/org/skills/coding-standards/SKILL.md`](https://git.farh.net/groombook/org/src/branch/main/skills/coding-standards/SKILL.md)
|
||||
for the engineering priority ordering, test requirements, no-hardcoded-values
|
||||
rules, CalVer versioning policy, and the `git.farh.net` container registry
|
||||
policy.
|
||||
|
||||
## Safety
|
||||
|
||||
See
|
||||
[`groombook/org/skills/safety/SKILL.md`](https://git.farh.net/groombook/org/src/branch/main/skills/safety/SKILL.md)
|
||||
for the non-negotiable rules: no plaintext secrets, no `kubectl apply` to
|
||||
`groombook`, no self-merge, no direct `tofu` runs, board approval for
|
||||
destructive actions, escalation protocol.
|
||||
@@ -0,0 +1,201 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { Hono } from "hono";
|
||||
import { getAuth } from "../lib/auth.js";
|
||||
|
||||
const NEW_USER_EMAIL = "new-sso-user@example.com";
|
||||
const NEW_USER_NAME = "New SSO User";
|
||||
const NEW_USER_ID = "11111111-2222-3333-4444-555555555555";
|
||||
|
||||
const BETTER_AUTH_SESSION = {
|
||||
user: {
|
||||
id: "auth-user-new",
|
||||
email: NEW_USER_EMAIL,
|
||||
name: NEW_USER_NAME,
|
||||
},
|
||||
session: {
|
||||
id: "ba-session-new",
|
||||
expiresAt: new Date(Date.now() + 60 * 60 * 1000),
|
||||
},
|
||||
};
|
||||
|
||||
let mockGetAuth: ReturnType<typeof vi.fn>;
|
||||
let mockGetSession: ReturnType<typeof vi.fn>;
|
||||
let existingClientRow: Record<string, unknown> | null = null;
|
||||
let insertedClientValues: Record<string, unknown> | null = null;
|
||||
let insertShouldThrow: { code?: string } | null = null;
|
||||
|
||||
function makeChainable(data: unknown[]): unknown {
|
||||
const arr = [...data];
|
||||
return new Proxy(arr, {
|
||||
get(target, prop) {
|
||||
if (prop === "where" || prop === "orderBy" || prop === "limit") {
|
||||
return () => makeChainable(target);
|
||||
}
|
||||
// @ts-expect-error proxy
|
||||
return target[prop];
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
vi.mock("@groombook/db", () => {
|
||||
const clients = new Proxy(
|
||||
{ _name: "clients" },
|
||||
{ get: (t, p) => (p === "_name" ? "clients" : { table: "clients", column: p }) }
|
||||
);
|
||||
|
||||
return {
|
||||
getDb: () => ({
|
||||
select: () => ({
|
||||
from: (table: { _name: string }) => {
|
||||
if (table._name === "clients") {
|
||||
return makeChainable(existingClientRow ? [existingClientRow] : []);
|
||||
}
|
||||
return makeChainable([]);
|
||||
},
|
||||
}),
|
||||
insert: (table: { _name: string }) => ({
|
||||
values: (vals: Record<string, unknown>) => {
|
||||
if (insertShouldThrow) {
|
||||
const err = new Error("unique violation") as Error & { code?: string };
|
||||
err.code = insertShouldThrow.code;
|
||||
throw err;
|
||||
}
|
||||
return {
|
||||
returning: () => {
|
||||
if (table._name === "clients") {
|
||||
insertedClientValues = { id: NEW_USER_ID, ...vals };
|
||||
return [insertedClientValues];
|
||||
}
|
||||
return [];
|
||||
},
|
||||
};
|
||||
},
|
||||
}),
|
||||
}),
|
||||
clients,
|
||||
eq: vi.fn(),
|
||||
and: vi.fn(),
|
||||
inArray: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("../lib/auth.js", () => ({
|
||||
getAuth: vi.fn(),
|
||||
}));
|
||||
|
||||
const { portalRouter } = await import("../routes/portal.js");
|
||||
|
||||
const app = new Hono();
|
||||
app.route("/portal", portalRouter);
|
||||
|
||||
describe("POST /portal/clients-from-auth (GRO-2359)", () => {
|
||||
beforeEach(() => {
|
||||
existingClientRow = null;
|
||||
insertedClientValues = null;
|
||||
insertShouldThrow = null;
|
||||
mockGetSession = vi.fn();
|
||||
mockGetAuth = vi.fn(() => ({
|
||||
api: {
|
||||
getSession: mockGetSession,
|
||||
},
|
||||
}));
|
||||
vi.mocked(getAuth).mockImplementation(mockGetAuth);
|
||||
});
|
||||
|
||||
it("returns 401 when no Better Auth session is present", async () => {
|
||||
mockGetSession.mockResolvedValue(null);
|
||||
const res = await app.request("/portal/clients-from-auth", {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ name: "Test User" }),
|
||||
});
|
||||
expect(res.status).toBe(401);
|
||||
const body = await res.json();
|
||||
expect(body.error).toBe("Unauthorized");
|
||||
});
|
||||
|
||||
it("returns 400 when body fails zod validation (empty name)", async () => {
|
||||
mockGetSession.mockResolvedValue(BETTER_AUTH_SESSION);
|
||||
const res = await app.request("/portal/clients-from-auth", {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ name: "" }),
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
it("creates a new client row bound to the auth user's email and returns 201", async () => {
|
||||
mockGetSession.mockResolvedValue(BETTER_AUTH_SESSION);
|
||||
const res = await app.request("/portal/clients-from-auth", {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({
|
||||
name: " New SSO User ",
|
||||
phone: "555-1234",
|
||||
address: "1 Main St",
|
||||
notes: "test note",
|
||||
}),
|
||||
});
|
||||
expect(res.status).toBe(201);
|
||||
const body = await res.json();
|
||||
expect(body).toMatchObject({
|
||||
id: NEW_USER_ID,
|
||||
name: "New SSO User",
|
||||
email: NEW_USER_EMAIL,
|
||||
});
|
||||
// Trim must be applied to the persisted values.
|
||||
expect(insertedClientValues).not.toBeNull();
|
||||
expect((insertedClientValues as Record<string, unknown>).name).toBe("New SSO User");
|
||||
expect((insertedClientValues as Record<string, unknown>).email).toBe(NEW_USER_EMAIL);
|
||||
expect((insertedClientValues as Record<string, unknown>).phone).toBe("555-1234");
|
||||
});
|
||||
|
||||
it("normalizes empty optional fields to null on insert", async () => {
|
||||
mockGetSession.mockResolvedValue(BETTER_AUTH_SESSION);
|
||||
await app.request("/portal/clients-from-auth", {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ name: "Test", phone: "", address: " " }),
|
||||
});
|
||||
expect(insertedClientValues).not.toBeNull();
|
||||
expect((insertedClientValues as Record<string, unknown>).phone).toBeNull();
|
||||
expect((insertedClientValues as Record<string, unknown>).address).toBeNull();
|
||||
});
|
||||
|
||||
it("returns 409 when a client row already exists for this email", async () => {
|
||||
mockGetSession.mockResolvedValue(BETTER_AUTH_SESSION);
|
||||
existingClientRow = { id: "existing-client-id", email: NEW_USER_EMAIL };
|
||||
const res = await app.request("/portal/clients-from-auth", {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ name: "Test" }),
|
||||
});
|
||||
expect(res.status).toBe(409);
|
||||
const body = await res.json();
|
||||
expect(body.error).toMatch(/already exists/i);
|
||||
expect(insertedClientValues).toBeNull();
|
||||
});
|
||||
|
||||
it("returns 409 on unique constraint race (23505)", async () => {
|
||||
mockGetSession.mockResolvedValue(BETTER_AUTH_SESSION);
|
||||
insertShouldThrow = { code: "23505" };
|
||||
const res = await app.request("/portal/clients-from-auth", {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ name: "Test" }),
|
||||
});
|
||||
expect(res.status).toBe(409);
|
||||
});
|
||||
|
||||
it("returns 503 when auth is not configured", async () => {
|
||||
mockGetAuth.mockImplementation(() => {
|
||||
throw new Error("Auth not initialized");
|
||||
});
|
||||
const res = await app.request("/portal/clients-from-auth", {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ name: "Test" }),
|
||||
});
|
||||
expect(res.status).toBe(503);
|
||||
});
|
||||
});
|
||||
@@ -147,6 +147,114 @@ portalRouter.post("/session-from-auth", async (c) => {
|
||||
);
|
||||
});
|
||||
|
||||
// GRO-2359 — register a brand-new SSO user. The post-auth handler in the
|
||||
// web portal redirects here when `session-from-auth` returns 404, so the
|
||||
// OOBE can complete a customer record for the new user. Auth is via the
|
||||
// Better Auth session (same shape as `session-from-auth`), so this is
|
||||
// registered BEFORE the `validatePortalSession` middleware.
|
||||
//
|
||||
// Contract:
|
||||
// POST /api/portal/clients-from-auth
|
||||
// Body: { name: string; phone?: string|null; address?: string|null; notes?: string|null }
|
||||
// 201: { id, name, email }
|
||||
// 400: invalid body (zod failure)
|
||||
// 401: no Better Auth session
|
||||
// 409: a `clients` row already exists for this email (portal selection case)
|
||||
// 500: insert failed
|
||||
//
|
||||
// We do NOT auto-link the user's auth account to the new client row; the
|
||||
// existing `session-from-auth` endpoint re-resolves the row by email on the
|
||||
// next call, so the OOBE's success path just navigates the user back to
|
||||
// `/` and lets the bridge mint a portal session.
|
||||
const createClientFromAuthSchema = z.object({
|
||||
name: z.string().min(1).max(200),
|
||||
phone: z.string().max(50).nullish(),
|
||||
address: z.string().max(500).nullish(),
|
||||
notes: z.string().max(2000).nullish(),
|
||||
});
|
||||
|
||||
portalRouter.post(
|
||||
"/clients-from-auth",
|
||||
zValidator("json", createClientFromAuthSchema),
|
||||
async (c) => {
|
||||
let auth;
|
||||
try {
|
||||
auth = getAuth();
|
||||
} catch {
|
||||
return c.json({ error: "Authentication not configured" }, 503);
|
||||
}
|
||||
|
||||
const session = await auth.api.getSession({
|
||||
headers: c.req.raw.headers,
|
||||
});
|
||||
|
||||
if (!session) {
|
||||
return c.json({ error: "Unauthorized" }, 401);
|
||||
}
|
||||
|
||||
const body = c.req.valid("json");
|
||||
const db = getDb();
|
||||
|
||||
// Pre-check: if a client already exists for this email, return 409 so
|
||||
// the OOBE can render the "portal selection" message (the user needs
|
||||
// to contact their groomer to link the new SSO identity to the
|
||||
// pre-existing customer record). We don't return the existing row to
|
||||
// avoid leaking PII about other accounts.
|
||||
const [existing] = await db
|
||||
.select({ id: clients.id })
|
||||
.from(clients)
|
||||
.where(eq(clients.email, session.user.email))
|
||||
.limit(1);
|
||||
|
||||
if (existing) {
|
||||
return c.json(
|
||||
{ error: "A customer record with this email already exists" },
|
||||
409,
|
||||
);
|
||||
}
|
||||
|
||||
let row;
|
||||
try {
|
||||
[row] = await db
|
||||
.insert(clients)
|
||||
.values({
|
||||
name: body.name.trim(),
|
||||
email: session.user.email,
|
||||
phone: body.phone?.trim() || null,
|
||||
address: body.address?.trim() || null,
|
||||
notes: body.notes?.trim() || null,
|
||||
})
|
||||
.returning();
|
||||
} catch (err) {
|
||||
// Concurrent insert from a parallel OOBE submit — treat as 409.
|
||||
if (
|
||||
err instanceof Error &&
|
||||
"code" in err &&
|
||||
(err as { code?: string }).code === "23505"
|
||||
) {
|
||||
return c.json(
|
||||
{ error: "A customer record with this email already exists" },
|
||||
409,
|
||||
);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
if (!row) {
|
||||
return c.json({ error: "Failed to create client" }, 500);
|
||||
}
|
||||
|
||||
return c.json(
|
||||
{
|
||||
id: row.id,
|
||||
name: row.name,
|
||||
email: row.email,
|
||||
},
|
||||
201,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
// Apply middleware to all portal routes
|
||||
portalRouter.use("/*", validatePortalSession, portalAudit);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user