Skip to content

chore: sync cognee-frontend from SaaS repo [CLO-381] - #4867

Open
goran-radonic wants to merge 8 commits into
devfrom
sync/frontend-20260901-185616
Open

chore: sync cognee-frontend from SaaS repo [CLO-381]#4867
goran-radonic wants to merge 8 commits into
devfrom
sync/frontend-20260901-185616

Conversation

@goran-radonic

Copy link
Copy Markdown
Contributor

Automated frontend sync

This PR was auto-generated by the sync-frontend-to-public workflow
in the SaaS repo (tracking issue: CLO-381).

Source commit: https://github.com/topoteretes/cognee-saas/commit/249459fe9d8dd96f677cd078066a766dc31ce62e

What changed: Shared frontend code (routes, components, modules)
was updated in the SaaS repo and synced to cognee-frontend/ via
scripts/sync-to-public.sh.

Cloud-only logic (Auth0, Stripe, Segment, billing/tenant backends) is
excluded; public stubs replace private module dependencies so shared
UI compiles and degrades gracefully in local mode.

Verified by the workflow before this PR was opened:

  • every @/ import resolves (verify-public-imports.js)
  • no known private import patterns leaked
  • package-lock.json regenerated to match the stripped package.json
  • npm ci && tsc --noEmit passes on the synced output

Review checklist

  • npm install && npm run dev works in cognee-frontend/
  • cognee -ui launches the updated frontend

Automated sync from topoteretes/cognee-saas.
Source commit: 249459fe9d8dd96f677cd078066a766dc31ce62e
Comment thread cognee-frontend/src/modules/integrations/__tests__/oauthOutcome.test.ts Dismissed
The last sync from the SaaS repo added a "prepare": "node
scripts/setup-git-hooks.js" script to cognee-frontend/package.json,
but scripts/setup-git-hooks.js never existed in this public repo.
npm runs "prepare" on every install, so npm ci now fails with
MODULE_NOT_FOUND anywhere it runs clean, including the
"Build Cognee UI Docker Image" CI job on PR #4867.

Git-hooks setup is SaaS-team tooling with no OSS counterpart, so
drop the script reference instead of trying to port the file over.
The latest sync from the SaaS repo dropped "output: standalone" from
next.config.ts, silently reverting COG-6319 (#4847). Without it,
`next build` produces no .next/standalone directory, so the
Dockerfile's runner stage has nothing to COPY and the "Build Cognee
UI Docker Image" CI job fails on PR #4867.
The latest sync from the SaaS repo dropped src/modules/config/*
and src/app/api/runtime-config/route.ts (COG-6343, #4821), and
layout.tsx lost its RuntimeConfigScript + force-dynamic wiring.
Without these, a published cognee/cognee-ui image bakes in whatever
COGNEE_BACKEND_URL was set at build time and ignores the one it's
actually run with — the entire point of shipping one prebuilt image.

Restored the files verbatim from 989e5ce (the last commit where
they existed) and re-wired layout.tsx. Verified against a real
`docker build` + `docker run`, matching every assertion in
frontend_docker_build_test.yml: /local-login and /api/runtime-config
both carry the runtime COGNEE_BACKEND_URL, and a malformed URL still
makes the container refuse to start and name the variable.
@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

An automated sync that claims to "restore runtime backend URL config" while systematically deleting it — the confidence of a script that never reads its own commit messages. 🤖

🔴 2 blockers, 1 major — changes requested

  • blockercognee-frontend/src/modules/instances/localFetch.ts:3 — Runtime config support removed; Docker containers can't override backend URL
  • blockercognee-frontend/src/app/(auth)/local-login/partials/LocalSignInForm.tsx:7 — Same regression; hardcoded fallback ignores COGNEE_BACKEND_URL
  • majorcognee-frontend/src/modules/configuration/pipelineSettings.ts:27 — SSR crash risk: localStorage accessed without typeof window guard
  • minorcognee-frontend/package.json:7test script removed without explanation

See inline comments for details.

Fix the review findings in PR #4867:
1. [blocker] `cognee-frontend/src/modules/instances/localFetch.ts:3` — Restore getLocalApiUrl() or inline its fallback chain: read runtime config from the DOM (via readRuntimeConfig()), fall back to NEXT_PUBLIC_LOCAL_API_URL, then construct from window.location.hostname. The layout still renders RuntimeConfigScript, so the server emits the config — clients just stopped reading it.
2. [blocker] `cognee-frontend/src/app/(auth)/local-login/partials/LocalSignInForm.tsx:7` — Restore the getLocalApiUrl() call (same fix as localFetch.ts) so Docker deployments can override the backend URL at container start with -e COGNEE_BACKEND_URL=...
3. [major] `cognee-frontend/src/modules/configuration/pipelineSettings.ts:27,39` — Guard localStorage access with "if (typeof window === 'undefined') return ..." in both getPipelineSettingsFromStorage (return defaults) and storePipelineSettingsLocally (early return) to prevent SSR crashes.
4. [minor] `cognee-frontend/package.json:7` — Restore the "test": "jest" script if it was accidentally removed, or document in the PR body why testing was dropped.
Then run the test suite.

Comment thread cognee-frontend/src/modules/instances/localFetch.ts Outdated
Comment thread cognee-frontend/src/app/(auth)/local-login/partials/LocalSignInForm.tsx Outdated
Comment thread cognee-frontend/src/modules/configuration/pipelineSettings.ts
Comment thread cognee-frontend/package.json
Follow-up to b0c7847: an adversarial review found the runtime-config
restore had covered 3 of 10 reverted call sites, and turned up three
regressions from the same sync that no reviewer had flagged.

- api/visualize and api/local-signout still read the build-time
  NEXT_PUBLIC_LOCAL_API_URL, so on any non-default backend the mindmap
  fetched from the wrong host and sign-out silently left the backend
  session alive (the route's catch swallowed it and still redirected).
  Both now use getServerBackendUrl(), which also gives that function
  real callers again. Verified with two listeners behind a container:
  every request lands on COGNEE_BACKEND_URL's port, none on the
  build-time one.

- isOwner was permanently false: useTenant() derives it from
  UserContext's availableTenants, the cloud UserProvider that fills
  that list is never mounted in local mode, and the sync deleted
  LocalProvider's `isOwner: true`. Every Connect button on the Data
  Sources cards rendered inert. LocalProvider now supplies the local
  workspace as an owned tenant; added a test that fails without it.

- getDatasets and useChat regained trailing slashes that 4c8ed2d
  removed for triggering cross-origin 307s that broke session auth.

- npm test pointed at a red suite: d3's ESM-only packages were never
  transformed, and missing clearMocks leaked call counts between
  tests. next/jest only appends to transformIgnorePatterns and its own
  blanket node_modules entry wins, so the resolved config is widened
  after the fact rather than through transpilePackages, which would
  change the production build to fix a test problem. 49 suites / 481
  tests green, from 42 / 416.

Also drops two dead build-time URL constants (getLocalUser, ApiKeysPage)
that 989e5ce had already removed for the same reason.
Comment thread cognee-frontend/package.json
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

An adversarial review that fixes ten call sites but doesn't check if their imports still resolve — chef's kiss levels of irony. 🎭

🔴 1 blocker — changes requested

  • blockercognee-frontend/package.json:34 — Missing server-only dependency breaks production build and Docker image

See inline comments for details.

Fix the review findings in PR #4867:
1. [blocker] `cognee-frontend/package.json:34` — Add `"server-only": "^0.0.1"` back to dependencies after the `react-markdown` line. The automated sync removed it, but `src/modules/config/serverRuntimeConfig.ts:5` still imports it, breaking production builds and causing runtime failures in `/api/visualize` and `/api/local-signout` routes. This package was intentionally added in #4847 (989e5ce84) to guard server-side env reads; the sync regression must be fixed before merge.
Then run `cd cognee-frontend && npm install && npm run build` to verify the build passes.

The sync dropped "server-only" from package.json and the lockfile, but
src/modules/config/serverRuntimeConfig.ts still imports it. It was added
deliberately in COG-6482 (b4343aa) to make the server-side guard on
COGNEE_BACKEND_URL explicit, so a client component importing that module
fails loudly instead of silently reading undefined and falling back to
localhost.

Nothing is visibly broken today: Next resolves the bare specifier through
its own alias (webpack-config.js matches /^server-only$/ against
next/dist/compiled/server-only), so the image builds and both route
handlers work. That is an implementation detail of Next, not a contract,
and it is the wrong thing to leave a deliberate guard resting on.
Comment thread cognee-frontend/src/app/api/schema/inventory/route.ts Outdated
Comment thread cognee-frontend/src/app/api/schema/provenance/route.ts Outdated
Comment thread cognee-frontend/src/app/api/schema-provenance/route.ts Outdated
Comment thread cognee-frontend/src/instrumentation.ts Outdated
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Five fix commits to repair a sync, and the "adversarial review" still missed three routes with the exact bug it just fixed — clearly even the adversary got tired after 26k lines.

🔴 3 blockers, 1 minor — changes requested

  • blockercognee-frontend/src/app/api/schema/inventory/route.ts:3 — Reads build-time URL instead of runtime COGNEE_BACKEND_URL, breaks non-localhost deployments
  • blockercognee-frontend/src/app/api/schema/provenance/route.ts:3 — Same build-time URL bug, duplicates the issue c81c093 fixed for visualize/local-signout
  • blockercognee-frontend/src/app/api/schema-provenance/route.ts:3 — Claims to mirror visualize but uses wrong config pattern; runtime URL ignored
  • minorcognee-frontend/src/instrumentation.ts:1 — Sync deleted startup validation; broken config now surfaces on first request, not at boot

See inline comments for details.

Fix the review findings in this PR:
1. [blocker] `cognee-frontend/src/app/api/schema/inventory/route.ts:3` — Import getServerBackendUrl from @/modules/config/serverRuntimeConfig and call it in the handler body (line 5: const localApiUrl = getServerBackendUrl();) instead of reading process.env.NEXT_PUBLIC_LOCAL_API_URL at module scope. Mirrors the pattern in /api/visualize/route.ts.
2. [blocker] `cognee-frontend/src/app/api/schema/provenance/route.ts:3` — Same fix: import getServerBackendUrl and call it at the start of the GET handler instead of the build-time env var.
3. [blocker] `cognee-frontend/src/app/api/schema-provenance/route.ts:3` — Same fix: import getServerBackendUrl, remove the module-scope localApiUrl constant (line 3), and call getServerBackendUrl() in the handler body.
4. [minor] `cognee-frontend/src/instrumentation.ts:1` — Restore the startup validation: import getServerBackendUrl, wrap the call in try/catch, log errors with console.error([cognee-ui] prefix). Not blocking but surfaces config issues earlier.
Then run the test suite.

Four more files the sync reverted, same family as c81c093.

The three schema proxy routes had their getServerBackendUrl() call
replaced by a build-time NEXT_PUBLIC_LOCAL_API_URL constant, so they
would fetch from whatever host was baked in at build time rather than
the container's COGNEE_BACKEND_URL. All three are now byte-identical to
989e5ce, and no hardcoded fallback is left anywhere under src.

Those three are currently unreachable: the sync deleted the schema page,
and getSchemaInventory calls the backend directly rather than through
the proxy. Rewired rather than deleted because the page still exists
upstream, so deleting them now would hand a later sync's page a 404.

instrumentation.ts was gutted to an empty register(), losing the startup
check that surfaces a broken COGNEE_BACKEND_URL in the first lines of
the log. The published image already refuses to boot on one via
docker-entrypoint.sh; this is what covers `next start` and
`cognee-cli -ui`, where killing the process is not the app's call.

Verified in a container: all five proxy routes issue their requests
against the runtime host and none against the build-time one, and the
startup check logs on a malformed URL while staying silent on a valid
one.
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

A frontend sync that accidentally shipped backend migration changes — and those changes disable the safeguard preventing database corruption on transient connection failures. 🎭

🔴 2 blockers — changes requested

  • blockercognee/modules/migrations/startup.py:238-241_relational_schema_exists now treats inspection failures as "empty" instead of raising, exactly the corruption path the old docstring warned against (transient network failure → mis-stamp at head)
  • blockercognee/tests/.../test_relational_schema_probe.py (deleted) — removed the test suite that validated the inspection-failure safeguard (test_uninspectable_database_raises_instead_of_reporting_fresh)

Backend changes in a frontend sync PR suggest the sync script grabbed more than intended. The migration probe change is high-risk: a populated database that's temporarily unreachable (server still starting, credential rotation, network hiccup) will now be treated as empty, triggering create_all + stamp head which silently skips existing tables and marks every pending migration as applied without running it — permanent mis-stamp.

See inline attempt above (GitHub API can't surface 349-file diffs for inline comments). Key evidence:

  • Old docstring (deleted, lines 231-239 base): "Guessing 'empty' would send a populated-but-unreachable database into the create_all + stamp-head bootstrap...leaving the database permanently mis-stamped at head."
  • Old code: filesystem pre-check for SQLite, let inspection failures propagate
  • New code: bare except Exception: return False — treats all failures as "empty"
Fix the review findings in PR #4867:
1. [blocker] `cognee/modules/migrations/startup.py:238-241` — Restore the original `_relational_schema_exists` logic: SQLite file existence pre-check (lines 247-257 in base branch a2bc9267), no broad exception handler, let inspection failures raise. The transient-failure → mis-stamp scenario the old code prevented is real.
2. [blocker] `cognee/tests/integration/infrastructure/relational/test_relational_schema_probe.py` — Restore the deleted test file (90 lines, commit a2bc9267) that validated the probe's tri-state contract, especially `test_uninspectable_database_raises_instead_of_reporting_fresh`.
3. Verify no other backend changes leaked into the sync — this is a frontend-only PR per the title, but startup.py + test deletion suggest the sync script's scope is too broad.
Then run the test suite, especially the restored `test_relational_schema_probe.py` integration tests.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants