Skip to content

feat(agent): add email-authenticated map moderation - #14

Open
Oba-One wants to merge 2 commits into
mainfrom
codex/map-moderation-magic-link
Open

feat(agent): add email-authenticated map moderation#14
Oba-One wants to merge 2 commits into
mainfrom
codex/map-moderation-magic-link

Conversation

@Oba-One

@Oba-One Oba-One commented Jul 14, 2026

Copy link
Copy Markdown
Member

Summary

  • add recipient-specific seven-day HMAC moderation links, per-recipient Resend delivery/idempotency, generic invalid-link responses, strict origin/JSON handling, and first-decision-wins transactions
  • add the static /map/moderate review surface with fragment-only credentials, CSP/no-referrer/noindex, safe-field projection, native confirmation dialogs, and approve/decline states
  • add migration 020, hidden operator-only Directus metadata, safe disabled-by-default Fly configuration, rollout/rollback documentation, and database/browser-contract coverage

The feature flag remains off by default, so existing Directus-link alerts remain the production behavior until the documented post-merge rollout.

Linear: follow-up to PRD-622. A dedicated follow-up issue still needs to be created because this session has no Linear connector and authenticated Brave control is unavailable.

Validation

  • node_modules/@typescript/native/bin/tsc -b --pretty false
  • 141 focused agent, moderation, map-node, website-route, Directus Studio, and public-content tests pass
  • migration 020 applies to local Postgres and replays as a no-op
  • real local database proof passes approve, decline, approved_at, review audit, sibling resolution, concurrent first-decision-wins, Directus-first resolution, cleanup, and fixture removal
  • Modern Web Guidance accessibility retrieval, browser-policy check, plan validation, and ui:check pass
  • Astro production build passes and emits /map/moderate
  • all three CodeQL review findings are fixed and their threads are resolved/outdated
  • CodeRabbit review passes
  • git diff --check
  • authenticated Brave QA at 375/1024/1440 — blocked because the browser-control runtime exits while reading the macOS OpenSSL configuration; no isolated-browser substitute was used

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added optional seven-day email magic links for approving or declining pending map submissions without requiring Directus login.
    • Added a secure moderation page with approval, rejection, private notes, and clear status handling.
    • Added configuration options for enabling links, setting the moderation URL, and securing access tokens.
    • Retained Directus links and existing moderation workflows as fallbacks.
  • Documentation

    • Added setup, verification, rollback, and operational guidance for moderation links.
  • Style

    • Updated font loading for improved standalone page behavior.

Walkthrough

Adds optional seven-day, recipient-specific moderation magic links for map-node alerts. Adds protected session and decision APIs, access-link persistence and cleanup, a private moderation page, Directus integration, deployment configuration, documentation, and tests.

Changes

Map-node moderation

Layer / File(s) Summary
Moderation contracts and access-link storage
packages/shared/src/map-nodes.ts, packages/agent/migrations/..., scripts/directus-..., scripts/public-content-contract.test.ts
Defines redacted moderation session types and stores recipient-specific access links with delivery, expiry, resolution, and cleanup fields.
Magic-link creation and delivery
packages/agent/src/map-node-moderation.ts, .env.example, packages/agent/fly.toml, packages/admin/README.md, README.md, scripts/map-node-moderation.test.ts
Adds HMAC tokens, seven-day expiry, recipient delivery tracking, retries, cleanup, email delivery, configuration, and Directus fallback behavior.
Authenticated moderation API
packages/agent/src/app.ts, packages/agent/src/map-nodes.ts, packages/agent/src/index.ts, scripts/agent-contract.test.ts, scripts/map-node-moderation.test.ts, scripts/map-node-moderation.integration.ts
Adds protected session and decision routes. Decisions validate input, lock submissions, record reviews, resolve sibling links, and consume used links.
Private moderation review page
packages/website/src/pages/map/moderate.astro, packages/website/src/layouts/GpLayout.astro, packages/website/src/styles/gp-tokens.css, packages/website/DESIGN.md, scripts/map-moderation-route.test.ts
Adds token handling, redacted node rendering, approve and decline flows, resolved and error states, security metadata, accessibility behavior, responsive styling, and browser tests.

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

Poem

I’m a rabbit with a link in my ear,
Seven days of reviews appear.
Approve or decline with a hop and a note,
Safe little tokens keep secrets afloat.
Directus waits when links sleep tight—
Carrots for tests, and deployment tonight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: email-authenticated map moderation.
Description check ✅ Passed The description directly explains the moderation links, review interface, configuration, documentation, and validation coverage.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
⚔️ Resolve merge conflicts 💡
  • Resolve merge conflict in branch codex/map-moderation-magic-link
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/map-moderation-magic-link

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Oba-One Oba-One added the automated/codex Created by Codex automation label Jul 14, 2026 — with ChatGPT Codex Connector
Comment thread scripts/map-moderation-route.test.ts Fixed
Comment thread scripts/map-moderation-route.test.ts Fixed
Comment thread scripts/map-moderation-route.test.ts Fixed
@Oba-One
Oba-One marked this pull request as ready for review August 1, 2026 20:06

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 6

🧹 Nitpick comments (11)
scripts/map-moderation-route.test.ts (3)

167-169: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Report a clear cause when the built route is missing.

readFile throws a raw ENOENT if packages/website/dist/map/moderate/index.html does not exist. The test then fails with no indication that a website build is required. Wrap the read and state the required command.

♻️ Proposed adjustment
 test.before(async () => {
-  routeHtml = await readFile(routeHtmlPath, 'utf8');
+  try {
+    routeHtml = await readFile(routeHtmlPath, 'utf8');
+  } catch {
+    throw new Error(
+      `Missing ${routeHtmlPath}. Build the website package before running this test.`
+    );
+  }
 });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/map-moderation-route.test.ts` around lines 167 - 169, Update the
test.before setup around routeHtmlPath/readFile to catch a missing built route
and report clearly that the website build must be run, including the required
build command, while preserving successful file loading.

41-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert that exactly one inline script exists.

extractInlineController takes the first <script> occurrence. If the build later emits another inline script before the controller, the harness silently executes the wrong code and the behavior tests pass against nothing. Count the <script tags with the existing openingTags helper and assert the count is one.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/map-moderation-route.test.ts` around lines 41 - 48, Update
extractInlineController to count script tags using the existing openingTags
helper and assert exactly one inline script is present before extracting it.
Preserve the current controller extraction and missing-tag assertions after
validating the count.

230-276: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add coverage for the already-resolved reload path.

The tests cover approve, decline with a note, and the invalid link. The controller has a distinct branch at moderate.astro Lines 218-223: a 409 with moderation_already_resolved closes both dialogs and reloads the session. That branch protects the first-decision-wins behavior in the browser. Add one case for it.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/map-moderation-route.test.ts` around lines 230 - 276, Add a test
covering the moderation_already_resolved 409 response when submitting a
decision, verifying both moderation dialogs close and the session is reloaded to
display the resolved state. Anchor the scenario in the existing createHarness
and fetchImpl patterns used by the approve and decline tests, and assert the
follow-up session request and resulting UI state.
packages/website/src/pages/map/moderate.astro (1)

293-297: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use the spacing tokens for padding and margins.

These rules hardcode pixel spacing (32px, 16px, 12px, 28px, 20px, 14px). The website token system supplies --gp-space-* for spacing. Replace the literals so this page tracks the shared scale.

As per coding guidelines: "Use 8px spacing via --gp-space-* tokens; use xs (4px) only for micro-adjustments, never for section spacing" and "Use var(--gp-*) CSS custom properties for all colors, sizes, and spacing".

Also applies to: 341-341, 365-368, 397-399

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/website/src/pages/map/moderate.astro` around lines 293 - 297,
Replace the hardcoded spacing values in the moderate page styles, including the
rules around main and the referenced sections, with the corresponding
--gp-space-* tokens. Cover all listed padding, margin, gap, and inset values
while preserving the existing layout and responsive behavior; use the shared 8px
spacing scale rather than introducing new literals.

Source: Coding guidelines

packages/agent/src/app.ts (2)

477-514: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the shared moderation request guard.

Both moderation handlers repeat the same origin check, JSON check, and bounded-body check with identical response bodies. Extract one guard that returns either the parsed object or the error response. This keeps the two routes in sync when the checks change.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/agent/src/app.ts` around lines 477 - 514, Extract the repeated
moderation request validation from the moderation route handlers into a shared
guard near the existing moderation helpers. Have it perform the origin check,
JSON check, and bounded-body parsing, returning either the parsed request object
or the corresponding error response; update both moderation handlers to use this
guard while preserving their current status codes and response bodies.

93-117: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Handle request-body failures before parsing moderation input.

The routes catch exceptions after readModerationJsonObject(context), but context.req.text() is awaited before JSON.parse(rawBody), so a stream abort or undecodable body can escape the handler and return 500 instead of invalid_moderation_request. Wrap the body read in try/catch and return null on failure.

Type the helper parameters with Context instead of relying on the permissive noImplicitAny setting.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/agent/src/app.ts` around lines 93 - 117, Update
readModerationJsonObject to catch failures from awaiting context.req.text() and
return null so aborted or undecodable request bodies produce
invalid_moderation_request handling. Also type the context parameters of
preparePrivateModerationResponse, isJsonRequest, and readModerationJsonObject as
Context.
packages/agent/migrations/020_map_node_moderation_access_links.sql (1)

73-88: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value

Access-link retention is undocumented for operators. intake.cleanup_map_node_moderation_access_links deletes rows 7 days after token expiry, while the operator documentation presents the collection as the audit correlation between the opaque moderation-link:<id> actor and the email recipient. The two statements need one agreed window.

  • packages/agent/migrations/020_map_node_moderation_access_links.sql#L73-L88: keep the 7-day grace deletion, or retain resolved rows longer if audits need the recipient correlation.
  • packages/admin/README.md#L263-L270: state the retention window for map_node_moderation_access_links so operators know the correlation expires.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/agent/migrations/020_map_node_moderation_access_links.sql` around
lines 73 - 88, Document the retention policy consistently across both sites:
preserve the 7-day grace deletion in
intake.cleanup_map_node_moderation_access_links, and update
packages/admin/README.md lines 263-270 to state that
map_node_moderation_access_links rows are removed 7 days after token expiry, so
operators understand when recipient correlation expires.
README.md (1)

181-181: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Secret-length wording does not match the implementation. Both documents require 32 bytes, but magicLinkConfigured checks config.linkSecret.length >= 32 on the string value, which counts characters.

  • README.md#L181: change "32-byte-or-longer" to at least 32 characters.
  • packages/admin/README.md#L272-L276: change "containing at least 32 random bytes" to at least 32 characters, and suggest 32 random bytes encoded as base64url.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@README.md` at line 181, The documentation’s secret-length requirements must
match the implementation’s character-based check. In README.md at lines 181-181,
change “32-byte-or-longer” to “at least 32 characters”; in
packages/admin/README.md at lines 272-276, replace the 32 random bytes
requirement with at least 32 characters and suggest using 32 random bytes
encoded as base64url.
scripts/map-node-moderation.test.ts (1)

76-91: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Make the privacy assertion non-vacuous.

Line 185 asserts that the email text contains no owner email, raw note marker, or IP address. notificationRow carries none of those fields, so the assertion passes even if the email builder started to include private input. Add private fields to the fixture that the delivery query would never select, then keep the assertion.

💚 Proposed fixture change
 const notificationRow = Object.freeze({
   id: 'c8e1fc85-5ca5-43f9-bd13-5069eb6c544f',
   kind: 'submission',
   attempts: 0,
   submissionId: '35cd495c-8043-4aa8-9cc0-5e5469b4fb70',
   displayName: 'Magic Link Member',
+  ownerEmail: 'private@example.org',
+  rawNote: 'Never email this raw note.',
+  ipAddress: '203.0.113.42',
   placeName: 'Oakland',

Also applies to: 185-185

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/map-node-moderation.test.ts` around lines 76 - 91, Add private input
fields to the notificationRow fixture used by the email-building test, using
values for an owner email, raw note marker, and IP address that the delivery
query does not select. Keep the existing privacy assertion at the
email-generation test unchanged so it verifies those fixture values are absent
from the output.
scripts/public-content-contract.test.ts (1)

607-616: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win

Assert all actions, not only read.

The test proves that neither role receives a read permission on intake.map_node_moderation_access_links. A create, update, or delete grant would also be a privacy or integrity problem. Assert that no permission of any action exists for these roles on that collection.

♻️ Proposed assertion
+  const accessLinkPermissions = plan.permissions.filter((permission) => (
+    permission.collection === 'intake.map_node_moderation_access_links' &&
+    ['Greenpill Steward Moderator', 'Greenpill Trusted Publisher'].includes(permission.role)
+  ));
   assert.equal(moderatorAccessLinkRead, undefined);
   assert.equal(trustedAccessLinkRead, undefined);
+  assert.deepEqual(accessLinkPermissions, []);

Also applies to: 649-650

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/public-content-contract.test.ts` around lines 607 - 616, Update the
permission assertions for Greenpill Steward Moderator and Greenpill Trusted
Publisher on intake.map_node_moderation_access_links to reject any matching
permission regardless of action, rather than checking only action === 'read'.
Apply the same all-actions assertion to the additional occurrence near the
referenced trusted-role checks, while preserving the existing role and
collection filters.
packages/agent/src/map-node-moderation.ts (1)

930-994: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Build the access-link select once.

The forUpdate and non-locking branches repeat the same 21-column projection. A future column change must be applied twice. Extract the shared projection with a sql fragment and append the lock clause.

♻️ Proposed extraction
-  const rows = forUpdate
-    ? await sql`
-        select
-          access.id::text,
-          ...
-        for update of access
-      `
-    : await sql`
-        select
-          access.id::text,
-          ...
-      `;
+  const projection = sql`
+    select
+      access.id::text,
+      access.notification_id::text as "notificationId",
+      access.submission_id::text as "submissionId",
+      access.recipient_email::text as "recipientEmail",
+      access.token_expires_at as "tokenExpiresAt",
+      access.delivery_status as "deliveryStatus",
+      access.consumed_at as "consumedAt",
+      access.resolved_at as "resolvedAt",
+      access.decision::text,
+      submission.status::text as "submissionStatus",
+      submission.updated_at as "submissionUpdatedAt",
+      submission.display_name as "displayName",
+      submission.place_name as "placeName",
+      submission.city,
+      submission.region,
+      submission.country,
+      submission.latitude::float8 as lat,
+      submission.longitude::float8 as long,
+      submission.themes,
+      submission.public_note as "publicNote",
+      submission.created_at as "createdAt"
+    from intake.map_node_moderation_access_links access
+    join intake.map_node_submissions submission on submission.id = access.submission_id
+    where access.id = ${accessLinkId}::uuid
+    limit 1
+  `;
+  const rows = forUpdate
+    ? await sql`${projection} for update of access`
+    : await projection;

Note that the existing tests match statement text such as for update of access, so verify the composed statement still matches.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/agent/src/map-node-moderation.ts` around lines 930 - 994, Refactor
selectModerationAccessLink to define the shared 21-column projection and query
body once using a reusable sql fragment, then append the conditional “for update
of access” clause only when forUpdate is true. Preserve the current joins,
filtering, limit, returned row shape, and exact lock-clause statement text so
existing tests continue to match.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/agent/src/app.ts`:
- Around line 439-475: Update the moderation session handler to pass the result
of mapNodeRepository.getModerationSession through
assertAuthenticatedMapNodeModerationSession before context.json returns it,
preserving the existing error handling and response flow.

In `@packages/agent/src/map-node-moderation.ts`:
- Around line 132-148: Update canSendModerationEmail and the delivery flow in
deliverQueuedMapNodeModerationNotifications so MAP_NODE_MODERATION_DIRECTUS_URL
is required only when the fallback moderation path needs it, while magic-link
delivery remains possible without queueUrl. Preserve the existing validation for
API key, sender, recipients, and ensure fallback delivery still rejects a
missing queueUrl.

In `@packages/website/src/pages/map/moderate.astro`:
- Line 359: Replace the hardcoded radii in the styles around the theme chip,
button, textarea, and container-query panel with the appropriate design tokens:
use --gp-radius-pill for the chip, button, and textarea, and a suitable
--gp-radius-* token for the panel.
- Around line 377-380: Remove the .button:focus-visible and
textarea:focus-visible rule from the moderate page styles, including its gold
outline and offset, so the global lime focus ring from gp-tokens.css remains in
effect.
- Around line 252-258: Update the decline dialog handlers around declineButton
and declineCancel so reopening or closing the dialog clears declineNote.value
and resets declineCount to its initial value, preventing discarded text and
counter state from persisting.

In `@scripts/map-moderation-route.test.ts`:
- Line 7: Update the rootDir initialization to convert the parent file URL with
fileURLToPath before passing it to resolve, preserving correct filesystem paths
for spaces and Windows drive letters. Import and use fileURLToPath in the test
setup while keeping the existing repository-root target unchanged.

---

Nitpick comments:
In `@packages/agent/migrations/020_map_node_moderation_access_links.sql`:
- Around line 73-88: Document the retention policy consistently across both
sites: preserve the 7-day grace deletion in
intake.cleanup_map_node_moderation_access_links, and update
packages/admin/README.md lines 263-270 to state that
map_node_moderation_access_links rows are removed 7 days after token expiry, so
operators understand when recipient correlation expires.

In `@packages/agent/src/app.ts`:
- Around line 477-514: Extract the repeated moderation request validation from
the moderation route handlers into a shared guard near the existing moderation
helpers. Have it perform the origin check, JSON check, and bounded-body parsing,
returning either the parsed request object or the corresponding error response;
update both moderation handlers to use this guard while preserving their current
status codes and response bodies.
- Around line 93-117: Update readModerationJsonObject to catch failures from
awaiting context.req.text() and return null so aborted or undecodable request
bodies produce invalid_moderation_request handling. Also type the context
parameters of preparePrivateModerationResponse, isJsonRequest, and
readModerationJsonObject as Context.

In `@packages/agent/src/map-node-moderation.ts`:
- Around line 930-994: Refactor selectModerationAccessLink to define the shared
21-column projection and query body once using a reusable sql fragment, then
append the conditional “for update of access” clause only when forUpdate is
true. Preserve the current joins, filtering, limit, returned row shape, and
exact lock-clause statement text so existing tests continue to match.

In `@packages/website/src/pages/map/moderate.astro`:
- Around line 293-297: Replace the hardcoded spacing values in the moderate page
styles, including the rules around main and the referenced sections, with the
corresponding --gp-space-* tokens. Cover all listed padding, margin, gap, and
inset values while preserving the existing layout and responsive behavior; use
the shared 8px spacing scale rather than introducing new literals.

In `@README.md`:
- Line 181: The documentation’s secret-length requirements must match the
implementation’s character-based check. In README.md at lines 181-181, change
“32-byte-or-longer” to “at least 32 characters”; in packages/admin/README.md at
lines 272-276, replace the 32 random bytes requirement with at least 32
characters and suggest using 32 random bytes encoded as base64url.

In `@scripts/map-moderation-route.test.ts`:
- Around line 167-169: Update the test.before setup around
routeHtmlPath/readFile to catch a missing built route and report clearly that
the website build must be run, including the required build command, while
preserving successful file loading.
- Around line 41-48: Update extractInlineController to count script tags using
the existing openingTags helper and assert exactly one inline script is present
before extracting it. Preserve the current controller extraction and missing-tag
assertions after validating the count.
- Around line 230-276: Add a test covering the moderation_already_resolved 409
response when submitting a decision, verifying both moderation dialogs close and
the session is reloaded to display the resolved state. Anchor the scenario in
the existing createHarness and fetchImpl patterns used by the approve and
decline tests, and assert the follow-up session request and resulting UI state.

In `@scripts/map-node-moderation.test.ts`:
- Around line 76-91: Add private input fields to the notificationRow fixture
used by the email-building test, using values for an owner email, raw note
marker, and IP address that the delivery query does not select. Keep the
existing privacy assertion at the email-generation test unchanged so it verifies
those fixture values are absent from the output.

In `@scripts/public-content-contract.test.ts`:
- Around line 607-616: Update the permission assertions for Greenpill Steward
Moderator and Greenpill Trusted Publisher on
intake.map_node_moderation_access_links to reject any matching permission
regardless of action, rather than checking only action === 'read'. Apply the
same all-actions assertion to the additional occurrence near the referenced
trusted-role checks, while preserving the existing role and collection filters.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 541d601c-a759-4fc5-be16-5f9d0d0c647d

📥 Commits

Reviewing files that changed from the base of the PR and between 9a07564 and 72ef2fe.

📒 Files selected for processing (22)
  • .env.example
  • README.md
  • packages/admin/README.md
  • packages/agent/fly.toml
  • packages/agent/migrations/020_map_node_moderation_access_links.sql
  • packages/agent/src/app.ts
  • packages/agent/src/index.ts
  • packages/agent/src/map-node-moderation.ts
  • packages/agent/src/map-nodes.ts
  • packages/shared/src/map-nodes.ts
  • packages/website/DESIGN.md
  • packages/website/src/layouts/GpLayout.astro
  • packages/website/src/pages/map/moderate.astro
  • packages/website/src/styles/gp-tokens.css
  • scripts/agent-contract.test.ts
  • scripts/directus-operational-content-setup.ts
  • scripts/directus-studio-setup.test.ts
  • scripts/directus-studio-setup.ts
  • scripts/map-moderation-route.test.ts
  • scripts/map-node-moderation.integration.ts
  • scripts/map-node-moderation.test.ts
  • scripts/public-content-contract.test.ts
💤 Files with no reviewable changes (1)
  • packages/website/src/styles/gp-tokens.css
📜 Review details
🧰 Additional context used
📓 Path-based instructions (15)
packages/agent/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

packages/agent/**/*.{ts,tsx}: Every new public agent route must have an exported route constant, a shared public payload contract in packages/shared, a public-safe normalizer or assertion, and a focused contract test.
Do not add ad hoc website fetch shapes, public database access, or route-local privacy filters.

packages/agent/**/*.{ts,tsx}: Every new public agent route must export a route constant, use a shared public payload contract from packages/shared, apply a public-safe normalizer or assertion, and include a focused contract test.
Do not add ad hoc website fetch shapes, public database access, or route-local privacy filters.

Files:

  • packages/agent/src/index.ts
  • packages/agent/src/app.ts
  • packages/agent/src/map-nodes.ts
  • packages/agent/src/map-node-moderation.ts
packages/{website,agent,admin}/**/*

📄 CodeRabbit inference engine (CLAUDE.md)

Never expose DATABASE_URL or database credentials to the public website deploy, Keystatic content, generated JSON, browser bundles, or future Vercel projects; credentials belong only on private Fly services.

Files:

  • packages/agent/src/index.ts
  • packages/website/src/layouts/GpLayout.astro
  • packages/agent/fly.toml
  • packages/website/DESIGN.md
  • packages/agent/src/app.ts
  • packages/admin/README.md
  • packages/agent/migrations/020_map_node_moderation_access_links.sql
  • packages/agent/src/map-nodes.ts
  • packages/website/src/pages/map/moderate.astro
  • packages/agent/src/map-node-moderation.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Private map/member-node intake must never expose emails, raw notes, IP addresses, user agents, spam metadata, steward review notes, pending submissions, or raw upstream EAS/Green Goods work and media feedback.

Files:

  • packages/agent/src/index.ts
  • scripts/directus-operational-content-setup.ts
  • scripts/directus-studio-setup.test.ts
  • scripts/map-moderation-route.test.ts
  • packages/agent/src/app.ts
  • scripts/directus-studio-setup.ts
  • packages/agent/src/map-nodes.ts
  • scripts/public-content-contract.test.ts
  • packages/shared/src/map-nodes.ts
  • scripts/map-node-moderation.integration.ts
  • scripts/agent-contract.test.ts
  • scripts/map-node-moderation.test.ts
  • packages/agent/src/map-node-moderation.ts
**/*.{ts,tsx,astro}

📄 CodeRabbit inference engine (AGENTS.md)

Keep implementation within the smallest applicable package boundary and do not expand beyond issue acceptance criteria.

Files:

  • packages/agent/src/index.ts
  • packages/website/src/layouts/GpLayout.astro
  • scripts/directus-operational-content-setup.ts
  • scripts/directus-studio-setup.test.ts
  • scripts/map-moderation-route.test.ts
  • packages/agent/src/app.ts
  • scripts/directus-studio-setup.ts
  • packages/agent/src/map-nodes.ts
  • scripts/public-content-contract.test.ts
  • packages/shared/src/map-nodes.ts
  • scripts/map-node-moderation.integration.ts
  • scripts/agent-contract.test.ts
  • scripts/map-node-moderation.test.ts
  • packages/website/src/pages/map/moderate.astro
  • packages/agent/src/map-node-moderation.ts
packages/website/**/*.{css,astro}

📄 CodeRabbit inference engine (packages/website/CLAUDE.md)

packages/website/**/*.{css,astro}: Use var(--gp-) CSS custom properties for all colors, sizes, and spacing; never hardcode hex colors except for data-visualization colors
Use var(--gp-font-display) (Spectral) only for display and headlines; var(--gp-font-body) (Manrope) for all UI and body text; var(--gp-font-mono) (JetBrains Mono) for overlines and technical metadata; never use literal font-family
Reserve --gp-primary (lime) for the single primary action per screen; use --gp-secondary (gold) for headlines only; use --gp-fg (off-white) for body text; never make gold interactive
Avoid grey and pure white/black; use --gp-off-white (#FAF7EE) for warmest neutral and --gp-green-950 for darkest; step the green scale (--gp-bg, --gp-surface, --gp-card, --gp-card-elev) for hierarchy before using shadow
Use --gp-radius-pill for buttons, chips, inputs, and avatars; use --gp-radius-{sm,md,lg,xl} for other elements; never mix sharp 4px corners with the pill language
Use clamp() tokens for display and headline font sizes to achieve fluid type; never hardcode font-size or add viewport @media for type scaling
Use @container queries for responsive layout inside #main-content (GpLayout), not viewport @media queries, for page sections, primitives, and in-content components; components that reflow on their own width must set container-type on their root
Maintain a 44×44px minimum touch target size for standalone interactive controls at mobile; chips must bump from 30→44px at mobile; buttons already meet 48px; inline text links and SVG map pins are exempt
Use white-space: nowrap on inline CTAs (text + arrow) as a complete link; use nowrap on chip body text; split overlines/bylines with each ·-separated segment as its own nowrap span; collapse middle breadcrumb crumbs to …
Use dvh/svh/lvh for mobile heights instead of vh to avoid cutting off content behind mobile browser chrome
Use 8px spacing via --gp-space-
tokens; use xs (4px) only for micro-adjustments, never for section spacing
...

Files:

  • packages/website/src/layouts/GpLayout.astro
  • packages/website/src/pages/map/moderate.astro
packages/website/**/*.astro

📄 CodeRabbit inference engine (packages/website/CLAUDE.md)

Use semantic HTML elements (button, a) instead of divs with onclick; add label for on every input; use cursor: pointer on actionables; ensure one main landmark per page; maintain stable layout with no load-time shift

Files:

  • packages/website/src/layouts/GpLayout.astro
  • packages/website/src/pages/map/moderate.astro
packages/website/**/*.{astro,ts,tsx,js,jsx,css}

📄 CodeRabbit inference engine (CLAUDE.md)

packages/website/**/*.{astro,ts,tsx,js,jsx,css}: For frontend, UI, CSS, accessibility, browser-proof, or web-design changes in packages/website, run bun run agentic:guidance first, then follow packages/website/DESIGN.md and the website token system.
Ensure reduced-motion behavior is clear and accessible in website UI implementations.

Files:

  • packages/website/src/layouts/GpLayout.astro
  • packages/website/src/pages/map/moderate.astro
packages/website/**/*.{astro,html}

📄 CodeRabbit inference engine (CLAUDE.md)

Prefer semantic HTML, native controls, platform CSS, and browser primitives; preserve landmarks, headings, accessible names, focus states, touch targets, and empty/error/loading states.

Files:

  • packages/website/src/layouts/GpLayout.astro
  • packages/website/src/pages/map/moderate.astro
packages/website/**/*.{astro,ts,tsx,css}

📄 CodeRabbit inference engine (AGENTS.md)

packages/website/**/*.{astro,ts,tsx,css}: Before frontend, UI, CSS, accessibility, browser-proof, or web-design work in packages/website, consult DESIGN.md, gp-tokens.css, and relevant UI primitives, and run bun run agentic:guidance.
Prefer semantic HTML, native controls, platform CSS, and browser primitives; preserve landmarks, headings, accessible names, focus states, touch targets, loading/error/empty states, and reduced-motion behavior.

Files:

  • packages/website/src/layouts/GpLayout.astro
  • packages/website/src/pages/map/moderate.astro
**/*.{env,env.*,toml,yml,yaml,json}

📄 CodeRabbit inference engine (AGENTS.md)

Keep DATABASE_URL only on private Fly services; never expose database credentials to public website deployments, Keystatic content, generated JSON, browser bundles, or Vercel projects.

Files:

  • packages/agent/fly.toml
packages/admin/**/*

📄 CodeRabbit inference engine (CLAUDE.md)

The admin package owns the self-hosted Directus deployment surface, but Directus must not become the public API.

Configure Directus access to Greenpill-owned content, intake, and impact schemas through role-scoped administration, with Data Studio metadata managed by the dedicated setup commands.

Files:

  • packages/admin/README.md
packages/agent/migrations/**/*

📄 CodeRabbit inference engine (CLAUDE.md)

Keep Greenpill-owned schema migrations in packages/agent/migrations; Directus may manage only its own system tables, roles, permissions, and Data Studio views.

Greenpill-owned schema migrations must remain in packages/agent/migrations; Directus must not own those migrations.

Files:

  • packages/agent/migrations/020_map_node_moderation_access_links.sql
packages/shared/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use shared contracts in packages/shared for public/private projections instead of duplicating privacy filters in the website or agent.

packages/shared/**/*.{ts,tsx}: Define reusable public/private payload normalization and privacy-boundary contracts in packages/shared instead of duplicating privacy filters in website or agent code.
Never expose private map/member intake data, including emails, raw notes, IP addresses, user agents, spam metadata, steward review notes, pending submissions, or raw upstream EAS/Green Goods work and media feedback.

Files:

  • packages/shared/src/map-nodes.ts
packages/website/src/pages/**/*.astro

📄 CodeRabbit inference engine (packages/website/CLAUDE.md)

Pages must wrap content in Container.astro inside GpLayout.astro, which sets body.gp-root and the container context

Keep the public website static and avoid direct public website database connections.

Files:

  • packages/website/src/pages/map/moderate.astro
packages/website/src/{pages,data}/**/*.{astro,json}

📄 CodeRabbit inference engine (CLAUDE.md)

Generated public JSON routes such as /locations.json and /impact-sources.json must derive from the approved operational content snapshot and remain public-safe.

Files:

  • packages/website/src/pages/map/moderate.astro
🪛 dotenv-linter (4.0.0)
.env.example

[warning] 31-31: [UnorderedKey] The MAP_NODE_MODERATION_MAGIC_LINK_ENABLED key should go before the MAP_NODE_MODERATION_RECIPIENTS key

(UnorderedKey)


[warning] 32-32: [UnorderedKey] The MAP_NODE_MODERATION_BASE_URL key should go before the MAP_NODE_MODERATION_DIRECTUS_URL key

(UnorderedKey)


[warning] 33-33: [UnorderedKey] The MAP_NODE_MODERATION_LINK_SECRET key should go before the MAP_NODE_MODERATION_MAGIC_LINK_ENABLED key

(UnorderedKey)

🔇 Additional comments (23)
packages/agent/src/map-nodes.ts (1)

32-33: LGTM!

Also applies to: 1729-1734

scripts/agent-contract.test.ts (1)

76-79: LGTM!

Also applies to: 102-103, 132-133

packages/website/src/pages/map/moderate.astro (1)

17-28: 🔒 Security & Privacy | 💤 Low value

Verify the token is removed from history state and from the parsed params.

The controller reads the token from the fragment and then calls replaceState. The fragment is cleared, which is correct. Two residues remain: hashParams keeps the token in a closure for the page lifetime, and window.history.state is preserved unchanged, which is correct only if no earlier navigation stored the fragment. Confirm the intended behavior for a back-navigation to this page after replaceState, because the restored entry then has no token and the page shows the invalid state instead of the review.

packages/website/src/layouts/GpLayout.astro (1)

58-58: LGTM!

packages/website/DESIGN.md (1)

362-362: LGTM!

scripts/map-node-moderation.integration.ts (1)

223-231: 📐 Maintainability & Code Quality

No change needed.

The relevant moderation-dependent rows are covered by on delete cascade: intake.map_node_reviews, intake.map_node_moderation_notifications, and intake.map_node_moderation_access_links via notification_id.

packages/agent/src/index.ts (1)

5-5: 🗄️ Data Integrity & Integration

No change needed.

packages/agent/src/map-nodes.ts imports getMapNodeModerationSession and moderateMapNode, but does not re-export them. No duplicate star export creates an ambiguous namespace entry here.

packages/agent/migrations/020_map_node_moderation_access_links.sql (1)

8-71: LGTM!

packages/agent/src/map-node-moderation.ts (7)

1-11: LGTM!

Also applies to: 34-57, 70-84, 115-123


150-242: LGTM!


294-328: LGTM!

Also applies to: 406-462, 464-541


595-646: LGTM!

Also applies to: 884-907


996-1038: 🎯 Functional Correctness | 💤 Low value

Archived submissions return an invalid-link error.

resolvedModerationSession accepts only approved or rejected. If an operator sets the submission status to archived in Directus, row.decision stays null and resolvedModerationSession returns null, so the recipient sees the generic invalid-or-expired message instead of a resolved state. Confirm this is the intended operator experience for archived nodes.


1040-1157: LGTM!


554-562: 🎯 Functional Correctness

No recipient-case mismatch.

access_link.recipient_email is constrained to lowercase before links are inserted, so the current send guard and verification path compare the same stored form.

			> Likely an incorrect or invalid review comment.
packages/shared/src/map-nodes.ts (1)

50-83: LGTM!

Also applies to: 483-515

scripts/directus-operational-content-setup.ts (1)

19-19: LGTM!

scripts/directus-studio-setup.test.ts (1)

14-14: LGTM!

Also applies to: 30-32, 91-91

scripts/map-node-moderation.test.ts (1)

2-13: LGTM!

Also applies to: 104-161, 163-184, 186-209, 211-265, 267-311, 313-370, 372-384, 386-396, 398-455

.env.example (1)

31-33: LGTM!

packages/agent/fly.toml (1)

16-17: LGTM!

packages/admin/README.md (1)

285-298: LGTM!

scripts/directus-studio-setup.ts (1)

105-105: 🔒 Security & Privacy

No change needed. STUDIO_BOOKMARKS has no map_node_moderation_access_links entries, and the access-link collection is excluded from steward read permissions.

Comment thread packages/agent/src/app.ts
Comment on lines +439 to +475
app.post(MAP_NODE_MODERATION_SESSION_ROUTE, async (context) => {
preparePrivateModerationResponse(context);
if (!isAllowedModerationOrigin(context.req.header('origin'))) {
return context.json({
error: {
code: 'invalid_moderation_origin',
message: 'This moderation request is not allowed.',
},
}, 403);
}
if (!isJsonRequest(context)) {
return context.json({
error: {
code: 'invalid_moderation_request',
message: 'This moderation request must use JSON.',
},
}, 415);
}

const input = await readModerationJsonObject(context);
if (!input) {
return context.json({
error: {
code: 'invalid_moderation_request',
message: 'This moderation request is invalid.',
},
}, 400);
}

try {
const session = await mapNodeRepository.getModerationSession(input?.token);
return context.json(session);
} catch (error) {
const response = publicErrorResponse(error);
return context.json(response.body, response.status as any);
}
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for an exported assertion/normalizer for the moderation session payload.
rg -n 'ModerationSession|assertAuthenticatedMapNodeModeration|containsPrivateMapNodeField' packages/shared/src packages/agent/src | head -50

Repository: greenpill-dev-guild/network

Length of output: 2471


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== app.ts moderation route =="
sed -n '430,475p' packages/agent/src/app.ts

echo
echo "== map-node-moderation relevant functions =="
sed -n '960,1040p' packages/agent/src/map-node-moderation.ts

echo
echo "== shared / agent map-node related exports =="
sed -n '1,120p' packages/shared/src/map-nodes.ts
sed -n '1,50p' packages/agent/src/map-node-moderation.ts
sed -n '1,80p' packages/agent/src/app.ts

echo
echo "== focused text search =="
rg -n 'ModerationSession|mapNodeRepository\.getModerationSession|toAuthenticatedMapNodeModerationNode|withPrivate|withPublic|assert|containsPrivateMapNodeField|isPublic|json\(session\)' packages/agent/src packages/shared/src

Repository: greenpill-dev-guild/network

Length of output: 17462


Guard the moderation session response with the shared public assertion.

getModerationSession() is returned directly in context.json(session). Apply assertAuthenticatedMapNodeModerationSession before returning so raw_note, owner_email, ip_address, and spam metadata cannot leak from this public agent route.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/agent/src/app.ts` around lines 439 - 475, Update the moderation
session handler to pass the result of mapNodeRepository.getModerationSession
through assertAuthenticatedMapNodeModerationSession before context.json returns
it, preserving the existing error handling and response flow.

Source: Coding guidelines

Comment on lines +132 to +148
function magicLinkConfigured(env: Record<string, string | undefined> = process.env): boolean {
const config = moderationConfig(env);
return Boolean(
config.magicLinkEnabled &&
config.linkSecret.length >= 32 &&
buildModerationUrl(config.linkBaseUrl, 'preview-token')
);
}

function canSendModerationEmail(env: Record<string, string | undefined> = process.env): boolean {
const config = moderationConfig(env);
return Boolean(config.apiKey && config.from && config.recipients.length && config.queueUrl);
}

function moderationLinkExpiry(now = new Date()): Date {
return new Date(now.getTime() + MAP_NODE_MODERATION_LINK_TTL_DAYS * 24 * 60 * 60 * 1000);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Magic-link delivery still depends on the Directus URL.

deliverQueuedMapNodeModerationNotifications returns early when canSendModerationEmail is false, and canSendModerationEmail requires config.queueUrl. queueUrl comes from MAP_NODE_MODERATION_DIRECTUS_URL. If an operator enables magic links but leaves MAP_NODE_MODERATION_DIRECTUS_URL empty, no moderation email is sent at all and the notification stays queued. Gate the Directus requirement on the fallback path only, or document that MAP_NODE_MODERATION_DIRECTUS_URL stays mandatory.

♻️ Proposed gate change
 function canSendModerationEmail(env: Record<string, string | undefined> = process.env): boolean {
   const config = moderationConfig(env);
-  return Boolean(config.apiKey && config.from && config.recipients.length && config.queueUrl);
+  const base = Boolean(config.apiKey && config.from && config.recipients.length);
+  return base && Boolean(config.queueUrl || magicLinkConfigured(env));
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function magicLinkConfigured(env: Record<string, string | undefined> = process.env): boolean {
const config = moderationConfig(env);
return Boolean(
config.magicLinkEnabled &&
config.linkSecret.length >= 32 &&
buildModerationUrl(config.linkBaseUrl, 'preview-token')
);
}
function canSendModerationEmail(env: Record<string, string | undefined> = process.env): boolean {
const config = moderationConfig(env);
return Boolean(config.apiKey && config.from && config.recipients.length && config.queueUrl);
}
function moderationLinkExpiry(now = new Date()): Date {
return new Date(now.getTime() + MAP_NODE_MODERATION_LINK_TTL_DAYS * 24 * 60 * 60 * 1000);
}
function magicLinkConfigured(env: Record<string, string | undefined> = process.env): boolean {
const config = moderationConfig(env);
return Boolean(
config.magicLinkEnabled &&
config.linkSecret.length >= 32 &&
buildModerationUrl(config.linkBaseUrl, 'preview-token')
);
}
function canSendModerationEmail(env: Record<string, string | undefined> = process.env): boolean {
const config = moderationConfig(env);
const base = Boolean(config.apiKey && config.from && config.recipients.length);
return base && Boolean(config.queueUrl || magicLinkConfigured(env));
}
function moderationLinkExpiry(now = new Date()): Date {
return new Date(now.getTime() + MAP_NODE_MODERATION_LINK_TTL_DAYS * 24 * 60 * 60 * 1000);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/agent/src/map-node-moderation.ts` around lines 132 - 148, Update
canSendModerationEmail and the delivery flow in
deliverQueuedMapNodeModerationNotifications so MAP_NODE_MODERATION_DIRECTUS_URL
is required only when the fallback moderation path needs it, while magic-link
delivery remains possible without queueUrl. Preserve the existing validation for
API key, sender, recipients, and ensure fallback delivery still rejects a
missing queueUrl.

Comment on lines +252 to +258
declineButton.addEventListener('click', () => {
actionFailure.hidden = true;
declineDialog.showModal();
declineNote.focus();
});
approveCancel.addEventListener('click', () => approveDialog.close());
declineCancel.addEventListener('click', () => declineDialog.close());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Reset the decline note when the dialog reopens.

The decline dialog keeps the previous note text and the previous counter value after a cancel. A moderator who cancels and reopens the dialog can submit a note they intended to discard. Clear declineNote.value and reset declineCount when the dialog opens or closes.

🐛 Proposed fix
           declineButton.addEventListener('click', () => {
             actionFailure.hidden = true;
+            declineNote.value = '';
+            declineCount.textContent = '0 / 500';
             declineDialog.showModal();
             declineNote.focus();
           });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
declineButton.addEventListener('click', () => {
actionFailure.hidden = true;
declineDialog.showModal();
declineNote.focus();
});
approveCancel.addEventListener('click', () => approveDialog.close());
declineCancel.addEventListener('click', () => declineDialog.close());
declineButton.addEventListener('click', () => {
actionFailure.hidden = true;
declineNote.value = '';
declineCount.textContent = '0 / 500';
declineDialog.showModal();
declineNote.focus();
});
approveCancel.addEventListener('click', () => approveDialog.close());
declineCancel.addEventListener('click', () => declineDialog.close());
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/website/src/pages/map/moderate.astro` around lines 252 - 258, Update
the decline dialog handlers around declineButton and declineCancel so reopening
or closing the dialog clears declineNote.value and resets declineCount to its
initial value, preventing discarded text and counter state from persisting.

min-height: 30px;
padding-inline: 12px;
border: 1px solid var(--gp-border);
border-radius: 999px;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use radius tokens instead of literal radii.

Four rules hardcode radii: 999px for the theme chip and the button, 12px for the textarea, and 18px for the panel in the container query. Chips, buttons, and inputs must use --gp-radius-pill. The panel must use a --gp-radius-* token.

As per coding guidelines: "Use --gp-radius-pill for buttons, chips, inputs, and avatars; use --gp-radius-{sm,md,lg,xl} for other elements".

♻️ Proposed fix
-        border-radius: 999px;
+        border-radius: var(--gp-radius-pill);
         color: var(--gp-primary);
-        border-radius: 12px;
+        border-radius: var(--gp-radius-pill);
         background: var(--gp-green-950);
-        .panel { border-radius: 18px; }
+        .panel { border-radius: var(--gp-radius-md); }

Also applies to: 370-370, 408-408, 416-416

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/website/src/pages/map/moderate.astro` at line 359, Replace the
hardcoded radii in the styles around the theme chip, button, textarea, and
container-query panel with the appropriate design tokens: use --gp-radius-pill
for the chip, button, and textarea, and a suitable --gp-radius-* token for the
panel.

Source: Coding guidelines

Comment on lines +377 to +380
.button:focus-visible, textarea:focus-visible {
outline: 3px solid var(--gp-secondary);
outline-offset: 3px;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Do not override :focus-visible, and do not use gold on interactive elements.

Lines 377-380 replace the global lime focus ring with a gold outline. The token system reserves gold for headlines and forbids overriding the global focus style. Remove this rule and let gp-tokens.css supply the focus ring.

As per coding guidelines: "Never override :focus-visible (lime ring) or reduced-motion global styles defined in gp-tokens.css" and "never make gold interactive".

♻️ Proposed fix
-      .button:focus-visible, textarea:focus-visible {
-        outline: 3px solid var(--gp-secondary);
-        outline-offset: 3px;
-      }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
.button:focus-visible, textarea:focus-visible {
outline: 3px solid var(--gp-secondary);
outline-offset: 3px;
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/website/src/pages/map/moderate.astro` around lines 377 - 380, Remove
the .button:focus-visible and textarea:focus-visible rule from the moderate page
styles, including its gold outline and offset, so the global lime focus ring
from gp-tokens.css remains in effect.

Source: Coding guidelines

import test from 'node:test';
import { runInNewContext } from 'node:vm';

const rootDir = resolve(new URL('..', import.meta.url).pathname);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use fileURLToPath to resolve the repository root.

URL.pathname keeps percent-encoding and prefixes a drive path with / on Windows. A checkout in a directory with a space resolves to a wrong path and the test fails to read the built HTML.

🐛 Proposed fix
-import { join, resolve } from 'node:path';
+import { join } from 'node:path';
+import { fileURLToPath } from 'node:url';
 import test from 'node:test';
 import { runInNewContext } from 'node:vm';
 
-const rootDir = resolve(new URL('..', import.meta.url).pathname);
+const rootDir = fileURLToPath(new URL('..', import.meta.url));
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/map-moderation-route.test.ts` at line 7, Update the rootDir
initialization to convert the parent file URL with fileURLToPath before passing
it to resolve, preserving correct filesystem paths for spaces and Windows drive
letters. Import and use fileURLToPath in the test setup while keeping the
existing repository-root target unchanged.

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

Copy link
Copy Markdown

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: 72ef2fe3f8

ℹ️ 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/agent/src/app.ts
const declaredLength = Number(context.req.header('content-length'));
if (Number.isFinite(declaredLength) && declaredLength > 4096) return null;

const rawBody = await context.req.text();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Enforce the limit before buffering request bodies

When a client omits Content-Length or uses chunked transfer encoding, both public moderation endpoints call context.req.text() before applying the 4 KiB check, so an unauthenticated request can force the service to buffer an arbitrarily large body. The size limit should be enforced while reading the stream, or by server middleware that rejects oversized bodies before materializing them.

Useful? React with 👍 / 👎.

Comment on lines +999 to +1002
const reviewedAtValue = row.resolvedAt ?? row.submissionUpdatedAt;
const reviewedAt = reviewedAtValue instanceof Date
? reviewedAtValue.toISOString()
: cleanString(reviewedAtValue);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Use the actual review time for Directus-first decisions

When Directus resolves a submission, the access-link row has no resolvedAt, so this fallback reports the submission's mutable updated_at as reviewedAt. If any field is edited after approval or rejection while the seven-day link remains valid, reopening the link displays that later edit time as the decision time; persist or query the status-transition timestamp instead, or omit the timestamp when it is unavailable.

Useful? React with 👍 / 👎.

return { queued: 0, delivered: 0, failed: 0, skipped: 0 };
}

if (magicLinkConfigured(env)) await cleanupMapNodeModerationAccessLinks(sql);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Run expired-link cleanup after disabling magic links

The only production cleanup call is guarded by magicLinkConfigured(env), so following the documented rollback path (MAP_NODE_MODERATION_MAGIC_LINK_ENABLED=false) stops purging previously issued access-link rows. Those rows contain private recipient email addresses and will then remain indefinitely unless the feature is re-enabled or the SQL function is invoked manually; cleanup should run independently of whether new magic links are enabled (and ideally independently of mail-provider readiness).

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automated/codex Created by Codex automation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants