Consolidate local Engraphis release branches#81
Merged
Coding-Dev-Tools merged 94 commits intoJul 27, 2026
Conversation
Closes defects that denied paying customers what they bought, hung or crashed the client when the cloud was unreachable, and leaked a credential into logs. Each fix carries a regression test. Paying customers were denied what they bought - /api/license always returned an empty feature list, so Analytics, Automation and Team rendered with PRO/TEAM lock badges for customers who had paid for them. It now reports the entitled feature set. - The plan is read from the registration and refresh handshake, which now discloses it, with an explicit precedence: ENGRAPHIS_CLOUD_PLAN override, then the persisted session plan, then the cached entitlement, then inference. A Team customer previously had to set an undocumented environment variable or be shown PRO. - Team upgrade links pointed at the Pro page. - A 409 consent_required is no longer treated as a billing answer. It was folded into the entitlement check, so a paying customer who had not set ENGRAPHIS_MANAGED_COMPUTE_CONSENT=1 was shown a panel telling them to buy the plan they already owned. Consent is now routed ahead of entitlement at every hosted call site. - loadSyncStatus rendered the purchase panel for every failure, including a transient network error or a 5xx, so a dropped connection told a paying customer to buy Pro. - /memory/license reported plan "local" for a connected paying customer, so the two license surfaces contradicted each other. Cloud failure modes - Draining an HTTP error body inside the except block let a timeout escape as an unhandled TimeoutError, surfacing as an opaque 500 with a traceback exactly when the cloud was flaky. - A DNS or resolution failure raised the same error as a genuinely malformed URL and was reported to the customer as a non-transient configuration problem with no retry. - Lapsed billing was indistinguishable from an outage: 402, 429 and 404 all collapsed into one 503, so a past_due customer retried forever instead of being sent to billing. - Connection attempts shared one deadline instead of giving each vetted address the full timeout, which turned a 10s refresh budget into N x 10s while holding the exclusive cross-process refresh lock. - PinnedHTTPSConnection pins a default connect timeout rather than inheriting urllib's block-forever default. Credential handling - The access token is excluded from the client's repr. The dataclass default printed a live bearer token into any traceback frame or debug log. - update_check.py uses the pinned HTTPS opener, so the one credential-adjacent request with a user-controllable endpoint is no longer exempt from the repo's SSRF and DNS-rebinding vetting. Updater - scripts/update.py had nineteen untimed network subprocess calls, all with captured output, so engraphis-update hung silently and indefinitely against a stalled index or unreachable remote. A stall mid-upgrade left the tree checked out to the new tag with the rollback unreached. Every call is now bounded and the rollback runs on timeout. Test suite: ~1209 -> 1363 passing, plus 11 end-to-end tests. The end-to-end suite previously only ever exercised a free customer, which is why a paying user seeing a lock badge survived this long; it now covers Pro, Team and lapsed Team.
…e floor
tests/test_client_launch.py and tests/test_hosted_plan_resolution.py import
engraphis.routes.v2_api, which imports FastAPI. The core-py39 CI job installs
numpy and pytest only, so both modules raised ModuleNotFoundError at collection
and interrupted the whole run rather than skipping.
Guard both with pytest.importorskip("fastapi"), matching the convention already
used by tests/test_agent_connect.py and tests/test_app_auth.py.
Verified against a numpy+pytest-only virtualenv: the suite collects and passes
with these two modules skipped, and the full extras install is unchanged at
1363 passed, 7 skipped.
Customers were told to hand-edit ENGRAPHIS_MANAGED_COMPUTE_CONSENT=1 in a .env file before Analytics or Automation would work. Consent now travels with the cloud account: connecting an installation accepts the terms that cover managed compute, so a connected installation is allowed by default and the variable is never shown to a user. An installation with no cloud session is still never allowed -- there is no account, so there is no agreement to rely on. The variable survives as an operator override only; =0 opts a connected installation back out. Also fixes two predicates that were inverted against their own comment. managedConsentRequired() was guarded on CURRENT_VIEW!=='analytics' && CURRENT_VIEW!=='automation', but its only four call sites run in exactly those two views, so it could never return true. hostedFeatureUnavailable() returned true for any error in those views regardless of status. Together a paying Pro customer hitting a 409, a 500, or a dropped connection was shown a panel selling Pro -- the exact regression the comment above them describes. Both are now status-only. The upload disclosure is deliberately kept: what is uploaded, the 16 MiB cap, and that it travels over HTTPS without end-to-end encryption. Two tests asserted the defect as intended behaviour and are replaced; the weakened predicate assertion is restored and mutation-tested. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
access_for_workspace merged the refresh answer onto the saved record with dict.update. _declared_entitlement deliberately omits cloud_features when the body carries no feature list, so a downgrade replaced plan while the previous plan's grants survived. A Team -> Pro downgrade therefore reported plan "pro" with "team" still in features, leaving the Team tab unlocked indefinitely. Fixed at the merge site so _declared_entitlement keeps its tested contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
subprocess.run(capture_output=True, timeout=N) does not honour N on Windows: after TimeoutExpired, CPython calls communicate() with no timeout, and the reader threads block until every inherited pipe write handle closes -- including grandchildren. git fetch and git ls-remote always spawn git-remote-https, so the three network calls stayed unbounded. Measured with a grandchild holding the pipe: a 3s budget returned after 30.2s before, and raises at 3.1s now. Network calls that need stdout go through a bounded Popen reader that kills the process tree and re-drains with a cap; the rest stop capturing. Every git call now gets stdin=DEVNULL and GIT_TERMINAL_PROMPT=0 so an expired token cannot park the updater on a credential prompt. Also moves _detect_install() inside main()'s try, so a stalled pip show prints its crafted message instead of a traceback. Three existing tests encoded the belief that timeout= alone was sufficient and are rewritten. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses three review findings on PR #73. The cached entitlement was served without comparing its stored organization_id against the organization the installation is bound to now. Reconnecting or re-pinning to a different organization while reusing the same state directory therefore displayed the PREVIOUS organization's plan and features -- for up to the refresh interval, or indefinitely when refresh is disabled. The session path already rejected this mismatch; the cache now does too, and fails closed so a rejected cache immediately schedules the correcting fetch. hosted_plan_summary() resolved _plan_entitlement() twice, once directly and once via _hosted_plan(), doubling the cloud-session and cache reads -- and the chance of scheduling a background refresh -- on every /api/license and /api/bootstrap. The _hosted_plan override seam that tests rely on is preserved: when a caller rebinds it, its answer still wins. access_for_workspace() was annotated workspace_id: str while the org-scoped entitlement read passes None, so the refresh body carried "workspace_id": null. A control plane that requires a string would 4xx, and the whole compatibility fallback silently returns None, meaning the cache from GET /v1/entitlements/{org} would never be written for older control planes. The key is now omitted rather than sent as null. Also passes a _NoRedirect instance rather than the class in update_check, matching every other caller. Verified behaviour was already identical -- build_opener instantiates a passed class -- so this is consistency only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses two more review findings on PR #73. _run() still reached subprocess.run(timeout=...) for non-captured steps, which kills only the immediate child. pip's resolver or git-remote-https could outlive the budget and keep touching the repository while the caller had already started rollback. Every step now goes through one spawn path with process-tree termination and a bounded post-kill drain, so capture selects whether stdout is piped rather than whether the budget is enforceable at all. git checkout tags/<tag> -- the destructive step -- ran above the rollback try, so a timed-out or non-zero checkout raised straight past it and left an editable install partially switched while reporting only a timeout. It now sits inside the same boundary. A killed checkout can also strand .git/index.lock, which blocks every later git command: the lock is removed only when it was absent before our checkout started and the checkout we killed is the plausible author, and otherwise it is named for the user rather than deleted, so a lock held by another live git process is never touched. The rollback checkout now runs with check=True. A failed restore used to be silent while main() still reported the previous installation restored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses a review finding on PR #73. A lapsed subscription answers 402 on token refresh. That is the control plane's authoritative billing answer, but it was caught by the same broad handler as offline/revoked/invalid and discarded. Because the saved session outranks the entitlement cache, cloud_access_active stayed true and the dashboard kept advertising paid features indefinitely while every hosted operation was denied -- the same two-surfaces-disagree failure this branch set out to fix. record_billing_denial() persists the denial: the access flag and the grants are cleared, the plan name is kept so the UI can still say which plan lapsed. Only 402 does this; an outage must never look like a cancellation. Regression tests both ways -- verified that the denial test fails without the fix (cloud_access_active stayed true with the full feature list) and that a 503 leaves a paying customer's access intact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to eab8a4c, which settled only the session record. Against an older control plane the refresh carries no entitlement fields, so that session stays planless: saved_entitlement() keeps answering {} and _plan_entitlement falls through to the compatibility cache, which went on advertising the paid plan while every cloud operation was denied. The denial has to settle both layers or it settles neither. _deny_entitlement_cache() clears the cached grants and access flag on 402 and keeps the plan name so the UI can still say which plan lapsed. Verified the new test fails without it -- the cache still reported cloud_access_active true. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Addresses four more review findings on PR #73. 401 was drawing the purchase panel. The cloud maps it to "the cloud session expired or was revoked; connect again" -- a credential problem, not a billing one -- so an already-paying customer whose refresh credential lapsed was sold the plan they already own. Only 402 and 501 are billing answers now; 401 falls through to the error branch where the reconnect instruction is what the customer reads. _managed_call flattened every CloudFeatureError into "managed cloud operation failed", so 429, 5xx and a non-consent 409 were indistinguishable and the actionable copy from _public_http_error / _public_session_error never reached anyone. Those messages are fixed local literals -- audited all 19 construction sites in cloud_features, the only interpolation being upgrade_url() and one template whose substituted value is one of two fixed strings -- so they pass through, bounded and control-character-checked, with the old placeholder kept as the fallback. An AST test now pins that invariant so a future f-string or reflected response field fails. Billing denials went unstamped: entitlement_checked_at never advanced and a repeat denial wrote nothing, so the 15-minute interval never suppressed the next attempt and every request rotated the credential again against a control plane that had already answered 402. Every denial is now stamped, including the repeat, in both the session record and the compatibility cache. Also replaces three PEP 604 dict | None annotations in scripts/update.py with Optional[dict]. Contrary to the finding these did not break import -- from __future__ import annotations keeps them as strings, verified on real 3.9.25 -- but typing.get_type_hints() did raise there, and the house style is 3.9-compatible syntax. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two review findings, both on code added earlier in this branch. The organization guard rejected only a mismatch. A fresh bootstrap carries just a refresh credential and a control URL, so the current organization is unknown until the first refresh names it -- and in that window a reused state directory served whatever it still held, i.e. the previous organization's paid plan, indefinitely with the refresh disabled. A cache scoped to an organization now requires a positive match, not merely the absence of a contradiction. record_billing_denial did an unguarded load-modify-save on the shared session file. Another worker rotating the credential concurrently could have its new value overwritten with the stale one; the control plane treats a spent credential as replay and revokes the whole family, so a lapsed subscription would have become a forced reconnect. The read, modify and save now happen inside the same _refresh_lock() that access_for_workspace rotates under. Verified the 402 reaches the handler with its status intact (a real 402 propagates out of access_for_workspace unrewritten) and that the lock is not re-entered on that path -- it has already unwound when the handler runs, which matters because _refresh_lock is not re-entrant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A second knowledge-graph renderer wrapping the already-vendored force-graph, opt-in behind ?graph-engine=next. Classic stays the default and the rollback path; graphRender falls through to it on any failure. Four styles are applied CSP-safely through a data-graph-style attribute rather than inline style. Fixed while reviewing, before this shipped: - XSS. setData stamped name on every node, and force-graph's nodeLabel defaults to that accessor and renders through innerHTML. Node names are entity labels extracted from ingested memories. Classic never set name, so the sink did not exist until this feature added it. Both accessors are now explicit and escaped. - Recursive Tarjan overflowed the stack at roughly 29.6k nodes on a chain component; rewritten iteratively. - The rAF loop never stopped: a hidden canvas kept repainting for the whole session. Added pause/resume driven from selectView and a real destroy(). - Unbounded O(V*E) betweenness on the main thread with Array.shift queues, now lazy, pivot-sampled and budgeted: 60k nodes went 25s to 3s, and 0s in the default view. - prefers-reduced-motion was ignored, against an existing dashboard contract. The CSP drift gate never scanned this asset -- it only ever read index.html, dashboard.css and dashboard.js -- so "the gate passes" was vacuous for any new first-party script. Extended to hold them to the same contract and to check that every /static script index.html references actually exists. All four new rules were negative-tested. The original test asserted formatting strings. Replaced with 24 tests that execute the asset under Node with only a bare window in scope, which is itself the load-order proof, and mutation-tested: removing the escaping fails them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adding force-graph.min.js and engraphis-graph.js to index.html loaded them on every dashboard page, not just the graph view. force-graph applies inline styles at runtime, the production CSP is style-src 'self', so the browser blocked them and logged an error per attempt. Eleven pre-existing e2e tests assert a clean console and failed -- a regression this branch introduced. dashboard.js already had loadForceGraph() lazily fetching the vendor bundle for the Classic renderer, so neither script ever needed to be in index.html. Both tags are removed and engraphis-graph.js gets a matching memoized loader. The naive version of that would silently downgrade a ?graph-engine=next deep link to Classic, because graphRenderEngine's guard cannot tell "not fetched yet" from "unavailable". The engine fetch now starts alongside the vendor bundle -- one round trip, not two -- and graphRender waits on it and re-enters, so only a genuine load failure degrades, and that path warns. Proved by executing the real routing source under Node with a stub DOM: deep link renders the engine, a failing asset falls back and warns. Both are tests, and reverting to defer-without-await fails them. The "referenced scripts exist" gate rule became three, so removing the tags strengthens it rather than hollowing it out: lazy script.src references are existence-checked (their only reference is a JS string literal), neither asset may reappear as a parsed script src in index.html, and each must keep a lazy loader so deferring cannot orphan them. All negative-tested. Local gate: 1429 passed, 0 failed. The CSP fix itself cannot be proven locally -- Playwright needs node_modules, which is not installed here -- so CI is the real verification for the 11 e2e assertions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…nnect `cloud_session.save_bootstrap()` is the only function that writes `~/.engraphis/cloud_session.json`, and it had zero production callers — only tests reached it. Meanwhile `docs/AGENT_CONNECT.md` and `.env.example` told customers to prefer that file. Nothing created it, so a paying customer had no supported way to connect a client and every paid feature was undeliverable. This adds the client half of the device-connect flow that the control plane already serves: - `engraphis/device_connect.py` — token validation, stable client identity, the POST to `/v1/devices/connect`, status-keyed error copy, and the call into `save_bootstrap`. - `scripts/connect.py` — the CLI, following `scripts/init.py` conventions (`main(argv=None) -> int`, argparse, no prompts). - `scripts/entry.py` — an `engraphis` front door that dispatches to the existing `engraphis-<verb>` mains, so the portal's `engraphis connect --token ...` is a runnable command. `engraphis-connect` is installed too. Credential handling: the connect token travels in the request body and nowhere else — never printed, logged, or persisted, and never quoted back in an error. Malformed or truncated tokens are rejected before any network call, so a bad paste costs no rate budget and consumes no token. Network access uses the repo's pinned opener with redirects blocked and an explicit timeout; provider bodies are never reflected into messages. A 401 (expired / already used / invalid, which the service deliberately makes indistinguishable) stays distinct from a 402 lapsed subscription, because the fixes differ. Client ids are minted-once random ULIDs in the owner-only state directory rather than a hardware fingerprint: a MAC- or hostname-derived id is a cross-account identifier the customer never agreed to ship, and it changes under exactly the conditions where stability was the point. The command verifies `cloud_session.configured()` after writing, and says so explicitly when only the compute endpoint is missing rather than half-reporting success. Docs updated: `docs/AGENT_CONNECT.md` and `.env.example` now describe `engraphis connect --token` as the real path to the session file. Tests: 43 new, covering the happy path writing a usable session, 401 printing actionable copy and writing nothing, 402 staying distinguishable, the token never appearing in stdout/stderr or any written file, and short/malformed tokens being rejected before the network. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`/api/license` hardcoded `"is_trial": False` and `"trial": {"used": False}`.
Those were the only assignments in the file, and the client read none of
`status`, `trial_ends_at`, or `trial_duration_seconds` from anywhere, so three
customer-visible states were wrong or unreachable:
* the dashboard's `'TRIAL'` badge branch could never be taken — a trialist saw
the same confident PRO/TEAM badge a subscriber sees;
* `trial.used` never became true, so "Start hosted Pro/Team trial" was offered
to everyone forever, including active subscribers and customers whose trial
was already spent. `start_trial` refuses any organization that already holds
an entitlement, so for every connected customer that button could only ever
return a `TrialAlreadyConsumedError`; and
* `cloud_access_active` was computed, returned, and rendered nowhere. After a
trial expired or a subscription lapsed the client kept `plan="pro"` and
cleared the feature list, so the customer got a confident PRO badge over
rows of locked features with no explanation of what had happened.
The client now reads what the control plane already sends on the calls it
already makes. `cloud_session` persists `status`, `is_trial`,
`trial_consumed`, and `trial_ends_at` from the registration/refresh handshake
alongside the plan fields, under the same wire names so one parser serves the
saved record, the entitlements read, and the compatibility cache. Every field
stays individually optional: an older control plane omits them and the honest
answer is "not a trial, none consumed", never an invented one. A billing
denial keeps the trial facts — a lapse does not un-consume a trial or move its
boundary — and drops the status it just contradicted.
`/api/license` derives one `access_state` from that: active, trial,
trial_expired, lapsed, or inactive. Each is a different thing to tell the
customer and a different thing to ask them to do, and the dashboard now says
which:
* active trial — TRIAL badge, the trial's end date, no trial CTA;
* spent trial — TRIAL ENDED, "your free trial has ended, your memories are
still local and usable, the trial cannot be started again",
subscribe;
* paying — PRO/TEAM badge, no trial CTA (a subscriber who never
trialled would still be refused one);
* lapsed — PRO INACTIVE, why it lapsed when the cloud named a status,
and update billing;
* never — LOCAL CORE, the local engine is complete on its own, and
the trial CTA — the one state it can actually be started in.
`plan_source`/`plan_checked_at` were emitted for support and displayed
nowhere; the panel now shows which rule produced the answer and when the cloud
last confirmed it. No inline styles or handlers were added; both asset gates
and `node --check` pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`connect()` spent the single-use connect token on the POST and only then called `save_bootstrap()`. If the state directory had become unwritable, or `cloud_session.json` had been replaced by a symlink, a hard link or a directory, the write failed after the token was already consumed: the customer was left with a spent token and no session, and had to go back to the portal for a new one. Worse, `atomic_private_text` raises `UnsafeStateFile`, which is an `OSError` and not a `CloudSessionError`, so it escaped `connect()`'s handler entirely and surfaced as a raw traceback rather than actionable copy. `cloud_session.preflight_save()` now runs before the exchange. It lives beside `_session_path()`, `_refresh_lock()` and `_save()` because that module owns the paths and the atomic write it predicts; a private copy of those rules in `device_connect` would drift from the save it is meant to model. It reuses the existing primitives -- `private_file_stat` on the session leaf and on its refresh lock, plus the randomized sibling temp file `atomic_private_text` has to create -- and never opens, creates or replaces the session leaf, so an existing session survives a failed pre-flight untouched and a first connect is not left half written. `_refresh_lock()` picks up the extracted `_refresh_lock_path()` so the lock filename is not duplicated. `device_connect._preflight_session_storage()` translates the failure into a `DeviceConnectError` that names the path to fix and states that the token is still redeemable, and `connect()` reuses the returned path for the summary instead of rebuilding it from `_state_dir()`. Tests: 6 new, all asserting `opener.calls == []` -- the POST is never issued. Session leaf and refresh lock replaced by a directory, a genuinely hard-linked session, an uncreatable state directory (`ENGRAPHIS_STATE_DIR` under a regular file), a read-only mount, and a pre-flight that creates nothing and leaves no probe file behind. All six fail against the pre-fix code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Six findings from PR #74 review. Particles were assigned per relation with no regard for graph size, so the Cyber style with flow enabled created tens of thousands of animated particles on a large workspace. Capped against the existing large-graph signal. Community clustering added every link to the adjacency, so one sparse influences relation merged two unrelated topics into a single component and Community Islands gave them the same colour and force centre. Matched to the classic renderer's semantics. Hover changed hilite/hoverSet without invalidating the canvas, so with reduced motion on, flow off, or the simulation settled, highlight changes were invisible. The redraw is now requested. Show unlinked nodes never reached the engine: graphData() supplied degree-zero nodes while the engine's own filter still dropped them. Leaving the graph view before /graph or a lazy script resolved ran pause against a null renderer, so the pending callback could still create and start one against a hidden pane -- the rAF leak this PR fixed once already, by another route. The CSP gate parsed script tags case-sensitively, so an equivalent HTML spelling browsers load eagerly slipped past both the eager-load rejection and the existence check. Now uses the HTMLParser already in the file, which normalises case. 1441 passed, 0 failed (+12 tests). All asset gates pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# Conflicts: # CHANGELOG.md
…aces Three unresolved review threads on #75, plus one semantic conflict the merge with main could not see. `default_compute_url` now reads the manifest's `compute_plane` and falls back to `DEFAULT_COMPUTE_URL`. The review asked for the constant to be replaced outright, but the shipped `engraphis-commercial/v2` manifest declares only `control_plane`: reading the absent key alone resolves `""`, and `cloud_session.configured()` then rejects the session a production connect just saved, leaving a paying customer "connected" but unusable. Preferring the key removes the drift point the review named -- a manifest-only endpoint change needs no code change -- while the constant stays as the fallback that keeps today's manifest working. `_validated_timeout` refuses a non-finite or non-positive timeout. `argparse`'s `type=float` accepts `nan` and `inf`, which reach the pinned opener's deadline arithmetic and raise `ValueError`/`OverflowError` from inside `urllib`, neither caught by `post_connect`. That broke this module's contract that every failure is a `DeviceConnectError`, so `engraphis connect --timeout nan` printed a traceback instead of the command's error and exit code. Checked at the top of `connect()` so a bad flag is reported as a bad flag rather than masked by whatever the identity or storage pre-flight hits first, and again in `post_connect()` because it is public and owns the socket call. `connect()` now also translates `OSError` from `save_bootstrap`. The pre-flight added in aae7d93 closes the common case, but the state directory can still change between the pre-flight and the write, and `UnsafeStateFile` is an `OSError`, not a `CloudSessionError`, so it escaped as a traceback at the worst moment. The token is spent by then, so the copy says so instead of promising a retry with the same one. The `REGISTRATION` fixture no longer claims the cloud grants `export`. #76 removed that key from the plan->feature tables in both repos because the signed compliance export was never implemented; a fixture asserting the control plane returns it would re-establish exactly the drift that removal cleaned up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…theme Three further findings from PR #74 review. Community IDs were assigned in raw payload order while graphRenderLegend() sorts communities by size and calls the largest "Cluster 1". Node colour indexes the palette by the ID itself, so whenever a smaller component appeared first the legend described one component with another's swatch. Components are now ranked by size before the IDs escape, matching the classic graphComputeCommunities(). Galaxy style held autoPauseRedraw(false) unconditionally, so a settled large graph repainted every node and link every frame forever. The starfield is the only paint force-graph cannot see; the classic path drops it past GPERF.large, so the same 600-node/2400-link signal now skips it and hands the redraw loop back to the vendor. Type colours resolved from hard-coded dark-theme constants, so switching to Light, Midnight, Solarized or Sepia moved the legend and controls while the canvas stayed dark. The engine cannot read CSS custom properties from a canvas, so the dashboard resolves --entity-* and supplies them through a new setThemeColors(), below style palettes and user overrides in priority. 1446 passed, 0 failed (+5 tests). ruff, externalize and manifest gates pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…test that The test raised CloudSessionError(..., status=503, transient=True), but that class takes only status -- transient belongs to CloudFeatureError. So the construction raised TypeError, the caller's broad except Exception swallowed it, and getattr(exc, "status", None) was None rather than 503. The assertion therefore passed while never exercising a transport failure at all, and could not have caught the regression it exists to prevent: loosening the 402 gate so any error clears a paying customer's access. Verified by mutation -- with the gate changed to any-truthy-status the test now fails with "an outage revoked a paying customer"; before this change it passed even with that mutation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both paths fail *after* the control plane has consumed the single-use connect token, so a raw traceback left the customer unable to tell whether to retry or fetch a new token from the portal. `http.client.IncompleteRead` (a truncated chunked reply) subclasses `HTTPException`/`ValueError` -- neither `OSError` nor `URLError` -- so it escaped both of `post_connect`'s handlers. A third clause is added *after* the transport clause, because `RemoteDisconnected` is both a `ConnectionResetError` and a `BadStatusLine` and means the peer never answered: that one keeps the retryable 503 copy, and a test pins the ordering. `save_bootstrap` re-runs `validate_cloud_base_url` on both endpoints, which resolves the host. A resolver that dies mid-connect raises `CloudUrlUnresolved` and an endpoint that starts resolving to a rejected address raises a bare `ValueError`; both are `ValueError`, so neither the `CloudSessionError` nor the `OSError` handler in `connect()` covered them. Translated in `connect()` rather than by dropping the re-validation, which is still wanted for `save_bootstrap`'s other callers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Six findings from the PR #74 review, all cases where the opt-in renderer diverged from the classic path it is meant to replace. - Relation labels: the dashboard's Labels checkbox drives two label layers on the classic path, but the engine configured no linkCanvasObject, so ticking it painted entity names only and a relation could be read only by hovering it. Paint them, with the classic gates (zoom, dense-graph). - zoomToNode: graphFocus() treats a false return as "run the recovery path". The engine answered from raw.nodes, which keeps the coordinates force-graph left on a node, so an entity hidden by a scope filter or by the auto-collapsed cluster view still reported success and the camera moved to nothing. Expand a collapsed view, then verify against the data force-graph is actually holding. - Reseeding: render() called graphData() unconditionally with newly allocated arrays, so Style, Color by, Labels and Flow each reset the d3 alpha and restarted the layout. Reseed only when the visible set really changed, and invalidate the paint when it did not. - Cooldown: nothing overrode force-graph's 15s default simulation window, so a large store kept simulating (and repainting) far past the 1.1s/80 ticks the classic path allows. Mirror its size-dependent bounds. - Dense graphs: curvature and arrowheads stayed on past the classic GPERF.dense threshold, paying a bezier and a filled triangle per relation across thousands of links. - Community colours: the cluster legend swatches are CSS (.graph-cluster-N) encoding the Cyber palette, but the engine's Cyber and Solar palettes were ordered differently from dashboard.js, so on the default style the legend described each of the first clusters with another cluster's colour. Each is covered by a test that fails without the change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Under ?graph-engine=next the Repel/Link/Gravity/Size sliders appeared dead.
dashboard.js::graphSet routes them through api.setSettings, which only asked
render() for a reheat when `mode` was present. applyForces() writes the new
charge / link / forceX-forceY / collide values straight into the simulation
force-graph is already running, and a settled graph sits at alpha~0 — so the
new forces were installed and nothing moved until the user found the Reheat
button. `size` is affected too: it feeds d3.forceCollide.
The classic branch of that same function does the opposite — it treats
`repel|link|gravity|size` as a layout change and reheats unless the user asked
for reduced motion. setSettings now matches that set (plus `mode`, which swaps
the whole force arrangement); render() already carries the reduced-motion
exemption, so it is honoured for free. Appearance-only settings (font,
link width, label density, labels, flow) still must not reheat: restarting the
layout because a label got bigger throws away the arrangement being read.
The regression test drives the API against the force-graph stand-in and counts
d3ReheatSimulation() calls per key. Pre-fix it reported
{repel: 0, link: 0, gravity: 0, size: 0, mode: 1}.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…nnect `cloud_session.save_bootstrap()` is the only function that writes `~/.engraphis/cloud_session.json`, and it had zero production callers — only tests reached it. Meanwhile `docs/AGENT_CONNECT.md` and `.env.example` told customers to prefer that file. Nothing created it, so a paying customer had no supported way to connect a client and every paid feature was undeliverable. This adds the client half of the device-connect flow that the control plane already serves: - `engraphis/device_connect.py` — token validation, stable client identity, the POST to `/v1/devices/connect`, status-keyed error copy, and the call into `save_bootstrap`. - `scripts/connect.py` — the CLI, following `scripts/init.py` conventions (`main(argv=None) -> int`, argparse, no prompts). - `scripts/entry.py` — an `engraphis` front door that dispatches to the existing `engraphis-<verb>` mains, so the portal's `engraphis connect --token ...` is a runnable command. `engraphis-connect` is installed too. Credential handling: the connect token travels in the request body and nowhere else — never printed, logged, or persisted, and never quoted back in an error. Malformed or truncated tokens are rejected before any network call, so a bad paste costs no rate budget and consumes no token. Network access uses the repo's pinned opener with redirects blocked and an explicit timeout; provider bodies are never reflected into messages. A 401 (expired / already used / invalid, which the service deliberately makes indistinguishable) stays distinct from a 402 lapsed subscription, because the fixes differ. Client ids are minted-once random ULIDs in the owner-only state directory rather than a hardware fingerprint: a MAC- or hostname-derived id is a cross-account identifier the customer never agreed to ship, and it changes under exactly the conditions where stability was the point. The command verifies `cloud_session.configured()` after writing, and says so explicitly when only the compute endpoint is missing rather than half-reporting success. Docs updated: `docs/AGENT_CONNECT.md` and `.env.example` now describe `engraphis connect --token` as the real path to the session file. Tests: 43 new, covering the happy path writing a usable session, 401 printing actionable copy and writing nothing, 402 staying distinguishable, the token never appearing in stdout/stderr or any written file, and short/malformed tokens being rejected before the network. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`connect()` spent the single-use connect token on the POST and only then called `save_bootstrap()`. If the state directory had become unwritable, or `cloud_session.json` had been replaced by a symlink, a hard link or a directory, the write failed after the token was already consumed: the customer was left with a spent token and no session, and had to go back to the portal for a new one. Worse, `atomic_private_text` raises `UnsafeStateFile`, which is an `OSError` and not a `CloudSessionError`, so it escaped `connect()`'s handler entirely and surfaced as a raw traceback rather than actionable copy. `cloud_session.preflight_save()` now runs before the exchange. It lives beside `_session_path()`, `_refresh_lock()` and `_save()` because that module owns the paths and the atomic write it predicts; a private copy of those rules in `device_connect` would drift from the save it is meant to model. It reuses the existing primitives -- `private_file_stat` on the session leaf and on its refresh lock, plus the randomized sibling temp file `atomic_private_text` has to create -- and never opens, creates or replaces the session leaf, so an existing session survives a failed pre-flight untouched and a first connect is not left half written. `_refresh_lock()` picks up the extracted `_refresh_lock_path()` so the lock filename is not duplicated. `device_connect._preflight_session_storage()` translates the failure into a `DeviceConnectError` that names the path to fix and states that the token is still redeemable, and `connect()` reuses the returned path for the summary instead of rebuilding it from `_state_dir()`. Tests: 6 new, all asserting `opener.calls == []` -- the POST is never issued. Session leaf and refresh lock replaced by a directory, a genuinely hard-linked session, an uncreatable state directory (`ENGRAPHIS_STATE_DIR` under a regular file), a read-only mount, and a pre-flight that creates nothing and leaves no probe file behind. All six fail against the pre-fix code. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…aces Three unresolved review threads on #75, plus one semantic conflict the merge with main could not see. `default_compute_url` now reads the manifest's `compute_plane` and falls back to `DEFAULT_COMPUTE_URL`. The review asked for the constant to be replaced outright, but the shipped `engraphis-commercial/v2` manifest declares only `control_plane`: reading the absent key alone resolves `""`, and `cloud_session.configured()` then rejects the session a production connect just saved, leaving a paying customer "connected" but unusable. Preferring the key removes the drift point the review named -- a manifest-only endpoint change needs no code change -- while the constant stays as the fallback that keeps today's manifest working. `_validated_timeout` refuses a non-finite or non-positive timeout. `argparse`'s `type=float` accepts `nan` and `inf`, which reach the pinned opener's deadline arithmetic and raise `ValueError`/`OverflowError` from inside `urllib`, neither caught by `post_connect`. That broke this module's contract that every failure is a `DeviceConnectError`, so `engraphis connect --timeout nan` printed a traceback instead of the command's error and exit code. Checked at the top of `connect()` so a bad flag is reported as a bad flag rather than masked by whatever the identity or storage pre-flight hits first, and again in `post_connect()` because it is public and owns the socket call. `connect()` now also translates `OSError` from `save_bootstrap`. The pre-flight added in aae7d93 closes the common case, but the state directory can still change between the pre-flight and the write, and `UnsafeStateFile` is an `OSError`, not a `CloudSessionError`, so it escaped as a traceback at the worst moment. The token is spent by then, so the copy says so instead of promising a retry with the same one. The `REGISTRATION` fixture no longer claims the cloud grants `export`. #76 removed that key from the plan->feature tables in both repos because the signed compliance export was never implemented; a fixture asserting the control plane returns it would re-establish exactly the drift that removal cleaned up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…olidate-engraphis-main-20260727
…ing' into codex/consolidate-engraphis-main-20260727 # Conflicts: # CHANGELOG.md # engraphis/cloud_session.py # engraphis/routes/v2_api.py # engraphis/static/dashboard.js # tests/e2e/commercial.spec.js # tests/test_client_launch.py # tests/test_cloud_session.py # tests/test_dashboard_auth_placement.py # tests/test_hosted_plan_resolution.py # tests/test_licensing_launch.py
…consolidate-engraphis-main-20260727 # Conflicts: # CHANGELOG.md # engraphis/device_connect.py # tests/test_device_connect.py
…nsolidate-engraphis-main-20260727 # Conflicts: # engraphis/dashboard_assets/engraphis-graph.js # engraphis/static/engraphis-graph.js # scripts/externalize_dashboard_assets.py # tests/e2e/graph-engine.spec.js # tests/test_graph_engine_asset.py
… codex/consolidate-engraphis-main-20260727 # Conflicts: # engraphis/hosted_client.py # engraphis/static/dashboard.js
…consolidate-engraphis-main-20260727
…ain-20260727 # Conflicts: # engraphis/cloud_session.py # engraphis/device_connect.py # scripts/connect.py # tests/test_device_connect.py
…aphis-main-20260727 # Conflicts: # engraphis/routes/v2_api.py # engraphis/static/dashboard.js # tests/test_client_launch.py # tests/test_dashboard_auth_placement.py # tests/test_hosted_plan_resolution.py
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Coding-Dev-Tools has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0e86f014f
ℹ️ 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 on lines
+704
to
+706
| resolved_compute = ( | ||
| compute_url if compute_url is not None else default_compute_url(resolved_control) | ||
| ) |
There was a problem hiding this comment.
When --control-url targets a non-shipped control plane and no explicit compute URL is supplied, default_compute_url() returns an empty string, and this path no longer considers the vetted compute_url in the registration response. A successful one-time token redemption therefore saves an empty endpoint and reports success even though cloud_session.configured() subsequently rejects the session and every hosted feature remains unavailable; preserve the response endpoint while continuing to let explicit operator overrides win.
Useful? React with 👍 / 👎.
Comment on lines
+649
to
+652
| except http.client.HTTPException as exc: | ||
| # Other malformed HTTP replies have no useful protocol status, but unlike a malformed | ||
| # status line they do not establish that this request reached the control plane. | ||
| raise CloudSessionError("Engraphis Cloud is temporarily unreachable.") from exc |
There was a problem hiding this comment.
When getresponse() raises another HTTPException, such as UnknownProtocol after receiving an HTTP/2.0 status line, the refresh POST has already been sent and may have consumed the single-use credential. This handler instead returns the default 503, which callers present as transient and invites retrying the unchanged on-disk credential; if the first request rotated it, that replay can revoke the credential family. These post-send protocol failures need the same non-transient reconnect handling as BadStatusLine and LineTooLong.
Useful? React with 👍 / 👎.
| except Exception: # noqa: BLE001 - a denial we cannot persist is still a denial | ||
| pass | ||
| _deny_entitlement_cache() | ||
| return None |
There was a problem hiding this comment.
When an older control plane omits entitlement fields during token refresh but its compatibility entitlement request returns 402, this unconditional return treats the authoritative billing denial like an outage. Because the compatibility cache remains the only plan source in that scenario, /api/license continues advertising its previous cloud_access_active value and paid features indefinitely while hosted operations are denied; handle 402 here by recording the denial and clearing the cached grants, as the token-refresh error path already does.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Consolidates the local release-hardening, launch, client-connect, graph-engine, trial-plan, and version branches into one verified change set.
The merge resolutions retain the current graph renderer, device-connect hardening, truthful local-only export behavior, and release metadata.
Validation:
python -m pytest tests/ -qruff check .python -m eval.harness --dataset eval/datasets/sample.jsonl --k 5python -m eval.harness --dataset eval/datasets/codemem.jsonl --k 5python -m eval.ablationGitHub must run its nine required checks before this PR can merge; the protected branch forbids direct pushes and merge commits.