Sync fork main into upstream: Tap ingestion, advanced GraphQL queries, and frontend refresh#42
Sync fork main into upstream: Tap ingestion, advanced GraphQL queries, and frontend refresh#42
Conversation
… pagination (hyperindex-q00.2)
- Add DIDFilterInput GraphQL type with only eq and in fields (no contains/startsWith/neq) - Replace StringFilterInput with DIDFilterInput for the did field in WhereInput - Introduce DIDFilter struct in repositories to carry eq and in conditions - Update extractFilters to populate DIDFilter.EQ and DIDFilter.IN from GraphQL args - Add buildDIDFilterClause helper to generate SQL WHERE conditions for DIDFilter - Update all repository methods to accept DIDFilter instead of plain string - Add tests for DIDFilterInput fields, extractFilters DID handling, and DID in filtering
…yConnection (hyperindex-q00.11)
feat: add batched admin DID picker with Bluesky typeahead
Deploy batch admin add and display
Merge pull request #13 from GainForest/main
…back - Drop removeFromTap argument from purgeActor mutation and all call sites; Tap should not be touched by the indexer (separation of concerns, avoids partial-failure state where local data is deleted before Tap call fails) - Remove RemoveRepoCallback type, field, setter, and startTap wiring - Wrap DeleteByDID raw error with fmt.Errorf context - Refactor TestActorsRepository_DeleteByDID to table-driven format Co-Authored-By: Claude <noreply@anthropic.com>
…d add normalization - Rename NEXT_PUBLIC_API_URL → NEXT_PUBLIC_HYPERINDEX_URL across all files - Add NEXT_PUBLIC_HYPERINDEX_URL to env.ts with normalizePublicURL() - Update HYPERINDEX_URL fallback chain: HYPERINDEX_URL → NEXT_PUBLIC_HYPERINDEX_URL → http://127.0.0.1:8080 - Drop HYPERGOAT_URL entirely from env.ts and graphql/client.ts - Import env.HYPERINDEX_URL in graphql/client.ts instead of reading process.env directly - Add inline normalizeUrl helper to next.config.ts (can't import from src/) - Update Dockerfile ARG/ENV, .env.example comments, and docs/ENV_VARS.md Co-Authored-By: Claude <noreply@anthropic.com>
Redirect target was resolving to the internal Railway address (0.0.0.0:8080) instead of the public domain because request.url reflects the internal proxy address. Fall back to requestUrl.origin for local dev. Co-Authored-By: Claude <noreply@anthropic.com>
Prevents the GraphiQL endpoint URL from being treated as a relative path by the browser when EXTERNAL_BASE_URL is set without a protocol prefix, which caused the hostname to be doubled in the request path. Co-Authored-By: Claude <noreply@anthropic.com>
Whitespace env values were collapsing to "" inside normalizeUrl/normalizePublicURL, causing the localhost fallback to never be reached. Apply the fallback via || after normalization so blank/whitespace values correctly fall through. Co-Authored-By: Claude <noreply@anthropic.com>
…alBaseURL Co-Authored-By: Claude <noreply@anthropic.com>
feat: add actor purge workflow and identity-based cleanup
fix: harden admin purge and env validation
chore: stop tracking local Beads runtime files
fix: gate settings UI to configured admin DIDs
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
🚅 Environment hyperindex-pr-42 in hypercerts has no services deployed. 4 services not affected by this PR
|
|
Important Review skippedToo many files! This PR contains 151 files, which is 1 over the limit of 150. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (151)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| | `NEXT_PUBLIC_API_URL` | `https://api.hi.gainforest.app` | | ||
| | `HYPERINDEX_URL` | `https://api.hi.gainforest.app` | | ||
| | `COOKIE_SECRET` | *(set on Railway, do not change)* | | ||
| | `ATPROTO_JWK_PRIVATE` | *(ES256 JWK, set on Railway, do not change)* | | ||
|
|
||
| **Note:** `NEXT_PUBLIC_API_URL` is a build-time variable (inlined by Next.js during `npm run build`). The `client/Dockerfile` declares `ARG NEXT_PUBLIC_API_URL` so Railway passes it during Docker build. | ||
|
|
There was a problem hiding this comment.
🔴 SKILL.md references the stale env var NEXT_PUBLIC_API_URL at lines 101 and 106, but this PR renames it to NEXT_PUBLIC_HYPERINDEX_URL everywhere else (Dockerfile, env.ts, .env.example, docs/ENV_VARS.md). A developer following SKILL.md to deploy on Railway will set the wrong build arg, causing Next.js to silently fall back to http://127.0.0.1:8080, which breaks the production frontend.
Extended reasoning...
What the bug is and how it manifests
The newly-added .agents/skills/deploy-railway/SKILL.md file was created by this PR. The Frontend env vars table on line 101 lists NEXT_PUBLIC_API_URL as the build-time variable to set. The note on line 106 states "The client/Dockerfile declares ARG NEXT_PUBLIC_API_URL". Both of these are wrong: this same PR renames the variable from NEXT_PUBLIC_API_URL to NEXT_PUBLIC_HYPERINDEX_URL across the entire codebase.
The specific code path that triggers it
client/Dockerfile (added in this PR) declares:
ARG NEXT_PUBLIC_HYPERINDEX_URL
ENV NEXT_PUBLIC_HYPERINDEX_URL=$NEXT_PUBLIC_HYPERINDEX_URL
Next.js bakes NEXT_PUBLIC_* variables into the JavaScript bundle at build time. If Railway sets NEXT_PUBLIC_API_URL (as SKILL.md instructs), Docker never sees NEXT_PUBLIC_HYPERINDEX_URL, so it remains unset. The client-side code in client/src/lib/env.ts then resolves NEXT_PUBLIC_HYPERINDEX_URL to an empty string and falls back to http://127.0.0.1:8080 — a loopback address unreachable from the browser in production.
Why existing code doesn't prevent it
client/src/lib/env.ts validates the URL configuration at module load time, but only checks that HYPERINDEX_URL (the server-side var) doesn't point to the client origin — it does not warn when NEXT_PUBLIC_HYPERINDEX_URL is missing. The misconfiguration produces no visible error; the frontend simply makes all GraphQL requests to 127.0.0.1:8080 and gets network failures in the user's browser.
What the impact would be
Any deployer who follows the SKILL.md instructions will end up with a broken Railway frontend. Every GraphQL query from the browser fails silently (or with a CORS/network error). The Vercel-style deployment works because Vercel injects env vars differently, but the Railway Docker build path is exactly what SKILL.md is trying to guide.
How to fix it
In SKILL.md, replace both occurrences of NEXT_PUBLIC_API_URL with NEXT_PUBLIC_HYPERINDEX_URL. Also, docs/ENV_VARS.md (added in the same PR) explicitly documents NEXT_PUBLIC_API_URL as "Removed — Renamed to NEXT_PUBLIC_HYPERINDEX_URL", so the skill file is already inconsistent with the authoritative reference within this same PR.
Step-by-step proof
- Developer reads SKILL.md and sets Railway build var:
NEXT_PUBLIC_API_URL=https://api.hi.gainforest.app - Railway runs:
docker build --build-arg NEXT_PUBLIC_API_URL=... client/ client/DockerfiledeclaresARG NEXT_PUBLIC_HYPERINDEX_URL— the passedNEXT_PUBLIC_API_URLbuild arg is unknown and silently ignored.NEXT_PUBLIC_HYPERINDEX_URLremains empty inside the Docker build context.npm run buildruns; Next.js inlinesprocess.env.NEXT_PUBLIC_HYPERINDEX_URLas""into the JS bundle.- At runtime,
env.tsresolves:normalizePublicURL("") → "", then falls back tohttp://127.0.0.1:8080. - All browser-side GraphQL calls go to
http://127.0.0.1:8080/graphql— a loopback address the user's browser cannot reach — silently breaking the production frontend.
| // MaxINListSize is the maximum number of values allowed in an IN filter clause. | ||
| // SQLite has a hard 999 parameter limit (SQLITE_MAX_VARIABLE_NUMBER). | ||
| // We cap well below that to leave room for other query parameters. | ||
| MaxINListSize = 100 | ||
|
|
||
| // MaxFilterConditions is the maximum number of individual filter conditions allowed per query. | ||
| // The DID filter does not count toward this cap. | ||
| MaxFilterConditions = 20 |
There was a problem hiding this comment.
🔴 The per-filter MaxINListSize=100 cap does not protect against the aggregate parameter count when multiple IN filters are combined. With MaxFilterConditions=20, a worst-case query generates 20×100 + 4 fixed params = 2004 SQL parameters — more than double SQLite's hard SQLITE_MAX_VARIABLE_NUMBER=999 limit stated in the code comments. Even a modest query with 10 fields each filtered by 100 IN values (1004 params) exceeds this limit. Neither buildFilterClause nor GetByCollectionSortedWithKeysetCursor checks the aggregate parameter count across all filter clauses combined, and the code comment actively misleads maintainers by implying the 100-value cap is sufficient protection.
Extended reasoning...
What the bug is and how it manifests
The code introduces two new constants: MaxINListSize = 100 (records.go:31-32) and MaxFilterConditions = 20 (records.go:35-37). The comment on MaxINListSize reads: "SQLite has a hard 999 parameter limit (SQLITE_MAX_VARIABLE_NUMBER). We cap well below that to leave room for other query parameters." This reasoning is only valid when there is a single IN clause in the query. The comment was designed with a single filter in mind, not the multiplicative case of 20 simultaneous IN filters.
The specific code path that triggers it
A user constructs a valid typed collection query with multiple in filter conditions, e.g. 10 different fields each with 100 values: {field1: {in: [...100 values...]}, field2: {in: [...100 values...]}, ...}. In buildFilterClause (records.go:381-473), each in operator adds len(inVals) placeholders. The individual per-filter enforcement at records.go:461-462 rejects any single IN list over 100, but there is no check on the cumulative parameter count. The query is then passed to SQLite with 10×100 + 1 (collection) + 3 (keyset cursor) = 1004 bound parameters.
Why existing code doesn't prevent it
Validation in builder.go:607-609 enforces MaxFilterConditions=20 per query — this limits the number of individual filter conditions but not the aggregate number of SQL parameters they consume. An IN filter with 100 values counts as 1 condition but 100 parameters. There is no code anywhere in buildFilterClause, GetByCollectionSortedWithKeysetCursor, GetByCollectionFilteredWithKeysetCursor, or the GraphQL resolver layer that computes or caps the total SQL parameter count across all combined filters.
What the impact would be
On SQLite builds where SQLITE_MAX_VARIABLE_NUMBER=999 (the pre-3.32.0 default), any query combining 10 or more in filters with 100 values each will cause SQLite to return a "too many SQL variables" error, which surfaces as a GraphQL error. One verifier notes that modernc.org/sqlite v1.44.3 likely bundles SQLite 3.32.0+ (which raised the limit to 32766), so the error may not manifest in production today. However, the code comment actively states the 999 limit as a design constraint, the invariant is violated by the constants introduced in this PR, and any deployment using a system SQLite or a build with the old limit would fail silently.
Step-by-step proof
- Schema has a collection with 10+ string fields (e.g.
org.hypercerts.collectionis a realistic example). - A client sends:
orgHypercertsCollection(where: {field1: {in: [v1...v100]}, field2: {in: [v1...v100]}, ..., field10: {in: [v1...v100]}}) extractFilters(builder.go:571) maps each field to 10FieldFilterstructs with operatorinand 100-elementValueslices. 10 conditions <MaxFilterConditions=20, so no error here.buildFilterClause(records.go:381) iterates and generates 10 × 100 = 1000?/$Nplaceholders.GetByCollectionSortedWithKeysetCursorprepends placeholder 1 (collection) and placeholders 2-4 (keyset cursor) = 4 fixed params.- Total SQL parameters = 1004 > 999. SQLite (legacy builds) returns
SQLITE_ERROR: too many SQL variables.
How to fix it
Track the total running parameter count in buildFilterClause (or in the callers), and return an error when the cumulative count would exceed a safe threshold (e.g. 950, leaving headroom for the fixed params). Alternatively, reduce MaxINListSize to a value where even the worst-case combination of MaxFilterConditions IN filters stays under the limit: 950 / 20 = 47 values per IN list would be safe. The existing comment should also be updated to reflect the multiplicative interaction between the two constants.
| searchQuery, _ := p.Args["query"].(string) | ||
| if utf8.RuneCountInString(searchQuery) < 3 { | ||
| return nil, fmt.Errorf("search query must be at least 3 characters") | ||
| } |
There was a problem hiding this comment.
🔴 The API documentation states the search query minimum is 2 characters, but the code in createSearchResolver() enforces a minimum of 3 characters. Clients following the documented contract will receive unexpected errors for 2-character queries; fix by updating the docs to say "minimum 3 characters".
Extended reasoning...
Bug: Doc/code mismatch on search query minimum length
The search resolver in internal/graphql/schema/builder.go (lines 875-878) enforces a minimum of 3 Unicode rune characters via utf8.RuneCountInString(searchQuery) < 3, returning the error "search query must be at least 3 characters". However, the agent API documentation table in client/src/app/docs/agents/route.ts (line 512) describes the query argument as "Search text (minimum 2 characters)".
Code path that triggers it: Any GraphQL client calling the search field with a 2-character query (e.g., {search(query: "hi") {...}}) will be rejected by the resolver, even though the published documentation explicitly allows it.
Why existing code doesn't prevent it: There is no integration test or documentation linting step that cross-checks the prose description against the runtime validation threshold. The code comment in builder.go even says "minimum 3 runes", but this was never propagated back to the documentation string.
Impact: Any API consumer who reads the documentation and builds logic around a 2-character minimum will encounter confusing, undocumented errors in production. This is a broken API contract that violates the principle of least surprise, particularly for callers building search-as-you-type UIs that start querying at 2 characters.
How to fix: Update the documentation table row in client/src/app/docs/agents/route.ts line 512 from Search text (minimum 2 characters) to Search text (minimum 3 characters) to match the enforced runtime behavior. Alternatively, lower the code threshold to 2 if 2-character searches are intended to be supported.
Step-by-step proof:
- A client reads the docs and sees:
|query| String! | Yes | Search text (minimum 2 characters) | - Client sends GraphQL query:
{ search(query: "hi") { edges { node { uri } } } } - The resolver runs
utf8.RuneCountInString("hi") < 3→2 < 3→true - Resolver returns error:
"search query must be at least 3 characters" - Client receives a 200 with a GraphQL error body, contradicting the documented minimum of 2.
2 participants
Summary
This PR syncs the fork’s
mainbranch into upstreammainand brings over a large set of backend, frontend, infra, and DX improvements.The biggest changes are:
What’s included
GraphQL / query improvements
where) supportsortBy/sortDirectionlast/before) and page size clampingtotalCounton public connectionsdidandrkeyfields to typed record GraphQL typesTap sidecar integration
internal/tappackage with event parsing, consumer, admin client, and handlerFrontend / UX refresh
Admin / product improvements
DevOps / CI / docs