Skip to content

feat(content): add contribution intake and promotion workflows#1037

Merged
willgriffin merged 4 commits intomainfrom
codex/pr-1033-facts-review
Mar 20, 2026
Merged

feat(content): add contribution intake and promotion workflows#1037
willgriffin merged 4 commits intomainfrom
codex/pr-1033-facts-review

Conversation

@willgriffin
Copy link
Copy Markdown
Contributor

Summary

  • add an opt-in contribution holding layer to smrt-content for inbound community submissions
  • normalize web and email intake into held contributions with contributors, revisions, attachments, and app-defined submission types
  • support approval and promotion into normal draft Content and Asset records with provenance, while keeping governance opt-in after promotion
  • export reusable contribution intake and admin Svelte components for consuming apps

What Changed

  • added contribution models and config: ContentContribution, ContentContributionRevision, ContentContributionAttachment, ContentContributionType, ContentContributor, and configureContentContributions()
  • generated new contribution API surfaces for submit, inbox, email ingest, revision history, contributor lookup, and promotion actions
  • extended the mock client, serializers, README, and package exports for contribution workflows
  • added contribution form, inbox, portal, type manager, and contributor manager components
  • wired smrt-messages and smrt-profiles into the package for email normalization and contributor identity resolution

Verification

  • cd packages/content && pnpm exec svelte-kit sync
  • cd packages/content && pnpm exec tsc --noEmit
  • cd packages/content && pnpm exec svelte-check --tsconfig ./tsconfig.json
  • cd packages/content && pnpm exec vitest run
  • cd packages/content && pnpm exec vitest run --coverage
  • cd packages/content && pnpm exec vite build

Coverage

  • statements: 82.00%
  • lines: 80.90%
  • functions: 80.83%
  • branches: 60.71%

Copilot AI review requested due to automatic review settings March 20, 2026 19:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces contribution intake + promotion workflows to smrt-content, expands governance/transparency/versioning infrastructure, and adds supporting SvelteKit routes/components plus CLI export improvements.

Changes:

  • Added contribution holding models, serializers, and exported APIs/components for intake/inbox/promotion flows.
  • Added content governance, transparency snapshots, content versions/reviews/corrections, and a published article page.
  • Updated CLI export to project/format fields more reliably and added tests + documentation/testing workflow updates.

Reviewed changes

Copilot reviewed 165 out of 222 changed files in this pull request and generated 12 comments.

Show a summary per file
File Description
packages/content/src/svelte/components/ContentContributionForm.svelte New contribution submission form component (types, contributor fields, file attach).
packages/content/src/svelte/components/ArticleList.svelte Fix Article type import path.
packages/content/src/svelte/components/ArticleCard.svelte Fix Article type import path and tighten tag parsing types.
packages/content/src/serialization.ts New centralized serializer helpers for content/governance/contribution objects.
packages/content/src/routes/articles/[slug]/+page.ts New published-article loader route (by-slug + transparency fetch).
packages/content/src/routes/articles/[slug]/+page.svelte New published article page UI with transparency sidebar.
packages/content/src/routes/api/v1/images/[id]/edit/+server.ts Add mock “AI variation” image edit endpoint.
packages/content/src/routes/api/v1/images/[id]/+server.ts Add CRUD endpoint for images by id.
packages/content/src/routes/api/v1/images/+server.ts Add list/create images endpoint + dev seeding behavior.
packages/content/src/routes/api/v1/contentversions/restoreIntoContent/+server.ts Auto-generated ContentVersion custom method route.
packages/content/src/routes/api/v1/contentversions/listForContent/+server.ts Auto-generated ContentVersion custom method route.
packages/content/src/routes/api/v1/contentversions/getVersion/+server.ts Auto-generated ContentVersion custom method route.
packages/content/src/routes/api/v1/contentversions/getNextVersionNumber/+server.ts Auto-generated ContentVersion custom method route.
packages/content/src/routes/api/v1/contentversions/getLatestPublishedForContent/+server.ts Auto-generated ContentVersion custom method route.
packages/content/src/routes/api/v1/contentversions/getLatestForContent/+server.ts Auto-generated ContentVersion custom method route.
packages/content/src/routes/api/v1/contentversions/createSnapshot/+server.ts Auto-generated ContentVersion custom method route.
packages/content/src/routes/api/v1/contentversions/[id]/transparency/+server.ts Auto-generated ContentVersion transparency action route.
packages/content/src/routes/api/v1/contentversions/[id]/+server.ts Auto-generated ContentVersion get-by-id route.
packages/content/src/routes/api/v1/contentversions/+server.ts Auto-generated ContentVersion list/create routes.
packages/content/src/routes/api/v1/contents/serialize.ts Re-export serializers for server routes.
packages/content/src/routes/api/v1/contents/governance/resolve/+server.ts Auto-generated Contents resolve governance route.
packages/content/src/routes/api/v1/contents/governance/+server.ts Auto-generated Contents governance definitions route.
packages/content/src/routes/api/v1/contents/facts/+server.ts Auto-generated Contents facts browse route.
packages/content/src/routes/api/v1/contents/by-slug/+server.ts Auto-generated Contents get-by-slug route.
packages/content/src/routes/api/v1/contents/[id]/versions/+server.ts Auto-generated Content version actions routes.
packages/content/src/routes/api/v1/contents/[id]/transparency/preview/+server.ts Auto-generated Content transparency preview route.
packages/content/src/routes/api/v1/contents/[id]/transparency/+server.ts Auto-generated Content published transparency route.
packages/content/src/routes/api/v1/contents/[id]/reviews/+server.ts Auto-generated Content review list/run routes.
packages/content/src/routes/api/v1/contents/[id]/review-profiles/[profileKey]/+server.ts Auto-generated Content review profile evaluation route.
packages/content/src/routes/api/v1/contents/[id]/review-profiles/+server.ts Auto-generated Content review profile list route.
packages/content/src/routes/api/v1/contents/[id]/governance/+server.ts Auto-generated Content governance state route.
packages/content/src/routes/api/v1/contents/[id]/facts/+server.ts Auto-generated Content facts state routes.
packages/content/src/routes/api/v1/contents/[id]/corrections/+server.ts Auto-generated Content corrections list/issue routes.
packages/content/src/routes/api/v1/contents/[id]/chat/+server.ts New content-chat API route integrating smrt-chat.
packages/content/src/routes/api/v1/contents/[id]/+server.ts Auto-generated Content get/update/delete routes using serializers.
packages/content/src/routes/api/v1/contents/+server.ts Auto-generated Content list/create routes using serializers.
packages/content/src/routes/api/v1/contentreviews/listForContentByPolicyKey/+server.ts Auto-generated ContentReview custom method route.
packages/content/src/routes/api/v1/contentreviews/listForContent/+server.ts Auto-generated ContentReview custom method route.
packages/content/src/routes/api/v1/contentreviews/getLatestForPolicyKey/+server.ts Auto-generated ContentReview custom method route.
packages/content/src/routes/api/v1/contentreviews/getLatestForContent/+server.ts Auto-generated ContentReview custom method route.
packages/content/src/routes/api/v1/contentreviews/createFromResult/+server.ts Auto-generated ContentReview custom method route.
packages/content/src/routes/api/v1/contentreviews/[id]/+server.ts Auto-generated ContentReview get/update routes.
packages/content/src/routes/api/v1/contentreviews/+server.ts Auto-generated ContentReview list/create routes.
packages/content/src/routes/api/v1/contentreferences/unlink/+server.ts Auto-generated ContentReference custom method route.
packages/content/src/routes/api/v1/contentreferences/link/+server.ts Auto-generated ContentReference custom method route.
packages/content/src/routes/api/v1/contentreferences/getForTarget/+server.ts Auto-generated ContentReference custom method route.
packages/content/src/routes/api/v1/contentreferences/getForSource/+server.ts Auto-generated ContentReference custom method route.
packages/content/src/routes/api/v1/contentreferences/[id]/+server.ts Auto-generated ContentReference CRUD routes.
packages/content/src/routes/api/v1/contentreferences/+server.ts Auto-generated ContentReference list/create routes.
packages/content/src/routes/api/v1/contentgovernanceprofiles/getByKey/+server.ts Auto-generated ContentGovernanceProfile custom method route.
packages/content/src/routes/api/v1/contentgovernanceprofiles/[id]/+server.ts Auto-generated ContentGovernanceProfile CRUD routes.
packages/content/src/routes/api/v1/contentgovernanceprofiles/+server.ts Auto-generated ContentGovernanceProfile list/create routes.
packages/content/src/routes/api/v1/contentgovernancepolicies/getByKey/+server.ts Auto-generated ContentGovernancePolicy custom method route.
packages/content/src/routes/api/v1/contentgovernancepolicies/[id]/+server.ts Auto-generated ContentGovernancePolicy CRUD routes.
packages/content/src/routes/api/v1/contentgovernancepolicies/+server.ts Auto-generated ContentGovernancePolicy list/create routes.
packages/content/src/routes/api/v1/contentgovernanceassignments/resolveForContent/+server.ts Auto-generated ContentGovernanceAssignment custom method route.
packages/content/src/routes/api/v1/contentgovernanceassignments/getByKey/+server.ts Auto-generated ContentGovernanceAssignment custom method route.
packages/content/src/routes/api/v1/contentgovernanceassignments/[id]/+server.ts Auto-generated ContentGovernanceAssignment CRUD routes.
packages/content/src/routes/api/v1/contentgovernanceassignments/+server.ts Auto-generated ContentGovernanceAssignment list/create routes.
packages/content/src/routes/api/v1/contentcorrections/listForContent/+server.ts Auto-generated ContentCorrection custom method route.
packages/content/src/routes/api/v1/contentcorrections/issue/+server.ts Auto-generated ContentCorrection custom method route.
packages/content/src/routes/api/v1/contentcorrections/getPublishedForContent/+server.ts Auto-generated ContentCorrection custom method route.
packages/content/src/routes/api/v1/contentcorrections/[id]/+server.ts Auto-generated ContentCorrection CRUD routes.
packages/content/src/routes/api/v1/contentcorrections/+server.ts Auto-generated ContentCorrection list/create routes.
packages/content/src/routes/api/v1/contentcontributors/getByProfileId/+server.ts Auto-generated ContentContributor custom method route.
packages/content/src/routes/api/v1/contentcontributors/getByEmail/+server.ts Auto-generated ContentContributor custom method route.
packages/content/src/routes/api/v1/contentcontributors/findOrCreateByEmail/+server.ts Auto-generated ContentContributor custom method route.
packages/content/src/routes/api/v1/contentcontributors/[id]/+server.ts Auto-generated ContentContributor CRUD routes.
packages/content/src/routes/api/v1/contentcontributors/+server.ts Auto-generated ContentContributor list/create routes.
packages/content/src/routes/api/v1/contentcontributiontypes/getByKey/+server.ts Auto-generated ContentContributionType custom method route.
packages/content/src/routes/api/v1/contentcontributiontypes/[id]/+server.ts Auto-generated ContentContributionType CRUD routes.
packages/content/src/routes/api/v1/contentcontributiontypes/+server.ts Auto-generated ContentContributionType list/create routes.
packages/content/src/routes/api/v1/contentcontributions/types/+server.ts Auto-generated ContentContribution types action route.
packages/content/src/routes/api/v1/contentcontributions/submit/+server.ts Auto-generated contribution web submit route.
packages/content/src/routes/api/v1/contentcontributions/ingest-email/+server.ts Auto-generated contribution email ingest route.
packages/content/src/routes/api/v1/contentcontributions/inbox/+server.ts Auto-generated contribution inbox listing route.
packages/content/src/routes/api/v1/contentcontributions/by-contributor/+server.ts Auto-generated contribution listing-by-contributor route.
packages/content/src/routes/api/v1/contentcontributions/[id]/withdraw/+server.ts Auto-generated contribution action route.
packages/content/src/routes/api/v1/contentcontributions/[id]/revisions/+server.ts Auto-generated contribution revision append route.
packages/content/src/routes/api/v1/contentcontributions/[id]/request-changes/+server.ts Auto-generated contribution action route.
packages/content/src/routes/api/v1/contentcontributions/[id]/reject/+server.ts Auto-generated contribution action route.
packages/content/src/routes/api/v1/contentcontributions/[id]/promote/+server.ts Auto-generated contribution action route.
packages/content/src/routes/api/v1/contentcontributions/[id]/approve/+server.ts Auto-generated contribution action route.
packages/content/src/routes/api/v1/contentcontributions/[id]/+server.ts Auto-generated ContentContribution CRUD routes.
packages/content/src/routes/api/v1/contentcontributions/+server.ts Auto-generated ContentContribution list/create routes.
packages/content/src/routes/api/v1/contentcontributionrevisions/listForContribution/+server.ts Auto-generated ContentContributionRevision custom method route.
packages/content/src/routes/api/v1/contentcontributionrevisions/getLatestForContribution/+server.ts Auto-generated ContentContributionRevision custom method route.
packages/content/src/routes/api/v1/contentcontributionrevisions/[id]/+server.ts Auto-generated ContentContributionRevision CRUD routes.
packages/content/src/routes/api/v1/contentcontributionrevisions/+server.ts Auto-generated ContentContributionRevision list/create routes.
packages/content/src/routes/api/v1/contentcontributionattachments/listForRevision/+server.ts Auto-generated ContentContributionAttachment custom method route.
packages/content/src/routes/api/v1/contentcontributionattachments/listForContribution/+server.ts Auto-generated ContentContributionAttachment custom method route.
packages/content/src/routes/api/v1/contentcontributionattachments/[id]/+server.ts Auto-generated ContentContributionAttachment CRUD routes.
packages/content/src/routes/api/v1/contentcontributionattachments/+server.ts Auto-generated ContentContributionAttachment list/create routes.
packages/content/src/publish-readiness.ts Add publish-readiness evaluation logic for governance publication gating.
packages/content/src/publish-readiness.test.ts Unit tests for publish-readiness evaluation behavior.
packages/content/src/lib/server/smrt.ts Centralize SMRT config + collection accessor for server runtime.
packages/content/src/lib/server/smrt-register.ts Auto-generated SMRT object registration for server runtime.
packages/content/src/lib/server/seed-images.ts Add server-side image seeding helper.
packages/content/src/lib/server/seed-contents.ts Add server-side content seeding helper.
packages/content/src/lib/server/content-api-serializers.ts Server serializer wrapper for Content API routes.
packages/content/src/index.ts Expand public exports for contributions, governance, versions, transparency, reviews, etc.
packages/content/src/hooks.server.ts Add server hook to bootstrap DB schema at runtime startup.
packages/content/src/contents.ts Add collection APIs for facts browsing, by-slug lookup, governance definitions/resolve.
packages/content/src/contents.spec.ts Add integration test for editor-style assetIds syncing.
packages/content/src/content-versions.ts Add ContentVersion collection helpers (snapshot, restore).
packages/content/src/content-version.ts Add ContentVersion SMRT object + transparency action.
packages/content/src/content-types.ts Disable API/MCP/CLI surfaces on STI subclasses.
packages/content/src/content-transparency.ts Add transparency normalization utilities + types.
packages/content/src/content-reviews.ts Add ContentReview collection helpers.
packages/content/src/content-review.ts Add ContentReview SMRT object.
packages/content/src/content-governance-profiles.ts Add governance profile collection helper(s).
packages/content/src/content-governance-profile.ts Add persisted governance profile model + validation + deletion guards.
packages/content/src/content-governance-policy.ts Add persisted governance policy model + deletion guards.
packages/content/src/content-governance-policies.ts Add governance policy collection helper(s).
packages/content/src/content-governance-assignments.ts Add governance assignment collection helper(s).
packages/content/src/content-governance-assignment.ts Add persisted governance assignment model + validation.
packages/content/src/content-corrections.ts Add ContentCorrection collection helpers.
packages/content/src/content-correction.ts Add ContentCorrection SMRT object.
packages/content/src/content-contributors.ts Add ContentContributor collection helpers + profile resolution.
packages/content/src/content-contributor.ts Add ContentContributor SMRT object + metadata helpers.
packages/content/src/content-contribution-types.ts Add ContentContributionType collection helper(s).
packages/content/src/content-contribution-type.ts Add persisted contribution type model + validation/deletion guard.
packages/content/src/content-contribution-revisions.ts Add ContentContributionRevision collection helper(s).
packages/content/src/content-contribution-revision.ts Add ContentContributionRevision SMRT object.
packages/content/src/content-contribution-attachments.ts Add ContentContributionAttachment collection helper(s).
packages/content/src/content-contribution-attachment.ts Add ContentContributionAttachment SMRT object.
packages/content/src/app/main.ts Remove prior non-SvelteKit app entrypoint.
packages/content/src/app/app.css Remove prior app CSS stub.
packages/content/src/app.html Add SvelteKit app shell (replaces Vite SPA index).
packages/content/scripts/seed-images.js Add CLI script for seeding images through API.
packages/content/package.json Update scripts + add dependencies for chat/facts/messages/profiles + kit.
packages/content/migrate.mjs Add migration helper for ensuring schemas via ObjectRegistry.
packages/content/index.html Remove prior Vite SPA index.
packages/content/README.md Document governance, transparency, and contribution workflows + exports.
packages/content/.gitignore Ignore generated routes except v1 server handlers.
packages/cli/src/commands/export.ts Improve projection/field naming handling + return-shape normalization + helpers.
packages/cli/src/commands/tests/export.test.ts Add unit tests for export helpers.
packages/chat/src/svelte/components/shared/Avatar.svelte Improve a11y labels/alt + handle blank names safely.
packages/chat/src/svelte/components/agent/message-blocks.ts Add helper to parse agent message fenced blocks into structured blocks.
packages/chat/src/svelte/components/agent/message-blocks.test.ts Add unit tests for message-block parsing.
packages/chat/src/services/ChatService.ts Register/get collections via ObjectRegistry + add initialize hook for tables.
package.json Add Vitest coverage provider dependency.
WORKFLOW.md Expand test strategy checklist and touched-package full-suite guidance.
TESTING_STANDARD.md Add release gate guidance + test pyramid details and coverage expectations.
CONTRIBUTING.md Update quality gate wording and expand test type definitions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1 to +18
function toJSON<T extends Record<string, any>>(value: any): T {
if (value && typeof value.toJSON === 'function') {
return value.toJSON();
}

return value as T;
}

export function serializeFact(fact: any) {
const data = toJSON<Record<string, any>>(fact);
return {
...data,
metadata:
typeof fact?.getMetadata === 'function'
? fact.getMetadata()
: data.metadata || {},
};
}
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

serializeFact (and the other serialize* helpers) assumes toJSON() returns an object that can be spread. If fact is null/undefined or toJSON() returns a non-object (e.g., null), { ...data } will throw at runtime. Make toJSON (or each caller) coerce non-object values to {} before spreading (e.g., return (value && typeof value === 'object' ? value : {}) as T).

Copilot uses AI. Check for mistakes.
Comment on lines +7 to +13
export const GET: RequestHandler = async () => {
try {
await seedImages();

// Ensure base tables are created before STI queries
await getCollection<any>('@happyvertical/smrt-assets:Asset');
await getCollection<any>('@happyvertical/smrt-assets:AssetAssociation');
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seedImages() is invoked on every GET to /api/v1/images regardless of environment. In production this can unintentionally populate real databases with test images. Gate seeding behind dev (or a dedicated SEED_* env flag) and avoid side effects on read endpoints.

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +22
export const PUT: RequestHandler = async ({ params, request }) => {
const collection = await getCollection<any>(
'@happyvertical/smrt-images:Image',
);
const item = await collection.get(params.id);
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The GET handler ensures the base STI tables (Asset, AssetAssociation) exist before querying images, but PUT/DELETE do not. If STI requires those tables, updates/deletes can fail even when reads succeed. Mirror the same base-table bootstrap in PUT and DELETE (or centralize in a shared helper).

Copilot uses AI. Check for mistakes.
const item = await collection.get(params.id);
if (!item) throw error(404, 'Not found');

const data = await request.json();
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blind Object.assign(item, data) allows clients to overwrite sensitive or server-controlled fields (e.g., id, tenantId, createdAt, parent linkage, etc.). Prefer an allowlist of mutable fields and assign only those, or validate/strip disallowed keys before saving.

Suggested change
const data = await request.json();
const data = await request.json();
// Prevent clients from overwriting sensitive or server-controlled fields
const disallowedKeys = [
'id',
'tenantId',
'createdAt',
'updatedAt',
'deletedAt',
'ownerId',
'userId',
'organizationId',
'orgId',
'parentId',
];
for (const key of disallowedKeys) {
if (key in data) {
delete (data as Record<string, unknown>)[key];
}
}

Copilot uses AI. Check for mistakes.
Comment on lines +9 to +14
): Promise<ContentContributionAttachment[]> {
return this.list({
where: { contributionId },
orderBy: 'createdAt ASC',
});
}
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

orderBy: 'createdAt ASC' likely won’t match the underlying DB column naming conventions used elsewhere in this PR (many queries use snake_case like created_at). If SmrtCollection.list() expects DB column names, ordering may silently fail or error. Use the correct column/field name consistently (e.g., created_at ASC if that is the mapped column).

Copilot uses AI. Check for mistakes.
Comment on lines +50 to +53
let draft = $state(createDraft({}, []));

$effect(() => {
draft = createDraft(initial, types);
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This $effect resets the entire draft whenever initial or types changes. If types is populated asynchronously (or a parent re-renders with a new array reference), this will wipe user-entered form state mid-edit. Consider only initializing draft on first mount or when initial meaningfully changes (e.g., compare keys/ids), and avoid resetting on types updates unless draft.typeKey is invalid.

Suggested change
let draft = $state(createDraft({}, []));
$effect(() => {
draft = createDraft(initial, types);
let draft = $state(createDraft(initial, types));
$effect(() => {
if (!types.length) {
return;
}
const hasValidType = types.some((type) => type.key === draft.typeKey);
if (!hasValidType) {
draft.typeKey = types[0].key;
}

Copilot uses AI. Check for mistakes.
Comment on lines +80 to +83
onsubmit={(event) => {
event.preventDefault();
handleSubmit();
}}
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These handlers are wired via onsubmit/onchange/onclick attributes rather than Svelte event directives. Prefer on:submit|preventDefault, on:change, and on:click for consistency with Svelte patterns and clearer event modifier support.

Suggested change
onsubmit={(event) => {
event.preventDefault();
handleSubmit();
}}
on:submit|preventDefault={handleSubmit}

Copilot uses AI. Check for mistakes.
Comment on lines +127 to +131
<input
type="file"
multiple
onchange={handleFileChange}
/>
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These handlers are wired via onsubmit/onchange/onclick attributes rather than Svelte event directives. Prefer on:submit|preventDefault, on:change, and on:click for consistency with Svelte patterns and clearer event modifier support.

Copilot uses AI. Check for mistakes.
<div class="actions">
<button type="submit">{submitLabel}</button>
{#if onCancel}
<button type="button" class="secondary" onclick={() => onCancel?.()}>
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These handlers are wired via onsubmit/onchange/onclick attributes rather than Svelte event directives. Prefer on:submit|preventDefault, on:change, and on:click for consistency with Svelte patterns and clearer event modifier support.

Copilot uses AI. Check for mistakes.
Comment on lines +160 to 167
const fieldColumns = fields.map((field) => ({
field,
column: toColumnName(field),
}));
const selectFields = fieldColumns.map(({ column }) => column).join(', ');

// Build WHERE clause
const whereClauses: string[] = [];
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selectFields (and later ORDER BY columns) are interpolated directly into SQL from fields/filters/orderBy. With the new toColumnName helper this is still not sanitized/quoted, so a malicious or malformed field name can break queries or be exploited (especially if config is user-supplied). Validate identifiers against a strict allowlist regex and/or quote identifiers safely; consider mapping only from known schema fields rather than accepting arbitrary strings.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5854753d98

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/content/src/content.ts Outdated
Comment on lines +444 to +446
const configuredGovernance = this.getConfiguredGovernance();
if (!configuredGovernance.isGoverned) {
return;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Validate published content against effective governance rules

When governance is added through the new persisted assignment tables instead of configureContentGovernance(), getConfiguredGovernance() still reports isGoverned=false, so this early return skips publish-readiness enforcement entirely. In that setup a tenant can mark a type governed in the DB and still publish without any required reviews, because resolveEffectiveContentGovernance() is never reached.

Useful? React with 👍 / 👎.

Comment on lines +301 to +303
if (existing) {
return existing.appendRevisionAction({
title: email.subject || existing.title,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reapply intake screening to email-thread follow-up revisions

For replies on an existing threadKey, the method computes intake a few lines above but then immediately appends the revision here without using that decision. As a result, reject/quarantine rules are enforced on the first inbound email only; later replies in the same thread can attach blocked files or disallowed text and still land on the contribution.

Useful? React with 👍 / 👎.

Comment on lines +152 to +155
matches = await this.semanticSearch(query, {
limit,
minSimilarity,
where: includeSuperseded ? undefined : { status: 'active' },
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve tenant filtering in semantic fact browse queries

When browseCatalog() receives a non-empty query and embeddings are available, this semanticSearch() call only filters by status. The tenant-scoped filtering built earlier is bypassed on that success path, so tenant-specific fact browsing can return facts from other tenants; this only avoids leaking data when semantic search throws and the fallback path runs instead.

Useful? React with 👍 / 👎.

@willgriffin
Copy link
Copy Markdown
Contributor Author

I've conducted an in-depth review of PR #1037, focusing on the architectural, security, and implementation aspects. Overall, the implementation correctly adheres to the SMRT framework conventions, incorporating robust handling of DuckDB idiosyncrasies.

Architectural Review

  • SMRT Conventions: The use of @smrt() and @TenantScoped({ mode: 'optional' }) in models like ContentContribution and ContentGovernancePolicy is correctly applied. The generated REST / CLI / MCP methods are properly encapsulated and correctly specify conflict columns.
  • Cross-package Associations: The promotion logic cleanly segregates and bridges boundaries across packages (e.g., orchestrating Assets, Content, and Contents without tight-coupling DB internals).
  • System Table Prefixing: I noted that content_governance_policies and similar configuration tables do not use the _smrt_ prefix. While typical system tables require this prefix, these are primarily domain-specific content configuration objects, so it's acceptable.
  • Schema Management Enhancements: The transition and refactoring in packages/core/src/schema/schema-manager.ts—specifically replacing SDK syncSchema() with explicit DDL execution and accommodating DuckDB-specific unique index constraints (with the exportTable fallback)—is heavily robust and handles racing conditions perfectly.

Security Review

  • API Boundary & RBAC: The custom actions (approveAction, promoteAction, etc.) in ContentContribution are exposed via auto-generated routes/api/v1/contentcontributions/+server.ts endpoints. While the logic within these actions checks internal invariants (this.status === 'promoted'), there aren't explicit @permission decorators attached to these methods to enforce role-based access. I recommend verifying that SvelteKit global hooks or package-level interceptors are applying proper RBAC blocks to these administrative endpoints, to prevent unauthorized editors from initiating an approval or promotion step.
  • Tenant Isolation: Heavy usages of @tenantId({ nullable: true }) and findByTenant ensure that tenant segregation is strictly observed.
  • Input Validation: safeParseJSONObject properly brackets potentially corrupted DB config strings into type-safe objects, mitigating potential parsing panics on startup. DDL operations explicitly escape properties utilizing quoteIdentifier, successfully mitigating SQL Injection vectors.

Implementation Review

  • SQLite Constraint Fallbacks: addMissingColumns in schema-manager.ts addresses the SQLite limitation with ALTER TABLE ADD COLUMN for NOT NULL constraints beautifully by relaxing NOT NULL if there is no default value.
  • Content Formatting & Transparency: Standard mappings in content-transparency.ts provide clean schemas for the AI frontend pipeline to visualize provenance reliably. The deduplication of empty/stale facts during normalization prevents UI pollution.

Fantastic additions—great work!

@willgriffin
Copy link
Copy Markdown
Contributor Author

Addressing the review pass on #1037:

Fixed in code:

  • hardened serializer/object coercion to avoid spreading null/non-object toJSON() values
  • gated image seeding behind dev / SMRT_CONTENT_SEED_IMAGES
  • bootstrapped base STI tables for image update/delete and restricted image PUTs to mutable fields only
  • normalized contribution attachment/revision ordering to snake_case column names
  • expanded content-package startup schema bootstrap to cover all registered @happyvertical/smrt-content classes
  • stopped contribution form resets from wiping user-entered state when types changes asynchronously
  • validated identifiers in CLI export SQL generation
  • enforced publish readiness for persisted governance assignments, not only static config
  • reapplied intake screening to email-thread follow-up revisions
  • preserved tenant scoping for semantic fact browsing
  • added regression tests for the new failure modes above

Deferred intentionally:

Verification run after the fixes:

  • packages/content: pnpm exec tsc --noEmit, pnpm exec svelte-check --tsconfig ./tsconfig.json, full pnpm exec vitest run, pnpm exec vite build
  • packages/cli: pnpm exec vitest run src/commands/__tests__/export.test.ts
  • packages/facts: pnpm exec vitest run src/__tests__/fact-collection.test.ts

@willgriffin willgriffin force-pushed the codex/pr-1033-facts-review branch from 0ed3a9b to bcf964b Compare March 20, 2026 20:36
@willgriffin willgriffin merged commit 7030c51 into main Mar 20, 2026
11 checks passed
@willgriffin willgriffin deleted the codex/pr-1033-facts-review branch March 20, 2026 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants