[perlaimad]

Summary

• Adds mutable off-chain World Seed support and initial World Bible creation.
• Adds Worlds API routes for create, browse/list, retrieve, hub aggregate, and founder editing.
• Creates founder membership and audit log in the world creation transaction.
• Registers Worlds routes in the API app.
• Updates development seed and README route docs.
• Hardens World Hub visibility so public hub data only exposes canon lore, not drafts.
• Validates World Bible content as JSON before it reaches Prisma.

Verification

• npm.cmd run db:validate
• npm.cmd test --workspace=@hivelore/api
• npm.cmd run lint
• npm.cmd run typecheck
• npm.cmd run build

[github-actions]

Vercel preview is ready:

https://hivelore-ng8jbcozb-kareemn1936s-projects.vercel.app

Commit: c7ef532

[mcfarhat]

@perlaimad reviewed properly now — sorry this sat without feedback. The authorization model is the strongest part: I tried to break it and couldn't. Identity is session-derived everywhere (your test asserting a client-supplied founderId/role is ignored is a nice touch), all six contribution routes chain requireSession → per-world permission → ownership scoping on authorId and worldId, so there's no IDOR. Your claim that the hub only exposes canon holds up — I verified the PUBLISHED_CANON filtering. contributions.test.ts is genuinely strong (rollback, concurrency, cross-user denial). Migrations are additive with sensible indexes.

⚠️ First, an ordering issue — read this before you rebase. #15 just merged, and it changes auth-middleware.ts: authenticateRequest is now async and both it and requireSession require a database option. Your authOptions() helper (routes/worlds.ts:107 and routes/contributions.ts:64) doesn't pass database, and you have 9 call sites. There will be no merge conflict — it'll merge clean and then break — so please handle it deliberately.

The dangerous part: routes/worlds.ts:135 calls authenticateRequest without await. If you add database but forget the await, the returned Promise is always truthy, so if (!authenticatedUser) never fires and the 401 guard on world creation silently stops working. Please add the await and re-check all 9 sites.

Then the review items:

1. Published canon can be silently rewritten. updateWorld (lib/worlds.ts:471-487) mutates the highest bible version in place with no guard on publishedAt/hiveReferenceId — so a founder PATCH can diverge the off-chain row from the on-chain record, which contradicts your own schema comment. Reject with 409 when published/anchored, or create version N+1. (Related: versionNumber is hardcoded to 1, so a second version is never created and history is lost.)
2. isJsonValue accepts null (routes/worlds.ts:27-59) but content is non-nullable → {"bible":{"content":null}} passes Zod then throws a Prisma error as an unhandled 500 instead of 400.
3. Unbounded bible content. Contributions cap at 100 KB but the bible has no size or depth limit, and the unauthenticated GET /worlds list embeds the full bible of every world — ~20 MB per anonymous request. Also isJsonValue recurses with no depth cap → stack-overflow 500. Apply the same byte cap and drop content from the list projection.
4. Founder canon edits are unaudited — every contribution mutation writes a WorldAuditLog row, but PATCH /worlds writes none (and the enum has no WORLD_BIBLE_UPDATED). For a canon-governance product that's a real traceability gap.
5. Slug race → 500 — generateUniqueWorldSlug runs outside the transaction, so two concurrent creates with the same title collide on P2002 unhandled. Use the existing prisma-conflicts.ts pattern → 409.
6. Two raw throw new Error in updateWorld → 500; reachable because the seed migration backfills no rows for pre-existing worlds. Make them 4xx.
7. Tests: the worlds fake DB ignores where in findMany/count, so the browse filters are untested (the ?q=moon assertion would pass even if filtering were broken), and the "creates atomically" test doesn't verify atomicity. contributions.test.ts is the bar — bring worlds up to it.

Also please link the Trello cards — this covers two (Worlds and Contribution editor), and note Kareem's #16 is built on this branch, so it lands after you.

[mcfarhat]

@perlaimad I checked the branch just now and I don't think the fixes have landed yet — I suspect they're still local and not pushed, because GitHub shows the branch unchanged.

What I see on m2-worlds-seed-bible-hub right now:

• Latest commit is Add structured contribution draft workflow, 2026-08-05 18:00 — no commits after my review (2026-08-06 22:17).
• The branch is 1 behind develop (not yet rebased onto the merged Fix/auth secret hardening #15).

You're right that CI is green (Quality ✅, Vercel Preview ✅) — but that's expected, and it's the trap here: none of the outstanding items are things CI can catch. There's no test that exercises a published-canon edit, no size limit assertion, no lint rule for a missing await. Green CI on this repo means "it compiled and deployed", not "the review items are done".

I re-verified each item against the current head, with line numbers so you can check yourself:

⚠️ The one that matters most — apps/api/src/routes/worlds.ts

• authOptions() (line 107) still returns only { audience, issuer, jwtSecret } — no database.
• Line 135 is still const authenticatedUser = authenticateRequest(request, authOptions()); — no await.

Since #15 merged, authenticateRequest is async and requires database. Without the await, it returns a Promise, which is always truthy — so if (!authenticatedUser) never fires and the 401 guard on world creation silently stops working. It won't throw, won't fail CI, and won't fail a test. Please fix all 9 call sites (routes/worlds.ts and routes/contributions.ts) and add the await.

The rest, all still unchanged:

1. lib/worlds.ts — updateWorld still has no guard on publishedAt/hiveReferenceId, so a founder PATCH can still silently rewrite published/anchored canon. Also versionNumber is hardcoded to 1 (line 208), so version N+1 is never created.
2. routes/worlds.ts:27-30 — isJsonValue still starts with if (value === null) return true;, so {"bible":{"content":null}} still becomes a 500 instead of a 400.
3. lib/worlds.ts:53-59 — worldInclude still attaches the full latest bible version with no field restriction and no size cap, and the unauthenticated GET /worlds list still returns it for every world.
4. Only 2 WorldAuditLog references in lib/worlds.ts — PATCH /worlds still writes no audit entry.
5. lib/worlds.ts:176 — generateUniqueWorldSlug is still called before $transaction (line 179), so the P2002 race stands.
6. Three raw throw new Error still at lines 172, 436, 475 → 500s instead of 4xx.
7. The worlds tests' fake DB still ignores where in findMany/count, so the browse filters remain untested.

What to do: git push your local work if it exists (if you rebased locally, you may need --force-with-lease), then rebase onto current develop and re-check the 9 auth call sites. Once it's pushed I'll re-review straight away — and please shout if any of the seven items is unclear or you disagree with it, that's fine too.

To be clear, none of this is a criticism of the work: the authorization model in this PR is the strongest part and I found no bypass or IDOR. It's just that what's on GitHub isn't what you're describing.

[mcfarhat]

@perlaimad re-reviewed at 37af8af. Every item from my 08-06 and 08-07 reviews is genuinely fixed in the code — I verified each one rather than taking the commit messages at face value. That's a real turnaround from the last round, and the way you fixed them matters as much as that you did:

• authOptions() / missing await — all 15 call sites now pass database, and both non-preHandler calls are awaited (worlds.ts:243, proposals.ts:110).
• isJsonValue accepting null — rewritten as validateJsonValue, rejects null and undefined, with a test asserting 400 for both.
• Unbounded bible content — 100 KB byte cap and depth cap, and you replaced the recursion with an explicit stack so a deep payload can't blow the stack. content is now dropped from the public list projection.
• Slug race — generateUniqueWorldSlug moved inside the transaction and P2002 mapped to 409, with tests at both ends.
• Raw throw new Error — zero remaining in worlds/contributions/canon-voting.
• The fake DB ignoring where — this is the one I want to call out. matchesWorldWhere is now applied in both findMany and count, the ?q= test asserts the exact result set, and you added fault injection (failNextSeedCreate, failNextAuditCreate) to prove rollback actually happens. That's the standard I was asking for, and it's now the best test suite in the repo.

129/129 tests pass, lint/typecheck/format clean, and your three migrations apply to a clean Postgres 16 with no drift.

Two are half-done:

• Published-canon versioning — the 409 guard is real and tested, but the "or create version N+1" half is untouched: versionNumber: 1 is still hardcoded (worlds.ts:274) and updateWorld still mutates in place. Net effect: a world gets exactly one bible version, and once published the bible can never be amended by any path. (It also makes staleBaseAtDecision in canon-voting.ts:444 permanently unreachable.)
• Audit coverage — WORLD_BIBLE_UPDATED is written for bible edits, but a PATCH changing title, description or seed (worlds.ts:495-544) still writes no audit row.

---

Blockers in the new canon-voting work

B1 — Nobody can finalize a decision. The feature cannot complete end-to-end.

POST .../finalize requires EXECUTE_CANON_STATUS_AFTER_THRESHOLD (routes/proposals.ts:270). That permission is held only by CURATOR (world-permissions.ts). The only place a WorldMembership is ever created is lib/worlds.ts:279-286, which always assigns FOUNDER — and FOUNDER's permission list does not include it. There is no route anywhere in apps/api/src that grants or changes a role.

So the world's own founder gets a 403 finalizing their own world's proposal, and no CURATOR can ever exist. The vote → decide → publish loop is unreachable for every user.

Cheapest fix is adding the permission to FOUNDER; the fuller one is a role-grant route. Either way, please add a test that runs the whole loop as a real founder — that's what would have caught this.

B2 — confirmCanonTransaction crashes on any real Hive block.

canon-voting.ts:628-642 calls searchBlocks with no operationTypes filter, then runs normalizeHafOperation(row) on every row before matching on transaction_id. hive/projection.ts:122 throws a bare Error for anything that isn't a comment or custom_json — and every real block is full of votes, transfers and producer rewards. It escapes as an unclassified error, so the client gets a 500 with the internal message in the body.

Your test passes only because it mocks a block containing exactly one operation. Fix: filter by transaction_id before normalizing, pass operationTypes, and wrap the normalize in a try/catch.

B3 — None of your 129 tests run in CI.

.github/workflows/ci.yml runs format, prisma validate/generate, lint, typecheck, build. There is no test step, and the root package.json has no test script at all. This is the exact trap from my 08-07 comment — green CI here means "it compiled", not "it works" — now with a much larger suite behind it. Repo-level, not yours; I'm carding it.

---

The one to fix before anything touches the chain

aiWarning.acknowledged is false data, and it gets written to Hive permanently.

canon-voting.ts:80 sets acknowledged: Boolean(warning) — that's "an AI warning exists", not "a human acknowledged it". And isMajorAiWarning (:68-72) substring-matches major / contradiction / warning against the stringified findings, so a report saying "no contradiction found" sets acknowledged: true. Meanwhile Proposal.aiWarningAcknowledgedAt exists in the schema and is never written or read anywhere.

That value is hashed into decisionPayloadHash and broadcast on-chain. We run on mainnet, always — there is no testnet in our stack — so a wrong acknowledged flag is a permanent, public, uncorrectable record attached to a canon decision. Please make it mean what it says (write and read aiWarningAcknowledgedAt), and replace the substring matcher with a structured severity field on the AI report.

Should-fix

• The proposal author can vote on their own proposal. No self-exclusion in castCanonVote or tallyCanonVotes. With minimumVotes: 5 and a 70% threshold, an author's own APPROVE is decisive — 4/5 approvals including their own clears the bar and gets written to Hive.
• POST .../canon-transaction/confirm has no authorization beyond having a session (proposals.ts:324-345) — no authorizeWorldPermission. A non-member reaches the business logic. Combined with a client-supplied blockNumber and no rate limit, any authenticated user can drive unbounded outbound HAF fetches from our server.
• Confirm is TOCTOU. The "already confirmed" check (:683-699) runs outside the transaction opened at :762, and the final update has no transactionId: null guard. Two concurrent confirms both pass, the second overwrites the first and orphans a HiveEvent. Use updateMany({ where: { id, transactionId: null } }) and check count.
• Six routes return a promise inside try/catch (proposals.ts:138, 156, 239, 279, 313, 348), so handleCanonVotingError never runs. Status codes are right via Fastify's default handler, but the body shape differs — {"error":"..."} on one path, {"statusCode":..,"code":..,"error":"Conflict","message":..} on the other. Two incompatible envelopes on one API, and it's how B2's raw error leaks.
• No rate limiting on any new route, despite the commit titled "canon voting system with rate limiting". app.ts:39 sets global: false and only auth.ts opts in. One READER can flood ProposalComment rows.
• Unbounded vote loading — findProposal and getVoteSummary select every AppVote row plus every AIReport with full findings, then tally in JS, on every anonymous page view. Use appVote.groupBy(['choice']).
• Public proposal detail has no status filter (canon-voting.ts:188-192), so a DRAFT proposal's full proposedContent is anonymously readable. Not reachable through today's routes, but status defaults to DRAFT and nothing enforces the invariant.
• The confirm UI can't be used as built — canon-vote-screen.tsx:747-769 asks the user to hand-type blockNumber and operationIndex, but Keychain only returns a transaction id. Resolve by transactionId alone.

Nits

mapDuplicateVoteConflict is dead (you use upsert, which never raises P2002). listProposalComments excludes soft-deleted rows in count but not in the list where, so totalCount disagrees with the rows returned — and the test bakes that in (hasMore: true, totalCount: 1 with 2 rows). Vote changes write no audit row, unlike every other mutation here. commentBodySchema allows MAX+1 then rejects in the lib. Migration 20260810120000 uses ALTER TYPE ... ADD VALUE without IF NOT EXISTS while its sibling uses it.

One process note

The merge commit 3527672 went up with apps/api/src/routes/auth.ts in a broken state — a botched conflict resolution left a syntax error and a duplicate /auth/refresh registration, so the file didn't parse. CI Quality failed on f582a86 and 3527672 and stayed red for about half an hour until f3263b8 cleaned it up. Nothing was lost and the final file is fine. Just push the fix before the broken commit next time rather than after — a red CI on the default path costs everyone else time.

Definition of done

Once B1 and B2 are fixed, this needs one real mainnet round-trip — broadcast a canon decision, read it back through confirmCanonTransaction, and post the transaction id here. We don't use the Hive testnet on any project. See the card for the account arrangements; don't create one yourself.

Also: please rebase #22 onto this branch. Right now #22 targets develop while containing all of #14, so it shows 41 files when only 14 are actually new.

Genuinely good work on the fixes — the rollback tests in particular. The new feature just needs to be exercised against something resembling real data before it can land.

[mcfarhat]

Closing this as superseded by #22 — no work is lost, and this is a correction of my tracking rather than a judgement on the code.

The check: git merge-base --is-ancestor pr14 pr22 → true, and git rev-list --count pr14 ^pr22 → 0. #22 is exactly this branch plus 8 commits. Every file here is carried there, byte-identical for lib/worlds.ts, routes/worlds.ts, lib/contributions.ts, routes/contributions.ts and seed.ts. Branch not deleted — #22 is built on it.

I owe you an apology for how this PR was tracked. For several days I described #14 as "the bottleneck blocking every other PR", and reported it as stalled because it hadn't been pushed to since 08-10. That was wrong twice over:

1. The dependency didn't exist. Kareem rebased his work onto develop on 08-07; from that point nothing of his was stacked on this branch. I never re-checked, and repeated the claim for three more days — including on the Trello card, where it's still uncorrected. I'll fix that.
2. You weren't stalled, you were working. 6ad0cc0 (yesterday 08:17) carries this PR's outstanding review work — the EXECUTE_CANON_STATUS_AFTER_THRESHOLD permission, the aiWarningAcknowledgedAt rewrite, the transaction-id filter — plus both repo-health cards. It just landed on hive-broadcast-reliability rather than here. I was reading last-push dates per PR instead of following the branch, so a full day of your output across five cards looked like silence. That's a flaw in how I was tracking, not in how you were working.

Two consequences worth stating plainly:

• Cards feat(api): add canon consistency and search #28 (canon voting) and feat(api): add observability readiness harness #30 (comments) were pointed at this PR, so when you wrote "blockers are addressed" and "fixes applied in PR 22", my tracking still said Implement world seed, initial bible, browse worlds, and world hub #14. I'm re-pointing both to Hive broadcast reliability #22 now. Anyone re-reviewing this branch would have found those blockers still live and told you they weren't fixed — which would have been my error presented as your problem.
• You did cards #36 and #37 unasked — the duplicate-baseline repair and making CI actually run tests. Both were marked "unassigned, repo-level". I verified the migration repair end to end: on a database that used the documented migrate resolve --applied workaround, your migrate deploy applies all 12 migrations cleanly with no drift. That fixes a thing that has been quietly costing every person on this project time, and nobody asked you to do it.

Everything continues on #22. I have a full review of it ready — including some things that need fixing before it can merge, and one governance question for Mohammad — and I'll post it there shortly so it's all in one place rather than split across two PRs.