diff --git a/go-api/internal/gateway/anthropic.go b/go-api/internal/gateway/anthropic.go index 872450d..e2f5624 100644 --- a/go-api/internal/gateway/anthropic.go +++ b/go-api/internal/gateway/anthropic.go @@ -345,15 +345,33 @@ func translate(err error) error { return &Error{Code: CodeUpstream, Message: "the model call failed", Cause: err} } + // The upstream reason, carried through when there is one — the same rule the + // OpenAI path already follows, and for the same reason it gives: a 400 that + // says which model id or tool schema was rejected is worth more than "the + // model rejected the request", and the trajectory records only the message. + // + // This was not a hypothetical. Every run on a deployment failed with a bare + // `gateway.invalid_request`, and neither the run detail nor anything a + // client could read said why — the one fact needed to fix it was discarded + // here, three lines from where it arrived. `Cause` keeps the full error for + // a Go caller; nothing reads it by the time a run is persisted. + detail := anthropicErrorMessage(apierr.RawJSON()) + withDetail := func(base string) string { + if detail == "" { + return base + } + return base + ": " + detail + } + switch apierr.StatusCode { case 400: - return &Error{Code: CodeInvalidRequest, Message: "the model rejected the request", Status: 400, Cause: err} + return &Error{Code: CodeInvalidRequest, Message: withDetail("the model rejected the request"), Status: 400, Cause: err} case 401, 403: - return &Error{Code: CodeUnauthorized, Message: "the model credentials were refused", Status: apierr.StatusCode, Cause: err} + return &Error{Code: CodeUnauthorized, Message: withDetail("the model credentials were refused"), Status: apierr.StatusCode, Cause: err} case 408: - return &Error{Code: CodeTimeout, Message: "the model call timed out", Status: 408, Cause: err} + return &Error{Code: CodeTimeout, Message: withDetail("the model call timed out"), Status: 408, Cause: err} case 429: - return &Error{Code: CodeRateLimited, Message: "the model is rate limiting this deployment", Status: 429, Cause: err} + return &Error{Code: CodeRateLimited, Message: withDetail("the model is rate limiting this deployment"), Status: 429, Cause: err} case 529: // Anthropic's "overloaded" — the service is up and temporarily out of // capacity. Named separately from the 500s because it is the one that @@ -368,12 +386,36 @@ func translate(err error) error { // message. return &Error{ Code: CodeUpstream, - Message: fmt.Sprintf("the model call failed (http %d)", apierr.StatusCode), + Message: withDetail(fmt.Sprintf("the model call failed (http %d)", apierr.StatusCode)), Status: apierr.StatusCode, Cause: err, } } } +// anthropicErrorMessage digs the human-readable reason out of an error body. +// +// Best-effort, exactly like its OpenAI counterpart: the envelope is documented +// and stable enough to be worth reading, and an unparseable body yields nothing +// rather than failing a failure. +func anthropicErrorMessage(raw string) string { + if raw == "" { + return "" + } + var envelope struct { + Error struct { + Message string `json:"message"` + Type string `json:"type"` + } `json:"error"` + } + if err := json.Unmarshal([]byte(raw), &envelope); err != nil { + return "" + } + if envelope.Error.Message != "" { + return envelope.Error.Message + } + return envelope.Error.Type +} + /* ── Streaming ──────────────────────────────────────────────────────────── */ // Stream is Complete, with the assistant's text delivered as it arrives. diff --git a/go-api/internal/gateway/anthropic_error_test.go b/go-api/internal/gateway/anthropic_error_test.go new file mode 100644 index 0000000..3bd6c11 --- /dev/null +++ b/go-api/internal/gateway/anthropic_error_test.go @@ -0,0 +1,46 @@ +package gateway + +import "testing" + +// The reason a 400 gives is the whole diagnosis, and it used to be thrown away. +// +// A deployment answered every single run with "The agent could not finish — +// something it needed did not answer." Retrieval worked, the run was created, +// the provider was reached, and the trajectory recorded `gateway.invalid_request` +// and nothing else. The actual cause was one sentence long and Anthropic had +// sent it: the account was out of credit. Nothing a client, a log or a run +// detail could show said so, because `translate` dropped the body. +// +// The envelope below is the real one, copied from that response. +func TestAnthropicErrorMessage(t *testing.T) { + cases := []struct { + name string + raw string + want string + }{ + { + name: "the outage this test exists for", + raw: `{"type":"error","error":{"type":"invalid_request_error",` + + `"message":"Your credit balance is too low to access the Anthropic API. ` + + `Please go to Plans & Billing to upgrade or purchase credits."},` + + `"request_id":"req_011CeiJqr8rTdpbZYhcL6VHG"}`, + want: "Your credit balance is too low to access the Anthropic API. " + + "Please go to Plans & Billing to upgrade or purchase credits.", + }, + { + name: "a message-less envelope falls back to the type", + raw: `{"type":"error","error":{"type":"overloaded_error"}}`, + want: "overloaded_error", + }, + // Best-effort by design: a failure to read a failure must not become a + // second failure. + {name: "unparseable body yields nothing", raw: `not json at all`, want: ""}, + {name: "empty body yields nothing", raw: ``, want: ""}, + } + + for _, c := range cases { + if got := anthropicErrorMessage(c.raw); got != c.want { + t.Errorf("%s: anthropicErrorMessage() = %q, want %q", c.name, got, c.want) + } + } +} diff --git a/go-api/internal/httpserver/authfixture_test.go b/go-api/internal/httpserver/authfixture_test.go index 33fc929..041bd85 100644 --- a/go-api/internal/httpserver/authfixture_test.go +++ b/go-api/internal/httpserver/authfixture_test.go @@ -66,13 +66,26 @@ func setStatus(t *testing.T, pool *pgxpool.Pool, userID, status string) { } } -// seededUser is the demo user the fixture loads into the test database. +// seededUser is the demo ADMINISTRATOR the fixture loads into the test +// database. +// +// The role is now part of the question. The fixture used to hold one account, +// so "the seeded user" and "the administrator" were the same row and ordering +// by date was enough to find it. It holds two since the Employer console gained +// somebody to sign in as, both created on the same seeded date, which left the +// tiebreak to a deterministic UUID — and picked the employer. Tests that assert +// an administrator's access were then asserting an employer's, and failed +// exactly as they should have. +// +// So it asks for what it means. Ordering is kept beneath the filter for the +// case of several administrators. func seededUser(t *testing.T, pool *pgxpool.Pool) (id, email string) { t.Helper() if err := pool.QueryRow(context.Background(), - `SELECT id::text, email::text FROM users ORDER BY created_date, id LIMIT 1`). + `SELECT id::text, email::text FROM users WHERE role = 'admin' + ORDER BY created_date, id LIMIT 1`). Scan(&id, &email); err != nil { - t.Fatalf("read the seeded user: %v", err) + t.Fatalf("read the seeded administrator: %v", err) } return id, email } diff --git a/go-api/internal/httpserver/employee_roles_test.go b/go-api/internal/httpserver/employee_roles_test.go new file mode 100644 index 0000000..e0dad6c --- /dev/null +++ b/go-api/internal/httpserver/employee_roles_test.go @@ -0,0 +1,195 @@ +package httpserver_test + +import ( + "net/http" + "testing" +) + +// Employee roles: what a worker declares they do. +// +// The properties here are the ones the conversational flow depends on and that +// no amount of frontend testing can establish, because they are decided by a +// SQL predicate and a derived column: +// +// THE OPERATOR IS NOT THE WORKER. An employer records a role for somebody +// else. If the subject were derived from the session — as created_by +// legitimately is — every role would be filed against whoever was signed in. +// +// A WORKER HOLDS MANY ROLES. There is deliberately no uniqueness on the +// worker, so a second declaration is a second row and a role already marked +// `placed` survives the worker declaring the same category again. The panel +// promises exactly this in its review step: "The worker can hold more than one +// role — recording this does not replace an existing one." + +func createEmployeeRole(t *testing.T, r *rbac, act actor, body map[string]any) map[string]any { + t.Helper() + got := r.as(act, "POST", "/api/v1/employee-roles", body) + if got.code != http.StatusCreated { + t.Fatalf("%s create employee role: %d (%v)", act.name, got.code, got.body) + } + return got.body["data"].(map[string]any) +} + +// The subject comes from the request; only the audit column comes from the +// session. This is the test that fails if anyone ever derives worker_email the +// way job-applications derives it for a talent caller. +func TestEmployeeRoleRecordsTheWorkerNotTheOperator(t *testing.T) { + r := newRBAC(t) + + rec := createEmployeeRole(t, r, r.empA, map[string]any{ + "worker_email": "someone-else@example.test", + "worker_name": "Someone Else", + "role_category": "Bartender", + }) + + if rec["worker_email"] == r.empA.email { + t.Fatal("the operator became the worker") + } + if got := rec["worker_email"]; got != "someone-else@example.test" { + t.Errorf("worker_email = %v, want the worker's", got) + } + if got := rec["created_by"]; got != r.empA.id { + t.Errorf("created_by = %v, want the operator %v", got, r.empA.id) + } +} + +// created_by is ReadOnly in the descriptor, so a caller cannot attribute a role +// to somebody else. The body's value is dropped, not honoured. +func TestEmployeeRoleCreatedByIsNotClientSettable(t *testing.T) { + r := newRBAC(t) + rec := createEmployeeRole(t, r, r.empA, map[string]any{ + "worker_email": "worker@example.test", "role_category": "Server", + "created_by": r.admin.id, + }) + if got := rec["created_by"]; got != r.empA.id { + t.Errorf("created_by = %v, want the caller %v — the body must not set it", got, r.empA.id) + } +} + +// The promise the review step makes, tested against the database. +func TestAWorkerHoldsManyRolesAndNoneReplaceAnother(t *testing.T) { + r := newRBAC(t) + const worker = "many-roles@example.test" + + first := createEmployeeRole(t, r, r.empA, map[string]any{ + "worker_email": worker, "worker_name": "Many Roles", + "role_category": "Bartender", "status": "placed", + }) + second := createEmployeeRole(t, r, r.empA, map[string]any{ + "worker_email": worker, "worker_name": "Many Roles", "role_category": "Server", + }) + // The same category again while the first is still placed: a worker who + // finished a Bartender placement and is seeking Bartender work again. + third := createEmployeeRole(t, r, r.empA, map[string]any{ + "worker_email": worker, "worker_name": "Many Roles", "role_category": "Bartender", + }) + + ids := map[string]bool{} + for _, rec := range []map[string]any{first, second, third} { + id := rec["id"].(string) + if ids[id] { + t.Fatalf("duplicate id %s — a role replaced another", id) + } + ids[id] = true + } + + got := r.ids(t, r.empA, "/api/v1/employee-roles?worker_email="+worker) + for id := range ids { + if !got[id] { + t.Errorf("role %s is missing — it was overwritten or filtered away", id) + } + } + if len(got) != 3 { + t.Errorf("%d roles for one worker, want 3", len(got)) + } + if first["status"] != "placed" { + t.Errorf("the first role's status = %v, want placed to survive", first["status"]) + } +} + +// Every field the conversation collects survives the round trip. Named from the +// payload the panel actually sends, so a column the flow fills and the API drops +// fails here rather than silently arriving empty. +func TestEmployeeRoleKeepsEveryCollectedField(t *testing.T) { + r := newRBAC(t) + + rec := createEmployeeRole(t, r, r.admin, map[string]any{ + "worker_email": "full@example.test", "worker_name": "Full Record", + "role_category": "Picker", "experience_years": 3, + "english_level": "native", "certifications": []string{"TIPS Certified"}, + "desired_pay_min": 30, "desired_pay_max": 40, + "availability": []string{"Weekdays"}, "notes": "recorded by the panel", + "status": "seeking", + }) + + for _, tc := range []struct { + field string + want any + }{ + {"role_category", "Picker"}, + {"experience_years", float64(3)}, + {"english_level", "native"}, + {"desired_pay_min", float64(30)}, + {"desired_pay_max", float64(40)}, + {"notes", "recorded by the panel"}, + {"status", "seeking"}, + } { + if got := rec[tc.field]; got != tc.want { + t.Errorf("%s = %#v, want %#v", tc.field, got, tc.want) + } + } + for _, tc := range []struct { + field string + want string + }{{"certifications", "TIPS Certified"}, {"availability", "Weekdays"}} { + list, _ := rec[tc.field].([]any) + if len(list) != 1 || list[0] != tc.want { + t.Errorf("%s = %#v, want [%q]", tc.field, rec[tc.field], tc.want) + } + } +} + +// Cross-tenant isolation stands on its own: an ADMIN in another organization +// gets 404, not 403, and never sees the row in a listing. +func TestEmployeeRolesAreInvisibleAcrossOrganizations(t *testing.T) { + r := newRBAC(t) + + rec := createEmployeeRole(t, r, r.admin, map[string]any{ + "worker_email": "inside@example.test", "role_category": "Bartender", + }) + id := rec["id"].(string) + + if got := r.as(r.outsider, "GET", "/api/v1/employee-roles/"+id, nil); got.code != http.StatusNotFound { + t.Errorf("outside admin GET = %d, want 404", got.code) + } + if r.ids(t, r.outsider, "/api/v1/employee-roles")[id] { + t.Error("a role leaked into another organization's listing") + } + if got := r.as(r.outsider, "PATCH", "/api/v1/employee-roles/"+id, + map[string]any{"notes": "n"}); got.code != http.StatusNotFound { + t.Errorf("outside admin PATCH = %d, want 404", got.code) + } +} + +// A talent caller reads only their own declared roles, and cannot create. +func TestTalentSeesOnlyItsOwnEmployeeRoles(t *testing.T) { + r := newRBAC(t) + + mine := createEmployeeRole(t, r, r.empA, map[string]any{ + "worker_email": r.talA.email, "role_category": "Bartender", + }) + theirs := createEmployeeRole(t, r, r.empA, map[string]any{ + "worker_email": r.talB.email, "role_category": "Server", + }) + + seen := r.ids(t, r.talA, "/api/v1/employee-roles") + if !seen[mine["id"].(string)] { + t.Error("talent cannot see its own declared role") + } + if seen[theirs["id"].(string)] { + t.Error("talent A can see talent B's declared role") + } + if got := r.as(r.talA, "GET", "/api/v1/employee-roles/"+theirs["id"].(string), nil); got.code != http.StatusNotFound { + t.Errorf("GET another talent's role = %d, want 404 — absent, not refused", got.code) + } +} diff --git a/go-api/internal/httpserver/worker_identity_test.go b/go-api/internal/httpserver/worker_identity_test.go new file mode 100644 index 0000000..20aa144 --- /dev/null +++ b/go-api/internal/httpserver/worker_identity_test.go @@ -0,0 +1,89 @@ +package httpserver_test + +import ( + "net/http" + "testing" +) + +// Who counts as the same person. +// +// The rule is the schema's and it is worth stating plainly, because the whole +// duplicate question turns on it: `worker_profiles` carries +// UNIQUE (org_id, email) and `email` is `citext`. So identity is the pair +// (organization, email), compared case-insensitively, and `full_name` carries +// NO uniqueness at all — an organization may employ any number of people with +// the same name, and they are different people. +// +// These are database guarantees rather than application checks, which is what +// makes them hold under concurrency: two simultaneous creates of the same +// identity cannot both win, whatever the callers checked first. + +func createWorker(t *testing.T, r *rbac, act actor, name, email string) response { + t.Helper() + return r.as(act, "POST", "/api/v1/worker-profiles", map[string]any{ + "full_name": name, "email": email, + }) +} + +// A name is not an identity. Two people who share one are two records. +func TestWorkersMayShareAName(t *testing.T) { + r := newRBAC(t) + const shared = "Shared Name" + + first := createWorker(t, r, r.admin, shared, "shared-name-1@example.test") + second := createWorker(t, r, r.admin, shared, "shared-name-2@example.test") + + for i, got := range []response{first, second} { + if got.code != http.StatusCreated { + t.Fatalf("create %d: %d (%v) — sharing a name must not block creation", i+1, got.code, got.body) + } + } + a := first.body["data"].(map[string]any) + b := second.body["data"].(map[string]any) + if a["id"] == b["id"] { + t.Fatal("two people sharing a name collapsed into one record") + } + if a["email"] == b["email"] { + t.Error("the second worker took the first one's email") + } +} + +// The same identity cannot be created twice, whoever it claims to be, and the +// refusal is a conflict a caller can act on rather than a 500. +func TestTheSameIdentityCannotBeCreatedTwice(t *testing.T) { + r := newRBAC(t) + const email = "one-identity@example.test" + + if got := createWorker(t, r, r.admin, "Person One", email); got.code != http.StatusCreated { + t.Fatalf("first create: %d (%v)", got.code, got.body) + } + + for _, tc := range []struct{ name, who, email string }{ + {"a different name on the same email", "Person Two", email}, + {"the same email in another case", "Person Three", "ONE-IDENTITY@EXAMPLE.TEST"}, + } { + t.Run(tc.name, func(t *testing.T) { + got := createWorker(t, r, r.admin, tc.who, tc.email) + if got.code != http.StatusConflict { + t.Errorf("= %d, want 409 — the identity is already taken", got.code) + } + if got.errCode(t) != "conflict" { + t.Errorf("error code = %q, want conflict", got.errCode(t)) + } + }) + } +} + +// The identity is scoped to the organization, so the same email in another +// tenant is another person and is allowed. +func TestTheSameEmailInAnotherOrganizationIsAnotherPerson(t *testing.T) { + r := newRBAC(t) + const email = "cross-tenant-identity@example.test" + + if got := createWorker(t, r, r.admin, "Inside", email); got.code != http.StatusCreated { + t.Fatalf("create inside: %d (%v)", got.code, got.body) + } + if got := createWorker(t, r, r.outsider, "Outside", email); got.code != http.StatusCreated { + t.Errorf("create in another organization = %d, want 201 — identity is (org, email)", got.code) + } +} diff --git a/go-api/internal/httpserver/worker_with_role_test.go b/go-api/internal/httpserver/worker_with_role_test.go new file mode 100644 index 0000000..6461bc6 --- /dev/null +++ b/go-api/internal/httpserver/worker_with_role_test.go @@ -0,0 +1,175 @@ +package httpserver_test + +import ( + "net/http" + "testing" +) + +// Recording a NEW person and their first declared role, atomically. +// +// The flow this endpoint exists for is a CREATION: HR is adding somebody the +// organization does not have yet. So the properties under test are about +// creation, not lookup — no worker id is accepted, no name is searched, and the +// email the caller states is the identity the row is keyed on. + +func createWorkerWithRole(t *testing.T, r *rbac, act actor, body map[string]any) response { + t.Helper() + return r.as(act, "POST", "/api/v1/worker-profiles/with-role", body) +} + +// The happy path, and the two records it must leave behind. +func TestCreateWorkerWithRoleCreatesBoth(t *testing.T) { + r := newRBAC(t) + const email = "new-person@example.test" + + got := createWorkerWithRole(t, r, r.empA, map[string]any{ + "full_name": "New Person", "email": email, + "role": map[string]any{ + "role_category": "Bartender", "experience_years": 3, + "english_level": "fluent", "certifications": []string{"A Certification"}, + "desired_pay_min": 30, "desired_pay_max": 40, + "availability": []string{"Weekdays"}, "notes": "recorded by the panel", + }, + }) + if got.code != http.StatusCreated { + t.Fatalf("= %d, want 201 (%v)", got.code, got.body) + } + + data := got.body["data"].(map[string]any) + worker := data["worker"].(map[string]any) + role := data["role"].(map[string]any) + + if worker["id"] == nil || worker["id"] == "" { + t.Fatal("no worker id came back") + } + // The whole point: the role points at the worker this call created. + if role["worker_profile_id"] != worker["id"] { + t.Errorf("role.worker_profile_id = %v, want the new worker %v", role["worker_profile_id"], worker["id"]) + } + if role["worker_email"] != worker["email"] { + t.Errorf("role.worker_email = %v, want %v", role["worker_email"], worker["email"]) + } + // The operator is the author, never the subject. + if role["created_by"] != r.empA.id { + t.Errorf("created_by = %v, want the operator %v", role["created_by"], r.empA.id) + } + if worker["email"] == r.empA.email { + t.Fatal("the operator became the worker") + } + // The role's own fields survived, and did not land on the worker. + if role["role_category"] != "Bartender" { + t.Errorf("role_category = %v", role["role_category"]) + } + if _, leaked := worker["role_category"]; leaked { + t.Error("a role field landed on the worker record") + } + + // Both are readable afterwards, under the caller's own org predicate. + if !r.ids(t, r.empA, "/api/v1/worker-profiles")[worker["id"].(string)] { + t.Error("the new worker is missing from the worker listing") + } + if !r.ids(t, r.empA, "/api/v1/employee-roles")[role["id"].(string)] { + t.Error("the new role is missing from the role listing") + } +} + +// A name is not an identity: same name, different emails, two people. +func TestCreateWorkerWithRoleAllowsARepeatedName(t *testing.T) { + r := newRBAC(t) + const name = "Repeated Name" + + first := createWorkerWithRole(t, r, r.admin, map[string]any{ + "full_name": name, "email": "repeat-1@example.test", + "role": map[string]any{"role_category": "Server"}, + }) + second := createWorkerWithRole(t, r, r.admin, map[string]any{ + "full_name": name, "email": "repeat-2@example.test", + "role": map[string]any{"role_category": "Chef"}, + }) + + for i, got := range []response{first, second} { + if got.code != http.StatusCreated { + t.Fatalf("create %d = %d (%v) — a shared name must not block creation", i+1, got.code, got.body) + } + } + a := first.body["data"].(map[string]any)["worker"].(map[string]any) + b := second.body["data"].(map[string]any)["worker"].(map[string]any) + if a["id"] == b["id"] { + t.Fatal("two people sharing a name collapsed into one record") + } +} + +// The identity is the email, and the database decides. A repeat is refused and +// leaves NOTHING behind — no worker, no role. +func TestCreateWorkerWithRoleRollsBackOnDuplicateIdentity(t *testing.T) { + r := newRBAC(t) + const email = "taken-identity@example.test" + + if got := createWorkerWithRole(t, r, r.admin, map[string]any{ + "full_name": "First Person", "email": email, + "role": map[string]any{"role_category": "Server"}, + }); got.code != http.StatusCreated { + t.Fatalf("first create: %d (%v)", got.code, got.body) + } + + before := len(r.ids(t, r.admin, "/api/v1/employee-roles")) + + got := createWorkerWithRole(t, r, r.admin, map[string]any{ + "full_name": "Second Person", "email": email, + "role": map[string]any{"role_category": "Chef"}, + }) + if got.code != http.StatusConflict { + t.Fatalf("duplicate identity = %d, want 409 (%v)", got.code, got.body) + } + if after := len(r.ids(t, r.admin, "/api/v1/employee-roles")); after != before { + t.Errorf("%d roles after a refused create, want %d — the transaction did not roll back", after, before) + } +} + +// A role cannot be recorded for nobody, and an email is never invented for a +// name that arrived without one. +func TestCreateWorkerWithRoleRequiresBothNameAndEmail(t *testing.T) { + r := newRBAC(t) + + for _, tc := range []struct { + name string + body map[string]any + }{ + {"no email", map[string]any{"full_name": "Nameless Email", "role": map[string]any{"role_category": "Server"}}}, + {"blank email", map[string]any{"full_name": "Blank", "email": " ", "role": map[string]any{"role_category": "Server"}}}, + {"no name", map[string]any{"email": "no-name@example.test", "role": map[string]any{"role_category": "Server"}}}, + {"neither", map[string]any{"role": map[string]any{"role_category": "Server"}}}, + } { + t.Run(tc.name, func(t *testing.T) { + got := createWorkerWithRole(t, r, r.admin, tc.body) + if got.code == http.StatusCreated { + t.Fatalf("accepted a worker with %s: %v", tc.name, got.body) + } + }) + } +} + +// Talent cannot record workers, and another organization cannot see the ones +// this one records. +func TestCreateWorkerWithRoleIsScopedAndAuthorized(t *testing.T) { + r := newRBAC(t) + + if got := createWorkerWithRole(t, r, r.talA, map[string]any{ + "full_name": "Not Allowed", "email": "not-allowed@example.test", + "role": map[string]any{"role_category": "Server"}, + }); got.code != http.StatusForbidden { + t.Errorf("talent create = %d, want 403", got.code) + } + + made := createWorkerWithRole(t, r, r.admin, map[string]any{ + "full_name": "Inside Only", "email": "inside-only@example.test", + "role": map[string]any{"role_category": "Server"}, + }) + if made.code != http.StatusCreated { + t.Fatalf("create: %d (%v)", made.code, made.body) + } + roleID := made.body["data"].(map[string]any)["role"].(map[string]any)["id"].(string) + if r.ids(t, r.outsider, "/api/v1/employee-roles")[roleID] { + t.Error("a role leaked into another organization") + } +} diff --git a/go-api/internal/httpserver/workflows.go b/go-api/internal/httpserver/workflows.go index 9fbe05f..2e43552 100644 --- a/go-api/internal/httpserver/workflows.go +++ b/go-api/internal/httpserver/workflows.go @@ -86,7 +86,50 @@ func errUnregisteredResource(path string) error { return unregisteredResourceErr func (s *Server) routeWorkflows(mux *http.ServeMux) int { mux.HandleFunc("POST /api/v1/job-applications/{id}/hire", s.handleHire) mux.HandleFunc("POST /api/v1/job-postings/{id}/assignments", s.handleAssign) - return 2 + mux.HandleFunc("POST /api/v1/worker-profiles/with-role", s.handleCreateWorkerWithRole) + return 3 +} + +// handleCreateWorkerWithRole records a NEW person and their first declared role +// in one transaction. +// +// Under `worker-profiles` rather than `employee-roles` because the worker is +// what the request creates; the role comes with it. A more specific literal +// than the generated `POST /api/v1/worker-profiles`, so the mux prefers it and +// neither route shadows the other. +// +// This is a CREATION flow. It takes a name and an email and never a worker id, +// and nothing in it searches for an existing person — an organization may +// employ many people who share a name, so a name cannot select anybody. +// Recording a second role for someone who already exists is +// POST /api/v1/employee-roles, unchanged. +func (s *Server) handleCreateWorkerWithRole(w http.ResponseWriter, r *http.Request) { + ident, ok := s.authorizeAll(w, r, + requirement{"worker-profiles", domain.OpCreate}, + requirement{"employee-roles", domain.OpCreate}, + ) + if !ok { + return + } + + body, err := decodeBody(r) + if err != nil { + writeError(w, s.log, err) + return + } + + result, err := s.workflows.CreateWorkerWithRole(r.Context(), ident, body) + if err != nil { + writeError(w, s.log, err) + return + } + + /* The worker's identity is not logged: an email is the person, and §10 puts + record content at DEBUG behind a per-tenant flag rather than at INFO. */ + s.log.Info("worker recorded with a declared role", "user_id", ident.UserID, + "worker_profile_id", result.Worker["id"], "employee_role_id", result.Role["id"]) + + writeJSON(w, http.StatusCreated, envelope{Data: result}) } // handleHire moves an application to `hired` and creates the staff record in diff --git a/go-api/internal/seeder/seeder.go b/go-api/internal/seeder/seeder.go index 4b52b87..113ecf6 100644 --- a/go-api/internal/seeder/seeder.go +++ b/go-api/internal/seeder/seeder.go @@ -57,8 +57,15 @@ func DeterministicUUID(name string) string { } // Fixture is the serialised frontend dataset. +// +// Users and DemoUser both name accounts, and both are read. Users is the list +// the generator writes today; DemoUser is the single account older fixtures +// carry, and is kept so a fixture written before the list existed still seeds. +// Where both are present the list wins and DemoUser is folded into it by id, so +// the demo administrator is written once rather than twice. type Fixture struct { DemoUser map[string]any `json:"demoUser"` + Users []map[string]any `json:"users"` Entities map[string][]map[string]any `json:"entities"` } @@ -165,7 +172,7 @@ func (s *Seeder) Run(ctx context.Context) (*Result, error) { } result := &Result{OrgID: orgID, Counts: map[string]int{}} - n, err := s.upsertUser(ctx, tx, orgID) + n, err := s.upsertUsers(ctx, tx, orgID) if err != nil { return nil, err } @@ -218,13 +225,49 @@ func (s *Seeder) upsertOrganization(ctx context.Context, tx pgx.Tx) (string, err return id, err } -// upsertUser writes the demo user and splits its preferences into their own -// table, as api-contract.md §9 describes. -func (s *Seeder) upsertUser(ctx context.Context, tx pgx.Tx, orgID string) (int, error) { - u := s.fixture.DemoUser - if u == nil { - return 0, nil +// seedUsers is the accounts to write, in fixture order and deduplicated by the +// legacy id the deterministic UUID is derived from. +// +// It used to be one account. That was not a simplification, it was the reason +// two thirds of the authorization table had never been exercised: `policy.go` +// has always had three roles and the fixture has always had one administrator, +// so there was nobody to sign in as to reach the employer console at all. +func (s *Seeder) seedUsers() []map[string]any { + seen := map[string]bool{} + var out []map[string]any + for _, u := range append(append([]map[string]any{}, s.fixture.Users...), s.fixture.DemoUser) { + if u == nil { + continue + } + legacy, _ := u["id"].(string) + if seen[legacy] { + continue + } + seen[legacy] = true + out = append(out, u) } + return out +} + +// upsertUsers writes the seeded accounts and splits their preferences into +// their own table, as api-contract.md §9 describes. +// +// Note what the ON CONFLICT set list below does NOT touch: password_hash. A +// re-seed refreshes who someone is and leaves them able to sign in, which is +// what makes `cmd/setpassword` a one-time action per account rather than a step +// after every `make seed`. +func (s *Seeder) upsertUsers(ctx context.Context, tx pgx.Tx, orgID string) (int, error) { + count := 0 + for _, u := range s.seedUsers() { + if err := s.upsertUser(ctx, tx, orgID, u); err != nil { + return count, err + } + count++ + } + return count, nil +} + +func (s *Seeder) upsertUser(ctx context.Context, tx pgx.Tx, orgID string, u map[string]any) error { legacy, _ := u["id"].(string) id := DeterministicUUID("User:" + legacy) created := stringOr(u["created_date"], iso(s.now)) @@ -240,7 +283,7 @@ func (s *Seeder) upsertUser(ctx context.Context, tx pgx.Tx, orgID string) (int, stringOr(u["email"], ""), stringOr(u["full_name"], ""), stringOr(u["role"], "admin"), stringOr(u["account_type"], "employer"), created) if err != nil { - return 0, err + return err } prefs, _ := u["preferences"].(map[string]any) @@ -257,7 +300,7 @@ func (s *Seeder) upsertUser(ctx context.Context, tx pgx.Tx, orgID string) (int, } extraJSON, err := json.Marshal(extra) if err != nil { - return 0, err + return err } _, err = tx.Exec(ctx, `INSERT INTO user_preferences (user_id, owliver_default, compact_density, email_digest, extra) @@ -271,9 +314,9 @@ func (s *Seeder) upsertUser(ctx context.Context, tx pgx.Tx, orgID string) (int, id, boolOr(prefs["owliverDefault"], true), boolOr(prefs["compactDensity"], false), boolOr(prefs["emailDigest"], true), extraJSON) if err != nil { - return 0, err + return err } - return 1, nil + return nil } // pruneShiftRecords deletes this organization's shift rows that this run did diff --git a/go-api/internal/service/workflows.go b/go-api/internal/service/workflows.go index b0d5f08..619e885 100644 --- a/go-api/internal/service/workflows.go +++ b/go-api/internal/service/workflows.go @@ -29,6 +29,7 @@ import ( "context" "errors" "fmt" + "strings" "time" "github.com/jackc/pgx/v5" @@ -110,6 +111,107 @@ type HireResult struct { Staff domain.Record `json:"staff"` } +// WorkerWithRoleResult is what recording a new worker returns: both records the +// flow created, so the caller renders the outcome without a follow-up read. +type WorkerWithRoleResult struct { + Worker domain.Record `json:"worker"` + Role domain.Record `json:"role"` +} + +// CreateWorkerWithRole records a NEW person and their first declared role, atomically. +// +// The third flow this file exists for, and it has the same shape as the two +// above: the frontend would otherwise POST a worker profile, then POST an +// employee role against the id it came back with, and a failure between them +// leaves a worker nobody meant to create with no role and no way to tell them +// apart from a real one. +// +// WHY THIS IS NOT "FIND THE WORKER, THEN ADD A ROLE" +// +// It is a creation flow, deliberately. A name is not an identity — an +// organization may employ any number of people who share one — so nothing here +// searches for an existing worker by name, and the caller cannot supply a +// worker id. The identity is the email, which the caller states outright, and +// the schema decides whether it is already taken: `worker_profiles` carries +// UNIQUE (org_id, email) over a `citext` column, so a repeat is a 409 from the +// database rather than a check this code could race against. +// +// Recording a SECOND role for someone who already exists is a different +// operation and stays where it was: POST /api/v1/employee-roles, which takes +// the worker's email and does not touch worker_profiles. +func (s *WorkflowService) CreateWorkerWithRole(ctx context.Context, ident authctx.Identity, + body domain.Record) (*WorkerWithRoleResult, error) { + + profiles, err := resourceByPath("worker-profiles") + if err != nil { + return nil, err + } + roles, err := resourceByPath("employee-roles") + if err != nil { + return nil, err + } + + // The two fields that identify the person. Both are the caller's to supply: + // an email is never derived from a name, and a missing one is asked for + // rather than invented. + name, _ := body["full_name"].(string) + email, _ := body["email"].(string) + if strings.TrimSpace(name) == "" || strings.TrimSpace(email) == "" { + return nil, domain.Validation( + "a new worker needs both a name and an email address", + map[string]string{"full_name": "required", "email": "required"}) + } + + // The role's own fields, taken from a nested object so that a key meant for + // the role can never land on the worker or the other way about. + roleBody, _ := body["role"].(map[string]any) + if roleBody == nil { + roleBody = domain.Record{} + } + + var out WorkerWithRoleResult + err = s.inTx(ctx, func(tx pgx.Tx) error { + workerRecord := domain.Record{"full_name": name, "email": email} + for _, k := range []string{"phone", "availability", "certifications", "experience_years", "skills"} { + if v, ok := body[k]; ok { + workerRecord[k] = v + } + } + + worker, err := repo.New(profiles, tx).Insert(ctx, ident, workerRecord) + if err != nil { + // A duplicate identity arrives here as the contract's conflict, + // translated by the repository from the unique violation. Returned + // as it is: the transaction rolls back, so no role is written for a + // worker that was never created. + return err + } + + // The role points at the worker this transaction just made, never at + // one found by name. org_id and created_by are the repository's, from + // the session. + roleRecord := domain.Record{} + for k, v := range roleBody { + roleRecord[k] = v + } + roleRecord["worker_profile_id"] = worker["id"] + roleRecord["worker_email"] = worker["email"] + roleRecord["worker_name"] = worker["full_name"] + + role, err := repo.New(roles, tx).Insert(ctx, ident, roleRecord) + if err != nil { + return err + } + + out = WorkerWithRoleResult{Worker: worker, Role: role} + return nil + }) + if err != nil { + return nil, err + } + return &out, nil +} + // Hire moves an application to `hired` and creates the staff record, atomically. // // Replaces the two-call sequence at krowHooks.js:302-303. The failure that diff --git a/seed/fixtures/seed.json b/seed/fixtures/seed.json index 8b82698..b9c17a3 100644 --- a/seed/fixtures/seed.json +++ b/seed/fixtures/seed.json @@ -13,6 +13,34 @@ "emailDigest": true } }, + "users": [ + { + "id": "user_demo", + "full_name": "Alex Rivera", + "email": "demo@krow.app", + "role": "admin", + "account_type": "employer", + "created_date": "2026-06-01T09:00:00.000Z", + "preferences": { + "owliverDefault": true, + "compactDensity": false, + "emailDigest": true + } + }, + { + "id": "user_employer", + "full_name": "Jordan Blake", + "email": "employer@krow.app", + "role": "employer", + "account_type": "employer", + "created_date": "2026-06-01T09:00:00.000Z", + "preferences": { + "owliverDefault": true, + "compactDensity": false, + "emailDigest": true + } + } + ], "entities": { "JobPosting": [ { @@ -5020,6 +5048,19 @@ "compactDensity": false, "emailDigest": true } + }, + { + "id": "user_employer", + "full_name": "Jordan Blake", + "email": "employer@krow.app", + "role": "employer", + "account_type": "employer", + "created_date": "2026-06-01T09:00:00.000Z", + "preferences": { + "owliverDefault": true, + "compactDensity": false, + "emailDigest": true + } } ] } diff --git a/skills/create-employee-role.md b/skills/create-employee-role.md index 65a89ca..9bfea38 100644 --- a/skills/create-employee-role.md +++ b/skills/create-employee-role.md @@ -57,13 +57,18 @@ which already carry the funnel, the interview and the outcome. Each line is `field | question | suggestions | required?`. Suggestions beginning with `@` come from the application's own data. -`@workers` is the worker profiles already on screen for this organization. -Picking one records the role against that person's profile and email; typing an -email address that has no profile yet also works, because a role can be declared -before a profile exists. The worker is always asked for and is never assumed to -be whoever is typing — an operator records this on somebody's behalf. +This creates a NEW employee. The name is not looked up — an organization may +employ several people who share one, so a name selects nobody — and the email is +asked for outright and never derived from the name, from the operator's account +or from an earlier conversation. The email is the identity: `worker_profiles` +carries UNIQUE (org_id, email), so the database decides whether this person +already exists. -- worker | Which worker is this role for? Type their name or email. | @workers | required +Recording another role for somebody who is already on file is a different +request and is not this flow. + +- worker_name | What is the new employee’s full name? | | required +- worker_email | What is their email address? | | required - role_category | What role do they work as? | @roles | required - experience_years | How much experience do they have? | No experience; 1 year; 2 years; 3+ years | optional - english_level | What is their English level? | @english | optional