feat(mktg-os): brand kit session-consumer persistence endpoint - #1317
feat(mktg-os): brand kit session-consumer persistence endpoint#1317joanreyero wants to merge 3 commits into
Conversation
…FXV2-2059) Implements the BFF write path of dec-brand-kit-storage-v1: POST /api/mktg-agents/brand-kit/persist scans the Guild session's raw events for the finalize_brand_kit tool-result envelope (brand-kit-output/v1), validates schema + 12-heading structural gate, recomputes content sha256, enforces the 20 MB cap, and PutObjects the raw markdown to the content-addressed key brand-kit/{project}/{sha256}.md. Graph Artifact writes deferred per A5 ruling; receipt returns {s3_key, content_sha256, project, version, intake_mode}. Signed-off-by: Joan Reyero <joan@reyero.io>
- Sort raw Guild events chronologically and warn on a full (possibly truncated) page so last-valid-wins selection is deterministic; prefer highest envelope version as tiebreaker. - Type-gate ownerToken from the request body. - Enforce intake question order (question_number === index+1) and an ISO-8601 shape gate on completed_at. - Cap envelope-extraction recursion depth; fix untruthful comments (recursion depth, byte-accurate size check). - Preserve non-Error throws in object-store error wrapping; document the benign HEAD/PUT race; use node:crypto specifier. - Add BrandKitIntakeMode alias; extend spec (18 tests). Signed-off-by: Joan Reyero <joan@reyero.io>
Signed-off-by: Joan Reyero <joan@reyero.io>
PR SummaryMedium Risk Overview The handler loads unfiltered Guild session events via new
Reviewed by Cursor Bugbot for commit f12fdf3. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f12fdf3. Configure here.
| const parsed = JSON.parse(objectText) as Record<string, unknown>; | ||
| if (parsed['contract'] === BRAND_KIT_CONTRACT_ID) { | ||
| found.push(parsed as unknown as BrandKitEnvelope); | ||
| } |
There was a problem hiding this comment.
Stream scan misses envelope_json
Medium Severity
scanForEnvelopeObjects only accepts objects with a top-level contract field and never unwraps envelope_json. Non-JSON stream bodies that embed the authoritative finalize tool-result shape therefore yield no candidates, even though that encoding is the documented A3 path for llm_start/llm_done payloads.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit f12fdf3. Configure here.
There was a problem hiding this comment.
Pull request overview
Adds a BFF endpoint for validating Guild Brand Kit output and persisting it to S3-compatible object storage.
Changes:
- Adds shared Brand Kit contracts, validation, extraction, and tests.
- Adds Guild event retrieval and Brand Kit persistence services.
- Adds the authenticated persistence route and AWS S3 dependency.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
yarn.lock |
Locks the S3 client dependency. |
packages/shared/src/utils/index.ts |
Exports Brand Kit utilities. |
packages/shared/src/utils/brand-kit.utils.ts |
Validates and extracts Brand Kit envelopes. |
packages/shared/src/utils/brand-kit.utils.spec.ts |
Tests shared Brand Kit utilities. |
packages/shared/src/interfaces/index.ts |
Exports Brand Kit interfaces. |
packages/shared/src/interfaces/brand-kit.interface.ts |
Defines envelope, request, and receipt contracts. |
packages/shared/src/constants/index.ts |
Exports Brand Kit constants. |
packages/shared/src/constants/brand-kit.constants.ts |
Defines validation and storage constraints. |
apps/lfx-one/src/server/services/object-store.service.ts |
Implements idempotent S3 writes. |
apps/lfx-one/src/server/services/guild.service.ts |
Fetches raw Guild session events. |
apps/lfx-one/src/server/services/brand-kit.service.ts |
Selects, validates, and persists Brand Kits. |
apps/lfx-one/src/server/routes/mktg-agents.route.ts |
Registers the persistence endpoint. |
apps/lfx-one/src/server/controllers/mktg-agents.controller.ts |
Validates ownership and handles persistence requests. |
apps/lfx-one/package.json |
Adds the AWS S3 client dependency. |
Suppressed comments (2)
apps/lfx-one/src/server/services/brand-kit.service.ts:104
- Every raw event is scanned as if it were trusted producer output.
getRawEventPayloadsincludestrigger_messageanduser_message, while/chatlets the session owner submit arbitrary text, so a follow-up containing a valid envelope JSON will be extracted and persisted without ever coming fromfinalize_brand_kit. This defeats the stated provenance boundary and lets any authenticated session owner write arbitrary Markdown under any allowed project slug. Retain the Guild event/tool metadata and accept only the verifiedfinalize_brand_kittool-result payload before extracting envelopes.
for (const payload of payloads) {
for (const candidate of extractBrandKitEnvelopeCandidates(payload)) {
apps/lfx-one/src/server/services/brand-kit.service.ts:109
- Hash equality is checked only after candidate selection. A higher-version candidate with a well-formed but incorrect
content_sha256passes this validator, replaces a lower fully valid candidate, and then makespersistFromSessionfail instead of falling back to the valid draft. Include the recomputed hash in candidate validity before comparing versions.
const result = validateBrandKitEnvelope(candidate);
if (result.valid) {
if (!best || candidate.version >= best.version) {
best = candidate;
| * posting a follow-up. | ||
| */ | ||
| public async persistBrandKit(req: Request, res: Response, next: NextFunction): Promise<void> { | ||
| const { sessionId, ownerToken } = req.body as BrandKitPersistRequest; |
| public async persistFromSession(req: Request, sessionId: string): Promise<BrandKitPersistReceipt> { | ||
| const payloads = await this.guildService.getRawEventPayloads(req, sessionId); | ||
|
|
||
| const envelope = this.findAuthoritativeEnvelope(req, payloads); |
| const record = value as Record<string, unknown>; | ||
| if (record['contract'] === BRAND_KIT_CONTRACT_ID) { | ||
| candidates.push(record as unknown as BrandKitEnvelope); |
|
Closed per Joan's direction — simplifying v1: the agent-invocation UI ships first WITHOUT object-storage persistence (deferred, not dropped; the branch is preserved as salvage for when persistence returns). |
Summary
Implements the BFF write path of dec-brand-kit-storage-v1 (LFXV2-2059, WorkItem
wi-brand-kit-bff-persist):POST /api/mktg-agents/brand-kit/persistvalidates a Guild session's final Brand Kit document against thebrand-kit-output/v1contract and persists it to versioned object storage — the agent holds no credentials.What it does
GuildService.getRawEventPayloads, unfiltered, chronologically sorted). Per the live-smoke A3 verdict, the authoritative envelope is thefinalize_brand_kittool result riding system events — the__submit__-ed text may be prose or abridged and is never trusted.envelope_jsondouble-encoding, and balanced-object scanning of non-JSON stream bodies; recursion depth-capped; highestversionwins, latest occurrence breaks ties.document_markdownbytes (text/markdown; charset=utf-8) to the content-addressed keybrand-kit/{project}/{content_sha256}.md— key derived from validated fields only. Content addressing makes the write idempotent (HEAD-then-PUT; race benign by construction).{s3_key, content_sha256, project, version, intake_mode}. Graph Artifact writes are deferred per the A5 ruling (wi-lfx-one-service-actor); the receipt carries exactly the fields needed for later Artifact minting.Auth: endpoint sits behind the global auth middleware; only the session's creator may persist (same HMAC owner-token proof as posting a follow-up, constant-time compare).
Storage env contract: standard
S3_BUCKET/AWS_REGION/S3_ENDPOINT_URL(MinIO local via endpoint override + path-style; AWS+IRSA deployed via default credential chain — no credential-mode branching in code). New direct dependency:@aws-sdk/client-s3.Testing
019fcd89-ecd8-f268-0000-8e14e8b4b22b: envelope recovered from the finalize tool result (2 abridged candidates correctly rejected), persisted to local MinIO (19,045 bytes), idempotent re-run returnedwritten: false, GetObject readback sha verified.yarn build,yarn check-types,yarn lint:check,yarn format:check,./check-headers.shall pass.Documented trade-offs
server/services/*.spec.ts); all non-trivial pure logic lives in the fully-tested shared utils, and the service was exercised end-to-end live.meeting-privacy.utils.spec.ts(missing vitest imports, from feat(meetings): add separate visibility and join restriction controls #1093) is unrelated to this branch.Protected files
apps/lfx-one/package.json/yarn.lock— single dependency addition (@aws-sdk/client-s3), no script or tooling changes.External references
marketing-os-agents/docs/contracts/brand-kit-output.{md,schema.json}(normative; the shared interface is its transcription)linux-foundation~brand-kit(Guild), deployed underwi-brand-kit-guild-deploy