feat: PageIndex doc sync, thread events, API routes; remove root .env…#12
feat: PageIndex doc sync, thread events, API routes; remove root .env…#12MohnishBangaru wants to merge 1 commit into
Conversation
….example - Listing sync-documents + PageIndex helpers; chat/negotiation route updates - Supabase thread events migration, schema; ignore nested node_modules/.next - Remove root .env.example; document env via README/CLAUDE/docker Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
This PR wires up negotiation-thread audit logging and listing-document indexing (PageIndex) into the Next.js negotiation/chat flow, while also updating local/dev ops ergonomics (Supabase config, Docker env/ports) and workspace structure.
Changes:
- Add
thread_eventsaudit-log table + migration and persist helpers used by/api/chatand/api/negotiations/*. - Add PageIndex upload + URL allowlisting utilities, plus listing-doc sync invoked from negotiation start and exposed via an API route.
- Refactor listing chat UI +
/api/chatto use the AI SDK tool pipeline and new negotiation gateway routes; update repo/workspace + env/Docker docs.
Reviewed changes
Copilot reviewed 39 out of 42 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| supabase/schema.sql | Adds thread_events table + index to the schema dump. |
| supabase/migrations/004_thread_events.sql | Introduces migration to create thread_events table + index. |
| supabase/config.toml | Adds Supabase CLI local config. |
| supabase/.gitignore | Ignores Supabase temp/branches and dotenvx files. |
| src/lib/thread-events.ts | Adds resilient audit-event persistence into Supabase. |
| src/lib/sync-listing-docs-pageindex.ts | Adds listing-doc URL fetch + PageIndex upload sync helper. |
| src/lib/supabase-admin.ts | Adds cached service-role Supabase client creation. |
| src/lib/pageindex.ts | Adds PageIndex upload + allowlisted URL fetching and size/type enforcement. |
| src/lib/negotiation-service-url.ts | Centralizes negotiation gateway base URL resolution. |
| src/components/listing-chat.tsx | Refactors listing chat to AI SDK useChat, tool-output rendering, and gateway bootstrap. |
| src/app/api/negotiations/start/route.ts | Starts gateway thread, syncs docs, and logs a thread event. |
| src/app/api/negotiations/[threadId]/takeover/route.ts | Proxies takeover to gateway + logs thread event. |
| src/app/api/negotiations/[threadId]/step/route.ts | Proxies step to gateway + logs thread event. |
| src/app/api/negotiations/[threadId]/resolve/route.ts | Proxies resolve to gateway + logs thread event. |
| src/app/api/chat/route.ts | Adds tool-orchestrated negotiation loop + listing chat specialization + research bundle. |
| scripts/run-thread-events-migration.cjs | Adds helper script to apply thread-events migration via direct Postgres. |
| pnpm-workspace.yaml | Defines workspace packages (root + packages/* + apps/*). |
| package.json | Adds workspace dependency + db script + pg dev deps + zod. |
| next.config.mjs | Adds optional allowedDevOrigins from env. |
| docker/README.md | Updates env setup instructions; adds tunneling guidance. |
| docker-compose.yml | Switches env instructions and makes host ports configurable via env vars. |
| apps/web/public/.gitkeep | Keeps public/ tracked in the apps/web app. |
| apps/web/package.json | Adds Vitest test script + dependency updates. |
| apps/web/next.config.ts | Loads repo-root .env/.env.local into process.env for the app. |
| apps/web/lib/thread-events.ts | Improves resilience/logging when thread-events persistence fails. |
| apps/web/lib/sync-listing-docs-pageindex.ts | Adds PageIndex document sync helper (apps/web variant). |
| apps/web/lib/pageindex.ts | Adds allowlist + max-size/type validation + URL-upload helper. |
| apps/web/lib/pageindex.allowlist.test.ts | Adds Vitest coverage for allowlisted URL logic. |
| apps/web/lib/negotiation-service-url.ts | Centralizes negotiation gateway URL (apps/web variant). |
| apps/web/app/api/negotiations/start/route.ts | Adds doc sync + service URL helper usage. |
| apps/web/app/api/negotiations/[threadId]/takeover/route.ts | Switches to getNegotiationServiceUrl(). |
| apps/web/app/api/negotiations/[threadId]/step/route.ts | Switches to getNegotiationServiceUrl(). |
| apps/web/app/api/negotiations/[threadId]/resolve/route.ts | Switches to getNegotiationServiceUrl(). |
| apps/web/app/api/listings/[listingId]/sync-documents/route.ts | Adds API route to trigger PageIndex doc sync per listing. |
| apps/web/app/api/listings/[listingId]/documents/route.ts | Reuses shared max-size/type constants from PageIndex lib. |
| apps/web/app/api/chat/route.ts | Extends chat route with listing-chat specialization + research bundle. |
| agentmarket_claude.md | Updates documented Gemini model name. |
| README.md | Updates env setup instructions to no longer rely on root .env.example. |
| CLAUDE.md | Updates env setup instructions similarly. |
| .gitignore | Broadens ignore patterns for nested node_modules/, .next/, out/. |
| .env.example | Removes root env example file. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const payload = startPayloadSchema.parse(await req.json()); | ||
| const documentSync = await syncListingDocumentsToPageIndex(payload.listingId); | ||
| const requestId = crypto.randomUUID(); |
There was a problem hiding this comment.
syncListingDocumentsToPageIndex is awaited inline before starting the negotiation. If a listing has multiple docs or PageIndex is slow, this will directly delay chat initialization and can also amplify costs by syncing on every session start. Consider making this best-effort and non-blocking (e.g., fire-and-forget/background job) or gating it to run only when docs changed / not yet indexed.
| return { | ||
| ok: true, | ||
| listingId, | ||
| synced, | ||
| skipped, | ||
| errors, | ||
| pageIndexConfigured: true, | ||
| }; |
There was a problem hiding this comment.
This function returns ok: true even when one or more uploads fail (errors are appended but ok never flips). That makes it hard for callers to detect partial/complete sync failure (and any API route using ok for HTTP status will report success). Consider setting ok based on whether errors.length === 0 (or introduce a separate partial flag).
| const payload = startPayloadSchema.parse(await req.json()); | ||
| const documentSync = await syncListingDocumentsToPageIndex(payload.listingId); | ||
| const requestId = crypto.randomUUID(); |
There was a problem hiding this comment.
This route awaits syncListingDocumentsToPageIndex before contacting the gateway. If PageIndex is slow or the listing has many docs, negotiation start latency will increase. Consider making document sync non-blocking/best-effort or only syncing when docs changed / not yet indexed.
| return Response.json( | ||
| { ...body, documentSync }, | ||
| { status: response.status }, | ||
| ); |
There was a problem hiding this comment.
body is obtained via await response.json() earlier in this handler; if the gateway returns non-JSON (or fetch throws), this route will currently error and return a generic 500. Consider wrapping the upstream call in try/catch and parsing via response.text() + safe JSON parsing so callers get a clear 5xx with an actionable message (similar to the more defensive start route in src/app/api/negotiations/start).
|
|
||
| const negotiationServiceUrl = | ||
| process.env.NEGOTIATION_SERVICE_URL ?? "http://localhost:8000"; | ||
| const negotiationServiceUrl = getNegotiationServiceUrl(); |
There was a problem hiding this comment.
The downstream fetch(${negotiationServiceUrl}/...) calls in this handler aren’t wrapped, so an unreachable gateway or non-JSON response will throw (fetch / response.json()) and bubble up as a generic 500. Consider adding the same try/catch + safe parsing pattern used in the newer start route so clients get a clear 5xx and message.
| const response = await fetch( | ||
| `${negotiationServiceUrl}/negotiations/${threadId}/resolve`, | ||
| { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify(envelope), | ||
| }, | ||
| ); | ||
|
|
||
| const result = await response.json(); | ||
| await persistThreadEvent({ | ||
| threadId, | ||
| eventType: "negotiation.resolve", | ||
| source: "negotiation-route", | ||
| requestId, | ||
| payload: { | ||
| request: envelope.payload, | ||
| response: result.payload ?? null, | ||
| status: response.status, | ||
| }, | ||
| }); | ||
| return Response.json(result, { status: response.status }); |
There was a problem hiding this comment.
This route assumes the upstream gateway always returns valid JSON and that fetch() never throws. If the gateway is unreachable or returns an HTML/plain-text error, fetch/response.json() will throw and the handler will 500 without a useful response. Mirror the robustness used in /api/negotiations/start (wrap fetch in try/catch and parse via response.text() + safe JSON parsing).
| const response = await fetch( | |
| `${negotiationServiceUrl}/negotiations/${threadId}/resolve`, | |
| { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify(envelope), | |
| }, | |
| ); | |
| const result = await response.json(); | |
| await persistThreadEvent({ | |
| threadId, | |
| eventType: "negotiation.resolve", | |
| source: "negotiation-route", | |
| requestId, | |
| payload: { | |
| request: envelope.payload, | |
| response: result.payload ?? null, | |
| status: response.status, | |
| }, | |
| }); | |
| return Response.json(result, { status: response.status }); | |
| try { | |
| const response = await fetch( | |
| `${negotiationServiceUrl}/negotiations/${threadId}/resolve`, | |
| { | |
| method: "POST", | |
| headers: { "Content-Type": "application/json" }, | |
| body: JSON.stringify(envelope), | |
| }, | |
| ); | |
| const text = await response.text(); | |
| let result: any; | |
| try { | |
| result = text ? JSON.parse(text) : {}; | |
| } catch { | |
| result = { raw: text }; | |
| } | |
| await persistThreadEvent({ | |
| threadId, | |
| eventType: "negotiation.resolve", | |
| source: "negotiation-route", | |
| requestId, | |
| payload: { | |
| request: envelope.payload, | |
| response: result && typeof result === "object" ? (result as any).payload ?? null : null, | |
| status: response.status, | |
| }, | |
| }); | |
| return Response.json(result, { status: response.status }); | |
| } catch (error) { | |
| const status = 502; | |
| await persistThreadEvent({ | |
| threadId, | |
| eventType: "negotiation.resolve", | |
| source: "negotiation-route", | |
| requestId, | |
| payload: { | |
| request: envelope.payload, | |
| response: null, | |
| status, | |
| error: | |
| error instanceof Error | |
| ? error.message | |
| : typeof error === "string" | |
| ? error | |
| : "Unknown error while contacting negotiation service", | |
| }, | |
| }); | |
| return Response.json( | |
| { | |
| error: "Failed to reach negotiation service", | |
| }, | |
| { status }, | |
| ); | |
| } |
| return { | ||
| ok: true, | ||
| listingId, | ||
| synced, | ||
| skipped, | ||
| errors, | ||
| pageIndexConfigured: true, | ||
| }; |
There was a problem hiding this comment.
This function returns ok: true even when one or more uploads fail (errors are appended but ok never flips). That makes it hard for callers to detect partial/complete sync failure (and any API route using ok for HTTP status will report success). Consider setting ok based on whether errors.length === 0 (or introduce a separate partial flag).
| ) { | ||
| const params = listingSchema.parse(await context.params); | ||
| const result = await syncListingDocumentsToPageIndex(params.listingId); | ||
| const status = result.ok ? 200 : 502; |
There was a problem hiding this comment.
This API route uses result.ok to choose between 200 vs 502, but syncListingDocumentsToPageIndex currently returns ok: true even when uploads fail. As a result this endpoint will almost always return 200 even on sync failures. Fix by making ok reflect errors (or compute status here from errors.length).
| const status = result.ok ? 200 : 502; | |
| const hasErrors = | |
| Array.isArray((result as any).errors) && (result as any).errors.length > 0; | |
| const status = hasErrors ? 502 : 200; |
| const response = await fetch( | ||
| `${negotiationServiceUrl}/negotiations/${threadId}/takeover`, | ||
| { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify(envelope), | ||
| }, | ||
| ); | ||
|
|
||
| const result = await response.json(); | ||
| await persistThreadEvent({ |
There was a problem hiding this comment.
This route assumes the upstream gateway always returns valid JSON and that fetch() never throws. If the gateway is unreachable or returns an HTML/plain-text error, fetch/response.json() will throw and the handler will 500 without a useful response. Mirror the robustness used in /api/negotiations/start (wrap fetch in try/catch and parse via response.text() + safe JSON parsing).
| const response = await fetch( | ||
| `${negotiationServiceUrl}/negotiations/${threadId}/step`, | ||
| { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify(envelope), | ||
| }, | ||
| ); | ||
|
|
||
| const body = await response.json(); | ||
| await persistThreadEvent({ |
There was a problem hiding this comment.
This route assumes the upstream gateway always returns valid JSON and that fetch() never throws. If the gateway is unreachable or returns an HTML/plain-text error, fetch/response.json() will throw and the handler will 500 without a useful response. Mirror the robustness used in /api/negotiations/start (wrap fetch in try/catch and parse via response.text() + safe JSON parsing).
….example