feat(frontend): merge insight-front into the monorepo - #2223
Merged
Conversation
…nd-global-services-gate fix(sales-dashboard): wire CRM Pipeline Now + gate dashboard for Global Services
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Replace the flat status + free-form error with a typed AuthStatus where "re-auth needed" is a status tag, not a predicate derived from magic strings. Drop isTerminalAuthFailure. Centralize the redirect in OidcManager.requireReauth(), called from the 401 path and the silent-renew failure, so the 401 check stays at the fetch boundary and the auth layer never references HTTP status. The gate is now pure presentation. Add a reauth_failed state with a retry so a redirect that fails to start can no longer pin the app behind a permanent overlay. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
From a full-flow review of the auth/reauth path: - safeReturnUrl: resolve against our origin and echo back only a same-origin path+query+hash. The prefix checks let backslash network paths (/\host, folded to //host by browsers) through as open redirects. - signIn: never persist /callback as the return target — a fresh visit has no code and loops back into the failure screen. - fetchWithAuth: only fall back to a dev/impersonation bearer when OIDC is disabled, so a URL override can't mint an unsigned alg:none bearer while a real OIDC token is briefly null mid-renew. - root beforeLoad: route first-load through requireReauth so a redirect that can't start lands in reauth_failed (retry UI) instead of throwing an unhandled rejection out of beforeLoad. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Second-pass review follow-ups: - fetchWithAuth: mint a dev bearer only when status is "disabled" AND reason is "dev_bypass". An unconfigured prod deploy (missing_oidc_config) now fails closed instead of letting a URL override forge a bearer. - callback retry: catch signIn() rejection so a redirect that can't start stays contained on the error screen instead of surfacing as an unhandled promise rejection. - types: note that reauth_required is also set by doInit on first load, where beforeLoad (not the component tree) performs the redirect. - tests: cover the status/reason-gated dev-bearer branch and the renewal escalation path (access-token expiry -> renewing, silent-renew failure -> reauth_required) by exercising the captured oidc-client handlers. Skipped the addUserUnloaded -> renewing suggestion: with monitorSession off it only fires via signOut's removeUser, which immediately resets and redirects; a terminal/trigger state there would either be overwritten or race the signout redirect. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Hard-fail PRs whose changed lines fall below 80% coverage, while keeping global coverage a non-failing warning. Single-component repo, so no per-component routing. - scripts/ci/diff-coverage.sh: single source of truth wrapping diff-cover against the Cobertura report; runs identically locally (pnpm test:coverage:diff) and in CI. - ci.yml: coverage-gate job (PR-only, needs: test) reuses the coverage artifact the test job already uploads, installs diff-cover, and fails the PR below threshold. Output goes to the job summary (no PR comment, so it works on fork PRs with a read-only token). The global irongut summary is unchanged (fail_below_min: false). Coverage is logic-line based on this Vitest 4 / v8 stack; bare JSX markup lines are not counted (an istanbul trial tracked the identical line set), so pure-markup diffs have no coverable lines and pass. Documented in the script. Follow-up: make coverage-gate a required status check in branch protection. Signed-off-by: Konstantin Tursunov <konstantin.tursunov@constructor.tech>
Replace the third-party irongut/CodeCoverageSummary action with a self-contained scripts/ci/coverage-summary.py that renders the global coverage markdown (overall line/branch + per-area breakdown, 60/80 thresholds) from the Cobertura report — mirroring constructorfabric/insight, which self-generates its coverage report instead of using an external action. The summary stays a non-failing warning written to the job summary; the new-code diff gate remains the only blocking check. Adds pnpm test:coverage:summary for local parity. Signed-off-by: Konstantin Tursunov <konstantin.tursunov@constructor.tech>
Mirror constructorfabric/insight's topology: the test job is now a pure producer (runs coverage, uploads the Cobertura artifact), and a single `coverage` job consumes it to both render the global coverage summary (non-failing warning, on push and PR) and enforce the new-code diff gate (pull requests only). Replaces the separate coverage-gate job. Signed-off-by: Konstantin Tursunov <konstantin.tursunov@constructor.tech>
diff-cover 10.x deprecated --markdown-report in favor of --format markdown:<file>; switch to it. Behavior-preserving: console report to stdout + markdown to the job summary, non-zero exit on failure. Signed-off-by: Konstantin Tursunov <konstantin.tursunov@constructor.tech>
Replace the diff-coverage.sh wrapper with diff-coverage.py, which runs diff-cover (--json-report, --fail-under 0) and renders the new-code result the way constructorfabric/insight's coverage.py does: a per-file table (New lines / Coverage / Min / Result with check/cross icons) plus the raw diff-cover output in a <details>, written to the job summary. The script decides PASS/FAIL itself (overall new-line coverage >= 80%) and exits non-zero on fail. test:coverage:diff updated accordingly. Signed-off-by: Konstantin Tursunov <konstantin.tursunov@constructor.tech>
Remove the test:coverage:diff / test:coverage:summary npm scripts that shelled out to python3. pnpm is now self-contained (Node/vitest only): devs run 'pnpm test:coverage' and open coverage/index.html. The Python coverage scripts (coverage-summary.py, diff-coverage.py) are invoked exclusively by ci.yml to process the coverage artifact pnpm produces; headers say so. Signed-off-by: Konstantin Tursunov <konstantin.tursunov@constructor.tech>
Renames the gate's name across the workflow, scripts, and report headers (e.g. report heading is now '## Code coverage gate'). Mechanism descriptions still note it measures coverage of changed lines, which is what diff-cover does. Signed-off-by: Konstantin Tursunov <konstantin.tursunov@constructor.tech>
…88f7b fix(auth): re-auth expired session at runtime
ci: add code coverage gate (diff-cover, 80% diff threshold)
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
The view fetches its own data, so stories drive it through MSW; the handler aggregates a fact cube, so group-by and filter clicks change the served series instead of replaying one payload. Export tests assert the produced CSV text and workbook cells. Tailwind now runs in the storybook Vitest project — without utilities every element collapses to content size and recharts never paints. Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
…tories test(metric-views): cover timeseries view states in Storybook
Switching the group-by dimension refires the collection query. The caption follows local state, so asserting it proved nothing and the legend query ran before the response landed — green locally, red on slower CI. Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
…-groupby-race test(metric-views): await the re-grouped series in the story
…orcement ci(security): enforce — block on secrets, CRITICAL, and new SAST findings
Seven `pnpm.overrides` entries plus one direct bump. Every range is bounded to
the major already installed, so nothing crosses a major boundary:
undici 7.25.0 -> 7.29.0 TLS bypass and cross-origin routing in
the SOCKS5 ProxyAgent; WebSocket DoS
fast-uri 3.1.2 -> 3.1.4 host confusion via backslash authority
and failed IDN canonicalization
postcss 8.5.14 -> 8.5.23 sourceMappingURL path traversal
js-yaml 4.1.1 -> 4.3.0 quadratic CPU on merge-key chains
hono 4.12.18 -> 4.12.32 CORS reflects any Origin with credentials
brace-expansion 5.0.6 -> 5.0.8 unbounded expansion, OOM crash
vite (direct) 8.0.11 -> 8.0.16 server.fs.deny bypass on Windows paths
Left alone, deliberately:
brace-expansion 1.x and 2.x. Their fixes, 1.1.17 and 2.1.3, were published on
29 and 28 July; pnpm-workspace.yaml sets minimumReleaseAge to seven days and
pnpm refuses them with ERR_PNPM_NO_MATURE_MATCHING_VERSION. Waiting is the
point of that setting.
uuid 8.3.2. Fixed only from 11.1.1, three majors up, inside a transitive
dependency of exceljs.
Local scan over the lock with dev dependencies included: 19 -> 7.
Signed-off-by: Gregory Gogin <grigoriy.gogin@constructor.tech>
Set up code-ranker reports
…-images-0e8fc498de build(deps): Bump node from 24-bookworm-slim to 25-bookworm-slim in the base-images group
fix(deps): clear the fixable HIGH advisories in the lock (#265)
Wires src/frontend into the existing pipelines instead of adding parallel ones, ahead of the frontend sources landing under that path. - ci.yml gains a `js` producer (lint, typecheck, both vitest projects under merged coverage) feeding the existing coverage-gate, so the frontend inherits the required check rather than needing a new one. Its Cobertura source root is rewritten to a repo-relative path: the producer runs in a container whose workspace differs from the gate job's, and neither coverage.py nor diff-cover can resolve an absolute root across that boundary. - components.py registers the component with an `overall_min` below the default; the new-code gate still applies in full. - build-images.yml builds and publishes ghcr.io/constructorfabric/insight-front per-arch with a merged manifest and provenance, and publish-chart bumps the subchart appVersion directly. The cross-repo `frontend_tag` dispatch input is removed. - merge-frontend fails loudly when a build leg fails, so a half-failed matrix cannot pin an appVersion to a manifest that was never assembled. - Compose reads ./src/frontend; INSIGHT_FRONT_PATH and the wizard's clone option are gone. Refs #1818 Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
The imported tree carried its own agent tooling, workflows and licence files. History keeps all of it; the working tree keeps only what the monorepo does not already provide. - Agent tooling (cypilot, .agents, .claude, .codex, .windsurf) gives way to .cf-studio. .claude/skills/shadcn was a symlink into .agents, so both had to go together. - .github/: workflows are superseded by ci.yml and build-images.yml; CODEOWNERS and copilot-instructions.md are only read at the repository root, so they were inert at this depth. The npm dependabot entry is ported to the root config, which had no npm ecosystem. - scripts/ci/ reimplemented scripts/ci/coverage.py. - LICENSE, NOTICE, SECURITY.md, docker-compose.yml and .semgrepignore duplicate their repository-root counterparts. .gitignore keeps .env.example tracked: the root `.env.*` rule would otherwise ignore it, and the root `!.env.*.example` negation does not match that name. Refs #1818 Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Running the suite shows the frontend clears the repository-wide 80% line floor with margin, so it needs no exemption: 2382/2498 lines with both vitest projects merged, which is what test:coverage:ci collects. Drops the per-component `overall_min` override introduced alongside the frontend component, and the machinery that read it — with no consumer it was speculative, and a lowered floor would have masked the browser project silently dropping out of the merged report. Also groups the `changes` job's step-output redirects, which shellcheck flagged (SC2129) once a third one was added. Refs #1818 Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
|
Important Review skippedToo many files! This PR contains 311 files, which is 211 over the limit of 100. To get a review, reduce the PR to 100 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (311)
You can disable this status message by setting the 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 |
ktursunov
approved these changes
Aug 5, 2026
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The job runs in a container, where the step shell is `sh`; dash rejects `set -o pipefail` and the step exited 2 after a green test run. Nothing in the script relies on it — the one pipe is tested through `if !`. Refs #1818 Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
node:25 ships neither corepack nor pnpm, so `corepack enable` exits 127 and the image build fails. Corepack is not installed to restore it: its package claims the `yarn`/`yarnpkg` bins the image already holds, so it would need `--force` to clobber them. pnpm is installed directly at the version `packageManager` declares, keeping package.json the single source of truth. Refs #1818 Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
Cut restatement and over-long rationale down to the parts that are not derivable from the code they sit above. Refs #1818 Signed-off-by: Aleksandr Barkhatov <pm@aleks.bar>
| @@ -0,0 +1,46 @@ | |||
| FROM node:25-bookworm-slim AS builder | |||
aleksdotbar
enabled auto-merge
August 5, 2026 07:33
cyberantonz
approved these changes
Aug 5, 2026
mitasovr
pushed a commit
to mitasovr/insight
that referenced
this pull request
Aug 5, 2026
…ctorfabric#1977) Port of the constructorfabric#1977 fix into the monorepo FE (`src/frontend`, merged from insight-front by constructorfabric#2223); supersedes insight-front#268. The shared metric formatters typed `v: number` with no internal null guard, so `Math.round(null) === 0` meant a single forgetful caller could silently render a fabricated 0 for missing data. Widen `formatMetricNumber`/`formatMetricValue` to `number | null | undefined` and return `NO_METRIC_VALUE` ("—") for null/undefined/non-finite — a real 0 still formats as 0. The "no data, not zero" guarantee is now structural rather than left to each caller (defense in depth; existing guarded call sites are unaffected). Verified: `pnpm exec vitest run src/lib/format.test.ts` 6/6 pass; `pnpm typecheck` clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Anton Zelenov <antonz@constructor.tech>
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
8 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.
Frontend source moves here from
constructorfabric/insight-front. History preserved —filter-repo --to-subdirectory-filter src/frontend, sourcea7d4ed01, 473 commits.Do not squash. Squash flattens the imported history into one commit. The queue is set to
MERGE— leave it.Merge after constructorfabric/insight-front#267. That retires the old image build; until it lands both repos publish
insight-front:latestfrom divergent trees, andlatestis the default compose mode.What lands
src/frontend/— sources. Agent tooling, workflows and duplicate licence files dropped from the tree; history keeps them.ci.yml—jsproducer: lint, typecheck, both vitest projects, merged coverage into the existingcoverage-gate. 2382/2498 lines, gated at the standard 80%. Runs in the Playwright image (browser story tests); tag and digest both pinned.build-images.yml— per-arch frontend build, multi-arch manifest, provenance.publish-chartbumps the subchart appVersion directly; the cross-repofrontend_tagdispatch input is removed.merge-frontendfails loudly on a half-failed matrix so a nonexistent tag can never reach a published chart../src/frontend.INSIGHT_FRONT_PATHand the wizard clone option are gone.Image name and tag format unchanged, so charts, values files and environment overlays need no edit.
Not in scope
Issue #1818 Phase 4: transfer open issues, archive insight-front, add the frontend jobs to required checks.
code-ranker.ymlis dropped with no monorepo equivalent — port it separately if wanted.Refs #1818