Skip to content

fix(api): api v1 review follow-ups (round trip, key revocation, docs) - #230

Merged
josue-commits merged 9 commits into
developfrom
fix/api-v1-review-followups
Oct 13, 2026
Merged

fix(api): api v1 review follow-ups (round trip, key revocation, docs)#230
josue-commits merged 9 commits into
developfrom
fix/api-v1-review-followups

Conversation

@josue-commits

@josue-commits josue-commits commented Oct 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Follow-ups from the independent review of API v1 wave 2 (#227, #228, #229, all in develop).

  1. A form built in the editor reads and saves back unchanged with a key.
    • message and scheduler steps may have a blank title, like reveal. The editor never refuses a blank title (its only save gate is formConfigSchema, where question is optional), so the line drawn is "steps that still make a whole screen without a title": a reveal has its headline, a message its body (helper), a scheduler the booking page. Justified in a comment on TITLE_OPTIONAL_TYPES.
    • branding.clientLogos passes when it equals the kit OR what the form already stores (live or draft), so a kit changed after the form was made no longer breaks a GET then PUT.
    • Same "stored value passes" rule for sliderUnitLabel: three templates seed it and the canvas shows it, but no control edits it. A new or changed value is still UNSUPPORTED_FIELD.
    • The "Customer feedback" template carried the legacy form-level reveal (config.reveal + triggersReveal), which a key refuses. It now carries a reveal step, the shape the builder folds it into on open.
    • Legacy fields the editor never produces still answer UNSUPPORTED_FIELD.
  2. A key follows its member. Disabling or removing a member, and the identity service's roster marking them inactive or dropping them, revokes their keys in the same transaction (runAtomically + revokeKeysOfInactiveMembersSql). Reactivating does not revive them. GET /v1/api-keys only lists keys of active members.
  3. Skill and "How to use the API" section up to date. Error paths with the body prefix (config.steps.N.flowGroup, destinations.N.settings.timeoutMs), accepted editor defaults (also pinned in the JSON Schema, so a config read back validates), steps that need no question, PUT /v1/forms/:id ignores destinations, previousPublishedAt, revert with no body answering 200. Section in EN and ES. The API keys changeset now names the event forms_api_key_created.

How it was tested

  • New apps/api/src/api-key-round-trip.spec.ts: every template in packages/db/src/templates, the seeded form (before and after a publish) and a form with untitled message, reveal and scheduler steps make GET then PUT unchanged with a key (200); a kit change after creation still saves; legacy fields and a changed slider unit are still refused.
  • New packages/db/src/api-keys-members.spec.ts: setMemberStatus, removeMember, projectRoster and projectMemberships revoke; reactivation does not revive; the list hides inactive members' keys. Controller test in api-keys.controller.spec.ts.
  • Skill and docs specs: paths, defaults, ignored destinations, previousPublishedAt, revert, schema accepts a normalized config.
  • pnpm typecheck, pnpm lint (2 pre-existing warnings) and pnpm test green on Node 22: api 823, web 1313, db 386, engine 436, destinations 125, types 59, shared 83, notifications 57.
  • Full @quill/db suite on Postgres 16 (386/386).
  • PUBLISH_GATE_LOCAL=1 pnpm run publish-gate passed; scripts/dash-check.sh origin/develop clean.

Notes

  • Keys of members disabled before this change stay unrevoked in the DB (hidden from the list, 401 while inactive). A data migration could revoke them; left out to avoid a migration number race with parallel work.
  • The forms_api_key_created event name is fixed in the changeset only: it is internal telemetry, so the customer-facing skill and section do not mention it.

Review fixes (added after review)

  • fix(api): disabling a member through the identity service now disables the local row and revokes their API keys in the same step, instead of relying on the roster re-read. Covered by a test where the roster read fails.
  • fix(web): the skill's JSON Schema accepts steps[].sliderUnitLabel and branding.clientLogos as read-back fields, so configs read from templates validate.
  • Test that runAtomically rolls back the member disable and the revoke together (SQLite and Postgres).

Known follow-up: keys of members disabled before this change are not backfilled as revoked; that needs a separate migration.

🤖 Generated with Claude Code

josue-commits and others added 7 commits October 12, 2026 18:33
@josue-commits @claude
fix(db): give the customer feedback template a reveal step
edd744b 
The template still carried the old form-level reveal (config.reveal plus
triggersReveal), which the builder folds into a reveal step on open and an
API key refuses. Author it in the builder's shape so a form made from it
can be read and saved back unchanged with a key.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Josue Hernandez <josue@daptatech.com>
@josue-commits @claude
fix(api): let a key save back any config the editor produces
9f25ab4 
With an API key, a form read with GET now saves back unchanged with PUT:
- message and scheduler steps may have a blank title, like reveal (the
  editor saves them that way);
- branding.clientLogos and a slider's sliderUnitLabel pass when they equal
  what the form already stores, so a brand kit changed after the form was
  made, or a template's slider unit, no longer breaks the round trip.

A test walks every template, the seeded form and an untitled content form
through GET then PUT with a key. Legacy fields still answer
UNSUPPORTED_FIELD.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Josue Hernandez <josue@daptatech.com>
@josue-commits @claude
fix(db): revoke a member's API keys when they leave the workspace
8970126 
Disabling or removing a member, and the identity service's roster marking
them inactive or no longer naming them, now revokes their API keys in the
same transaction. Reactivating the member later does not bring the old
keys back. The key list also hides keys of members who are not active.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Josue Hernandez <josue@daptatech.com>
@josue-commits @claude
docs(web): bring the api skill up to date with the v1 review fixes
dc9006b 
- Error paths carry the body prefix (config.steps.N.flowGroup, and
  destinations.N.settings.timeoutMs on the destinations route).
- The editor defaults are accepted (derived flowGroup, buttonText
  "Continue", reveal.enabled, scheduler.prefill), and the JSON Schema
  keeps them pinned to that value so a config read back validates.
- Reveal, message and scheduler steps need no question.
- PUT /v1/forms/{id} ignores destinations.
- previousPublishedAt on every form; revert takes no body and answers
  200 with the form.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Josue Hernandez <josue@daptatech.com>
@josue-commits @claude
docs(shared): explain error paths, editor defaults and revert in the …
9d33841 
…api section

The "How to use the API" section of the Developers page gains two notes
(error paths with the body prefix; the editor defaults a key may send back
and the steps that need no question), says PUT /v1/forms/{id} ignores
destinations, and that a revert takes no body, answers 200 with the form
and that every form carries previousPublishedAt. English and Spanish.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Josue Hernandez <josue@daptatech.com>
@josue-commits @claude
chore: changeset for the api v1 review follow-ups
38633bb 
Also names the key creation event as PostHog receives it,
forms_api_key_created, in the API keys changeset.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Josue Hernandez <josue@daptatech.com>
@josue-commits @claude
docs(api): say in openapi that a member's keys go with them
261cbdf 
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Josue Hernandez <josue@daptatech.com>
josue-commits and others added 2 commits October 12, 2026 18:47
@josue-commits @claude
fix(api): revoke a member's keys as soon as they are disabled upstream
2358b04 
Disabling a member through the identity service only reached the local
row through the roster re-read that follows, which swallows its errors.
When that read failed or lagged, the member stayed active here and their
API keys kept working. Apply the disable locally right after the upstream
write, so the keys are revoked in the same transaction.

Also pin runAtomically's rollback with a test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Josue Hernandez <josue@daptatech.com>
@josue-commits @claude
fix(web): let the skill's config schema take template and kit fields …
705c92c 
…back

A form made from a template carries sliderUnitLabel on its sliders, and
the brand kit writes branding.clientLogos. The API takes both back as
read and the skill tells the agent to send them back, but the shipped
JSON Schema dropped them, so a config read from such a form failed
validation. Keep their shape, with a description that says to send them
back unchanged and never add or change them.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Josue Hernandez <josue@daptatech.com>
@josue-commits
josue-commits merged commit d2261c2 into develop Oct 13, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@feggome1 feggome1 Awaiting requested review from feggome1 feggome1 is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@josue-commits