Skip to content

UN-4270 [FEAT] v1 scaffold, strict TypeScript and import boundary - #2380

Merged
jaseemjaskp merged 16 commits into
mainfrom
UN-4270-v1-scaffold
Nov 2, 2026
Merged

jaseemjaskp merged 16 commits into
mainfrom
UN-4270-v1-scaffold

Conversation

@jaseemjaskp

@jaseemjaskp jaseemjaskp commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

What

  • Adds frontend/src/v1/, an isolated tree for the v1 frontend. It has the folder taxonomy from the spec plus an assets/ folder for static files, a public entry (src/v1/index.ts, which v0 imports as @v1) and a placeholder for the cloud overlay.
  • Adds a strict v1 TypeScript config (src/v1/tsconfig.json).
  • Enforces the import boundary both ways:
    • v1 may import nothing outside src/v1.
    • v0 may import only the @v1 entry.
  • Bans on v1 code:
    • Biome: prop-types, antd, component .css, explicit any.
    • Guard script: the ant- class prefix, PropTypes assignments, component style files outside styles/.
  • Enforces the folder contract, e.g. components/ui/ may import only lib/ and styles/, and pages may not import other pages, app/, test/ (except from test files) or the v1 entry.
  • Adds the typecheck:v1, lint:v1 and test:v1 scripts, and runs typecheck:v1 and lint:v1 in the frontend CI typecheck job. v0 scripts are unchanged.

Why

  • P0-T01 (UN-4270, spec) gives v1 code a place where it cannot touch v0 and v0 cannot reach into it.
  • It blocks the rest of phase P0: T02–T05 and T07–T11.

How

Rule Enforced by
v1 ↛ v0 (../, @/, src/), v0 ↛ v1 except @v1 Biome noRestrictedImports in two overrides in biome.json. This already runs in biome ci src/, pre-commit and editors
prop-types, antd, @ant-design/*, component .css, explicit any Biome, in the src/v1/** override
strict, no JS, unused locals/params, only the @v1 alias src/v1/tsconfig.json. It lives in the folder so editors pick it up; v0 tsconfig.json/jsconfig.json exclude src/v1
TypeScript-only files, style files only in styles/, folder taxonomy scripts/check-v1.mjs --files, run first by typecheck:v1
Folder import contract, PropTypes assignments, ant- class prefix scripts/check-v1.mjs --code, run by lint:v1

Why some rules live in a script rather than Biome:

  • Biome overrides replace rule options instead of merging them, so per-folder overrides would drop the v1 bans. The v1-base-rules-still-apply fixture checks that the shared rules still apply under v1.
  • allowJs: false makes tsc skip a .js file rather than fail on it. That is why the --files check runs before tsc.
  • TypeScript 7 has no JS API, so the script parses imports itself. That stays reliable because v1 specifiers can only take three forms: ./, @v1/, or a package name.
  • The ant- check is temporary. Remove it once the shadcn migration completes.

test-fixtures/v1-boundary/ holds one case per rule. For each case, the runner builds a throwaway frontend tree, overlays the case's files and runs the real package.json scripts. It uses node --test, and test-fixtures/** is excluded from vitest, so the v0 suite never collects it.

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

  • No. v0 code is untouched, and the v0 build output is byte-identical (see Notes on Testing).
  • The new Biome override for v0 only rejects imports from src/v1, and nothing in v0 imports from there today.
  • It also applies to the cloud src/plugins/ tree, which cloud CI lints with this biome.json. That is intended: cloud v0 plugins must not reach into v1 either.

Database Migrations

  • None

Env Config

  • None

Relevant Docs

  • P0-T01 spec · Phase P0
  • frontend/src/v1/README.md: the rules, the folder contract table and the scripts.

Related Issues or PRs

  • UN-4270. Sub-tasks: UN-4516, UN-4517, UN-4518, UN-4519, UN-4520, UN-4521.
  • Story UN-4258 (P0 · Foundation and guardrails).
  • Out of scope here:
    • typecheck:v1, lint:v1 and test:v1 all run in the frontend CI typecheck job; the remaining CI gates are P0-T10 (UN-4363).
    • The v1 unit harness is P0-T08 (UN-4329).

Dependencies Versions

  • None added

Notes on Testing

v0 no-regression, before (main @ 6e14fa7) and after:

Check Before After
vite build: sha256 of every non-sourcemap output file (239 files) 76925c7e… identical
bun run test --run 40 files / 629 passed 40 files / 629 passed
bun run typecheck pass pass
biome ci src/ exit 0, 799 warnings / 127 infos exit 0, 799 / 127

v1:

  • bun run typecheck:v1 and bun run lint:v1 pass on the empty scaffold.
  • bun run test:v1 passes 46/46, in CI as well: one case per rule, generated tests over every folder pair, import form and outside spelling (v1 and v0), plus checks that every case folder is listed and that no v1 or fixture file is git-ignored.

Two other fixes the fixtures turned up:

  • .gitignore: the root Python lib/ rule matched src/v1/lib/, so its files would never have been committed. The PR re-includes it, and a test guards against it happening again.
  • check-json pre-commit hook: the exclude now covers frontend/src/v1/tsconfig.json, which contains comments, matching what the hook already does for frontend/tsconfig.json.

Screenshots

N/A

Checklist

I have read and understood the Contribution Guidelines.

…N-4270)

Adds frontend/src/v1/ as an isolated tree for the v1 frontend (P0-T01):

- Folder taxonomy from the spec, a public entry (src/v1/index.ts, imported
  by v0 as `@v1`) and a cloud overlay placeholder.
- src/v1/tsconfig.json: strict, no JS, unused locals/params are errors,
  `@v1` alias only. v0's tsconfig/jsconfig exclude src/v1.
- Biome overrides for the two-way import boundary and v1 bans (no `../`,
  `@/`, prop-types, antd, component CSS; noExplicitAny).
- scripts/check-v1.mjs for what Biome cannot express: TypeScript-only
  files, the folder taxonomy and import contract, PropTypes, `ant-`.
- typecheck:v1, lint:v1 and test:v1 scripts; v0 scripts are unchanged.
- test-fixtures/v1-boundary/ proves each rule fails and passes as
  specified, run with node --test so the v0 vitest suite never sees it.
- .gitignore re-includes src/v1/lib/ (the Python `lib/` rule matched it).

v0 build output, test counts, typecheck and Biome results are unchanged.
…270)

check-v1.mjs:
- Scan comments with a string/template-aware tokenizer; a `/*` or `//`
  inside a string or line comment no longer hides the imports after it.
- Read template-literal specifiers, import.meta.glob and `import(..., opts)`.
- Flag specifiers that leave src/v1 (`/`, `..`, `@/`, `src/`) itself, not
  only via Biome, which skips template literals.
- Check `@import`/`url()` and `ant-` in styles/ CSS; report symlinks;
  catch built `ant-` names; treat .test/.spec .mts/.cts as test files;
  ignore .DS_Store; drop the unused --root option.

biome.json: also ban `..`, `./../**`, `/**`, `antd/**`, `prop-types/**`.
tsconfig: skipLibCheck off so v1's own .d.ts files are checked.
.gitignore: re-include lib/ at any depth under src/v1.

Fixtures: a generated matrix over every folder-contract pair, every import
form, every outside spelling, a symlink case, and cases for each new rule;
tsc cases assert error codes; output matching excludes the command text and
spawn errors fail loudly. Comment and README corrections.
@jaseemjaskp

Copy link
Copy Markdown
Contributor Author

Self-review (draft)

Four review passes (code, tests, comments, silent failures) were run against 0c5bacb57. Fixes are in 5c55a7c12.

Fixed

  • Comment stripping hid real imports. A /* inside a string ("/projects/*") or inside a // comment blanked everything up to the next */, so contract violations in between passed. check-v1.mjs now uses a tokenizer that knows about strings and template literals.
  • Imports that went unchecked:
    • template-literal specifiers (import(`@v1/...`))
    • import.meta.glob
    • import(x, opts)
    • root-absolute /src/... (this one passed Biome, check-v1 and tsc alike)
    • bare .. and ./../...
  • Banned subpaths: antd/** and prop-types/** are now banned too, not only the bare package names.
  • CSS in styles/: @import / url() targets outside v1 and ant- classes are now checked. Built ant- names (`ant-${x}`) are caught too.
  • Symlinks under v1 are now reported. Before, file symlinks were invisible to the walker.
  • skipLibCheck is off for v1, so hand-written .d.ts files under v1 are type-checked.
  • .gitignore: lib/ is now re-included at any depth under src/v1, not only at the top level.
  • Fixtures:
    • The full folder contract is tested as a generated matrix, every folder pair in both file and folder form. The expected table is written out independently of the script.
    • Every import form has its own case, and so does every "outside v1" spelling.
    • The test-file exemption now has failing cases as well as passing ones.
    • Passing cases now cover CSS in styles/ and a .tsx component.
    • tsc cases assert error codes (TS7006, TS18048, TS6133).
    • Output matching excludes the command text.
    • Spawn errors fail loudly.
    • Check that the new tests bite: run against the previous guard script and biome.json, 9 of them fail; with the fixes, 38/38 pass.
  • Docs and comments: the README now matches the code. The unused --root option is gone. Internal task IDs are replaced with Jira keys.

Not changed, by design or out of scope

  • The :v1 scripts are not in CI yet. That is P0-T10 (UN-4363). Until then CI enforces only the Biome half: the boundary and the banned imports.
  • v0 → v1 through template literals, import.meta.glob, require or CSS @import is not caught. Only Biome watches the v0 side, and it doesn't read those forms. This is low risk: it would take deliberate effort, and Vite is ESM-only.
  • test:v1 keeps --passWithNoTests until the v1 unit harness (UN-4329) adds its first test.
  • pages/* may import app/, test/ and the entry, as the spec's table says ("everything except other pages"). Worth a spec decision if pages should be stricter.

v0 regression (re-run on 5c55a7c12)

  • Build output is identical: 76925c7e….
  • Tests: 40 files, 629 passing.
  • typecheck: pass.
  • biome ci src/: 799 warnings, 127 infos (same as before).

… (UN-4270)

- ci-frontend-lint: the typecheck job also runs `typecheck:v1` and
  `lint:v1`, so the strict v1 tsconfig and the scripts/check-v1.mjs checks
  (folder contract, TypeScript-only files, PropTypes, `ant-`) gate PRs now
  rather than from P0-T10.
- Pages may no longer import app/, test/ or the v1 entry, only the shared
  folders (components, shell, services, stores, hooks, lib, styles,
  plugins). Test files in a page may still import test/. Contract table,
  README and the fixture matrix updated together.
…(UN-4270)

check-v1.mjs: split the comment scanner into small helpers (cognitive
complexity 40 -> under 15), drop the nested ternary and nested template
literal, use String.raw for the import regexes, keep CLI flags in a Set.

Fixture runner:
- Cases and the expected folder contract are parsed text tables instead of
  repeated object literals (the 10% duplication).
- Sorts use an explicit localeCompare comparator.
- The contract matrix is built by a helper.
- Spawns /bin/sh by absolute path and no longer needs `git init` (Biome only
  needs the ignore file). The git check-ignore call keeps a PATH lookup, with
  the reason and a NOSONAR marker.

Fixture cases: re-exports rewritten as small functions and read-only props,
without changing what each case exercises.
@jaseemjaskp
jaseemjaskp marked this pull request as ready for review October 27, 2026 05:22
@greptile-apps

greptile-apps Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

via Greptile

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge, with no new actionable issues found.

Summary

Adds an isolated TypeScript frontend scaffold under src/v1/, with import rules, file checks, fixtures, and CI steps.

  • The v1 frontend has its own source tree and TypeScript setup.
  • The frontend checks keep v0 and v1 imports within their allowed boundaries.
  • Frontend CI runs the v1 checks alongside its existing typecheck.

Reviews (13) · Last reviewed commit: "chore: git-ignore cloud v1 plugins copie..." · Reviewed by Greptile

Comment thread frontend/biome.json Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
…nnot see (UN-4270)

Addresses Greptile review on #2380:
- import.meta.glob([...]): every pattern in the array is checked, not only
  the first, so an allowed first pattern no longer hides a forbidden one.
- check-v1 --code also scans v0 (src/ outside v1, including the cloud
  src/plugins overlay) for template-literal imports, globs, require(),
  @/v1, src/v1, relative paths and CSS @import that reach into v1 other
  than through `@v1`. Biome still covers plain imports.

Fixtures: Greptile's example as a case, a glob array with a forbidden later
pattern, and a generated test of every v0 -> v1 form (denied and allowed).
The v0 scan reports nothing on the current tree.

@muhammad-ali-e muhammad-ali-e left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this from the Lane I side. It looks solid: typecheck:v1, lint:v1 and test:v1 all pass on the branch (40/40), and the folder contract matches the spec. I have one inline comment, on tsconfig.json, and one question:

Static assets. The taxonomy has no folder for static assets, so src/v1/assets/logo.svg fails --files. Lane I's P1-T22 needs SVG icons and provider logos, and v1 can't reach v0's src/assets/. Today components/ui/icons/x.svg passes, imported as ./icons/x.svg?react. Is that the intended place, or should there be an assets/ folder? A line in the README either way would stop each lane picking its own spot. The spec says only that generated icon maps live under v1.


Generated by Claude Code

Comment thread frontend/tsconfig.json Resolved

@muhammad-ali-e muhammad-ali-e left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This follows up my earlier review with a fuller pass, using the team's standardized 19-lens rubric (unstract:review-focused: one cold reader, then specialists for security, testing, and dependencies/build). Nothing here blocks merge. There's no Critical or High finding, and security, cross-document consistency and the v0 no-regression claims all checked out. The inline comments are mostly fixture gaps, where a guard rule can break without any test failing.

Two notes that don't need a line comment:

  • CI doesn't run the fixtures. You've already called that P0-T10 scope. Until P0-T10 lands, the fixtures are the only check on the Biome-override semantics that the "Why some rules live in a script" section depends on.
  • The "Notes on Testing" section says test:v1 passes 22/22. It now runs 40 tests.

Generated by Claude Code

Comment thread frontend/biome.json Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/biome.json Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/package.json Resolved

@athul-rs athul-rs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict — APPROVE WITH NITS

Summary — Critical: 0 · High: 0 · Medium: 5 · Low: 6 · Lenses run: 19/19 (7 assessed N/A: 5, 6, 8, 10, 12, 18, 19)

Reviewed at 81e15b159 against merge-base 6e14fa790 (71 files, +1211/-11). CI is green at this head, including Run v1 typecheck and Run v1 lint.

The Medium findings are fix-soon, not blockers. #2, #3 and #4 are silent until v1 holds code, so they should land (or be ticketed) before the first real v1 code arrives (UN-4271 onward). If one is expected to be hit sooner, the verdict moves to REQUEST CHANGES.

Findings are anchored inline: #1 ci-frontend-lint.yaml, #2 and #3 check-v1.mjs, #4 tsconfig.json, #5 the fixtures, #6 vite.config.js, #7 manifest.ts, #8 README, #9 check-v1.mjs:98, #10 check-v1.mjs:440, #11 biome.json.

Unanchored findings

  • frontend/scripts/check-v1.mjs and frontend/test-fixtures/**/*.mjs (about 900 lines) sit outside both biome ci src/ and the pre-commit hook (files: ^frontend/src/), so the guard tooling is not linted in CI. Run locally at Biome 2.3.13, scripts/ is clean; the test-fixtures diagnostics (1 error, 3 warnings) are the intentional bad cases.

Lens checklist

  • Lens 1: See findings #3, #8 (the "reliable parser" claim in the description does not hold for interpolated or glob specifiers; the README enforcement claim)
  • Lens 2: Clean
  • Lens 3: See findings #2, #3, #10, #11
  • Lens 4: Clean (security scan: no findings at confidence >= 8)
  • Lens 5: N/A — no migrations or schema
  • Lens 6: N/A — no concurrency primitives
  • Lens 7: See findings #4, #6, #7, #11
  • Lens 8: N/A — build and lint tooling, no runtime or external calls
  • Lens 9: Clean (check-v1 scans 24 MB of the real src/ in about 0.2 s)
  • Lens 10: N/A — no runtime paths
  • Lens 11: See findings #1
  • Lens 12: N/A — no prompt, model or agent surface
  • Lens 13: See findings #1, #5
  • Lens 14: Clean (no dependency added, lockfile untouched)
  • Lens 15: Clean (Biome 2.3.13 biome ci src/: 0 errors, 799 warnings and 127 infos, identical to the baseline in the description)
  • Lens 16: See findings #8, #9
  • Lens 17: Clean (merge-result sweep against origin/main found no stale restatement outside this PR's files)
  • Lens 18: N/A — no chart, values, on-prem build or docs surface
  • Lens 19: N/A — no scaling or lifecycle values

Open questions for the author

  • Is bun run test:v1 meant to join the typecheck job in this PR, or is the deferral to UN-4363 firm? (#1)
  • Is the interpolated-import form (import(`./pages/${name}`)) allowed in v1 and in v0's @v1 mount? If so the checker needs to read it; if not it needs to reject it. (#3)
  • Does the cloud manifest.ts overlay constrain the shape that UN-4285 and UN-4286 will define? (#7)

Assumptions made

  • APPROVE WITH NITS assumes the Medium findings can land as follow-ups, given v1 holds no code yet.

Comment thread .github/workflows/ci-frontend-lint.yaml Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/tsconfig.json Resolved
Comment thread frontend/src/v1/plugins/manifest.ts Resolved
Comment thread frontend/src/v1/README.md Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/biome.json Resolved

@vishnuszipstack vishnuszipstack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review with the PR Review Toolkit (code reviewer, silent-failure hunter, type design, test coverage, comment accuracy, simplifier). I left out anything already raised in this PR's threads; where a comment builds on an existing one (#2, #3, #5, #9), it says so. Each [Medium] bypass, and the Biome false positive, was reproduced on a scratch copy of the tree.

Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/biome.json Resolved
Comment thread frontend/src/v1/tsconfig.json Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread .github/workflows/ci-frontend-lint.yaml Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
…ins in the ignore check (UN-4270)

Reported by Athul while building #2394/#2395 on this branch:
- src/v1/tsconfig.json: skipLibCheck back on, as in tsconfig.json. Off, tsc
  also checked third-party typings, and @types/chai 5.2.3 and vitest 3.2.6
  both declare `containSubset`, so any v1 test importing vitest failed
  typecheck:v1. A fixture test now imports vitest to keep this covered.
- The "no v1 file is git-ignored" check skips src/v1/plugins/<id>/: cloud
  plugins copied there are git-ignored on purpose (#2394).
From Ali's, Athul's and Vishnu's reviews on #2380, each reproduced first:

- tsconfig/jsconfig map `@v1/*` too: once v0 imports the entry, tsc follows
  it into src/v1, where cross-folder `@v1/...` imports must resolve
  (would break `bun run typecheck` at UN-4271). Lint keeps v0 on `@v1`.
- check-v1 scanner tracks regex literals, so a `/*` inside one no longer
  hides the rest of the file; CSS blanks only block comments (`//` is URL).
- Imports are read by argument: literals, templates by their fixed prefix,
  every glob pattern, and new URL(..., import.meta.url). In v1 an import the
  guard cannot read fails closed; a computed v1 path is allowed only in
  folders that may import everything (the lazy route registry in app/).
- v0 specifiers are normalized before the check (/src/v1, @/./v1, ...).
- v1 styles may not @import antd packages.
- Biome's v0 rule matches only alias spellings of src/v1, so `uuid/v1` or
  `@/api/v1` no longer fail; relative spellings are check-v1's (in CI).
- CI runs `test:v1` (fixtures) on Node 22; setup-biome pinned to a SHA
  (SonarCloud S7637).

Fixtures assert the exact message and count per form, cover each fix, and
check that comments, JSX and look-alike class names stay quiet. Against the
previous guard, 9 of the new tests fail.
Comment thread frontend/scripts/check-v1.mjs Resolved
Static files (icons, logos, images) get one home, src/v1/assets/, imported
as @v1/assets/... (e.g. logo.svg?react). v1 cannot reach v0's src/assets/.
Raised by Ali on #2380: Lane I's P1-T22 needs SVG icons and provider logos,
and a .svg outside the taxonomy failed --files.

components/ui, components/patterns, shell, pages and styles may import
assets/; assets/ imports nothing but itself (room for a generated icon map).
README, contract table and the fixture matrix updated together;
v1-allowed-imports imports an SVG as a React component under typecheck:v1.
…r (UN-4270)

Greptile P1 on #2380, introduced by 3889a34: the code-only view blanked
string bodies, and the apostrophe in JSX text (`It's`, `{name}'s`) opened a
"string" that ran to the end of the line, hiding a forbidden import after
it.

A quote now starts a string only in expression position (after an operator,
`(`, `,`, `=`, `[`, `{` or a keyword such as return/case/import/from) and only
if it closes on its line, as JS strings must; otherwise it is JSX text. The
regex-literal check shares the same token test.

Fixtures: Greptile's example and the `{name}'s` form each must report the
import; a JSX attribute and quoted JSX text stay quiet. A differential run
over the 409 real v0 files finds no real import hidden by the code-only view.
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved

@athul-rs athul-rs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR review — #2380 (FOLLOWUP round 2, head feb8c5f, previous review at 81e15b1)

Reviewed under unstract v2.1.73 (review-deep, 19-lens rubric) with review-lean round-2 scoping: the agents read only the delta 81e15b159..feb8c5f1d (5 commits, 20 files, +550/-164); prior findings verified by the orchestrator.
Signals: lenses 3, 13, 16, 17 raised (check-v1.mjs scanner rewrite; boundary fixtures; README; biome.json pattern change). Security deep-dive: unavailable — skipped by review-lean, the delta is lint and boundary tooling with no runtime attack surface; the round-1 deep-dive on this PR found nothing.

Verdict — REQUEST CHANGES

Summary — Critical: 0 · High: 1 · Medium: 7 · Low: 5 · Lenses run: 19/19 (delta-scoped; 9 N/A for tooling-only scope). Open = 8 new (#12–#19: 1 High, 5 Medium, 2 Low) + 5 from round 1 not fully resolved (#3, #5 Medium; #7, #9, #10 Low).

Prior context fetched — followup-fetch.sh exited 0 (6 of 6 files). Since-boundary: 81e15b159.

Scope change: NO — the delta stays inside the round-1 surface (check-v1.mjs, fixtures, CI step, tsconfig/biome/vite config, README).

Most round-1 fixes landed and hold. CI is green on the head and runs the fixture suite (46/46). The blocker is new in this round: the scanner's comment/string/regex state can be flipped by ordinary JSX text, after which check-v1 prints ok for the rest of the file (#12). Three rounds of point fixes to the hand-written tokenizer have each opened a new hole of the same class, so the request is the structural fix, or at minimum the end-of-file tripwire, not another patch.

Prior findings status

  • #1 [Medium] test:v1 run by no CI job → RESOLVED — .github/workflows/ci-frontend-lint.yaml:147 runs bun run test:v1; the typecheck job on feb8c5f1d logs # tests 46 # pass 46 # fail 0.
  • #2 [Medium] /* inside a regex literal read as a comment → RESOLVED — regex literals are tracked (check-v1.mjs:236, fixture afterSlashStarInRegex at boundary.fixtures.mjs:346); re-ran const r = /\/*$/; followed by a v0 import on a scratch copy and it is reported. The wider desync class is #12.
  • #3 [Medium] import forms the checker cannot read pass silently → PARTIALLY RESOLVED — a variable or fully computed specifier now fails closed (fixture v1-unanalysable-import; re-ran import(n), reported). Residual: import("./" + x), a template prefix without a trailing slash, and escaped specifiers still pass silently — see #13.
  • #4 [Medium] root tsc fails once v0 imports @v1 → RESOLVED — @v1/* mapped in tsconfig.json:47-48 and jsconfig.json:6-7; fixture v0-tsx-imports-entry; CI typecheck green.
  • #5 [Medium] five guard branches have no fixture → PARTIALLY RESOLVED — items 3 and 4 covered (variant-outline tenant-id, commented-out imports). Items 1 (@ant-design/icons), 2 (a v1 package import such as react) and 5 (@/ under typecheck:v1) still have no fixture; deleting those branches leaves the suite green. Author deferred them; no waiver recorded.
  • #6 [Low] Vite resolves @v1/<anything> → RESOLVED — vite.config.js:188 now says only lint keeps v0 to the entry.
  • #7 [Low] pluginManifest typed readonly [] → NOT RESOLVED — src/v1/plugins/manifest.ts is unchanged; the author defers it to #2394, which replaces the file. Residual failure mode unchanged (no declared contract for the cloud overlay until #2394 lands).
  • #8 [Low] README claims only enforced rules → RESOLVED — src/v1/README.md:4-6 rewritten. The new wording overclaims in a different way; see #18.
  • #9 [Low] six comments misstate the code → PARTIALLY RESOLVED — five of six fixed. Residual: a ../ that stays inside v1 is still reported as "outside src/v1" (check-v1.mjs:375-380); the author keeps the ban by design and defers the message wording.
  • #10 [Low] v0 symlinks skipped silently → NOT RESOLVED — check-v1.mjs:682 still skips a symlink under v0; the author agreed and deferred it. No waiver recorded.
  • #11 [Low] v0 rule flags third-party v1 segments → RESOLVED — biome.json:190-199 is anchored to alias spellings. The narrowing dropped relative spellings from Biome; see #15.

New findings

Findings #12–#19 are posted inline below. Replies on the round-1 threads carry each prior finding's status.

Unanchored findings

None.

Lens checklist

  1. Requirements & intent — Clean (code-reviewer; title follows UN-XXXX [TYPE] title)
  2. Correctness — See findings #19
  3. Error handling — See findings #12, #13, #14, #16
  4. Security — N/A — delta is lint and boundary tooling; security deep-dive skipped (review-lean), round-1 scan found nothing
  5. Data integrity & migrations — N/A — no data or schema
  6. Concurrency — N/A — no concurrency primitives
  7. API compatibility — N/A — no public API; the tool boundary with cloud is under #15
  8. Reliability & resilience — N/A — build tooling, no runtime calls
  9. Performance & cost — Clean (code-reviewer)
  10. Observability — N/A — no runtime paths
  11. Operational safety — Clean (orchestrator: CI step order and exit codes checked; the two new action SHAs match their tags)
  12. LLM/agent — N/A — not touched
  13. Testing — See findings #17
  14. Dependencies & build — Clean (orchestrator: no dependency change)
  15. Code quality — Clean (orchestrator: pinned Biome ran in CI on the head, biome-ci passed and the lint report shows no issues)
  16. Documentation accuracy — See findings #18
  17. Cross-document consistency — See findings #15
  18. On-prem deployment experience — N/A — no chart, on-prem values or on-prem build path touched
  19. Scaling & workload lifecycle — N/A — no workload values

Open questions for the author

  • Will cloud CI run check-v1 --code over src/plugins/, or should Biome keep relative spellings? (#15)
  • Is #7 still expected to close with #2394, and #10 and #5's remaining items tracked somewhere?

Assumptions made

  • {done}/{total} and /* in JSX text are treated as likely in real v1 pages. If v1 were never to hold JSX text, #12 would drop to Medium.

cost: agents=4 wall=12:41 lenses-examined=10 of 19 files=20 lines=714

Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/biome.json Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/src/v1/README.md Resolved
Comment thread frontend/src/v1/tsconfig.json Resolved
…okenizer (UN-4270)

The tokenizer guessed regex vs division and string vs JSX text from the
previous token. JSX text such as `{done}/{total}`, `files/*.ts`, a stray
backtick, a multi-line attribute or `range.from / x` flipped it into a state
that never closed, and check-v1 then passed the rest of the file.

check-v1 now parses with @babel/parser (TypeScript 7 has no JS parser API)
and walks import/export statements, import(), require(), import.meta.glob,
new URL(..., import.meta.url), `typeof import()` and `import x = require()`.
Escapes are decoded and methods named import/require are not imports.

Also:
- v1 rejects concatenated specifiers, template prefixes with no `/`, a
  non-literal new URL and files that do not parse; v0 reports a template or
  glob prefix that could grow into src/v1.
- v1 styles: check @reference/@plugin/@config and the folder contract.
- Biome: relative v0 -> v1 spellings are back in the v0 group.
- A `../` that stays inside v1 is reported as such, not as "outside".
- v0 symlinks into src/v1 are reported; file symlinks are scanned.
- README: name what Biome covers; asset placement is convention.
- Fixtures for every reported repro and the remaining #5 branches; drop the
  unused src/v1/plugins/<id> ignore filter.
Comment thread frontend/scripts/check-v1.mjs Resolved
…s (UN-4270)

`import("@v1/x" as const)` wrapped the specifier in a TSAsExpression, so
v0 treated it as opaque and skipped it. Unwrap as/satisfies/<T>x/x! first.
Also share one helper between the outside and climbs tests.

@vishnuszipstack vishnuszipstack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up review at daa1a18. Every finding below was reproduced on a scratch tree (check-v1: ok on input that Vite bundles across the boundary), and none duplicates an earlier thread.

Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/tsconfig.json Resolved
- import.meta.glob: resolve relative patterns against the `base` option; a
  base that is not a string literal is unanalysable in v1.
- See through TypeScript wrappers on the import markers too
  (`import.meta.url as string`, `(import.meta.glob as any)()`,
  `(require as ...)()`, `new (URL as ...)()`, a glob array `as const`).
- v0: judge a `+` chain or `.concat()` by the string it starts with, as
  Vite globs it, and a conditional by both branches.
- Report a `..` after the first wildcard or `${}`, in v1 and v0.
- Read stylesheets with PostCSS: case-insensitive at-rules, `@import"x"`
  with no space, and `/*` inside strings.
- Root tsconfig includes vite-env.d.ts, so v1 assets, CSS and
  import.meta.env reached through `@v1` typecheck in the v0 lane.
- Parse .d.ts files in declaration mode.
- Split nodes() and checkV0() into helpers (Sonar S3776).
@vishnuszipstack
vishnuszipstack self-requested a review November 2, 2026 07:02

@vishnuszipstack vishnuszipstack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of 73b9ffa. The six findings from my last pass (glob base, TS wrappers on the import markers, v0 concatenation, .. after a wildcard, @import"x", vite-env.d.ts) are fixed. I re-ran each repro and each is now reported. Two new High findings remain, both in forms check-v1 never reads; the inline comments have repros and fixes. Medium items are left out of this pass.

Comment thread frontend/scripts/check-v1.mjs Resolved
Comment thread frontend/scripts/check-v1.mjs Resolved
…file types (UN-4270)

- CSS values are read with postcss-value-parser: every url() and every
  string inside image-set() / -webkit-image-set(), with backslash escapes
  removed as Vite does. @import targets use the same parser; an unreadable
  one is unanalysable.
- v1 holds only code, styles (in styles/), Vite asset types, JSON, Markdown
  and .gitkeep; --files reports any other type. STYLE_EXT and the Biome
  component-style group cover all of Vite's CSS languages.
- --code reads every non-asset file in v1 and v0, parsing an unknown type
  as code (Vite can load any file with ?worker), and checks style files
  wherever they are, so --code alone catches what --files rejects.
The cloud build copies its v1 plugins into src/v1/plugins/<id>/, as it
does v0 plugins into src/plugins/. Ignore them, keeping only the public
placeholder manifest.ts tracked, so cloud code cannot be committed here.

The ignore-check test skips a local copy of those plugins, and a new test
asserts they are ignored while manifest.ts is not.
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Frontend Lint Report (Biome)

✅ All checks passed! No linting or formatting issues found.

@sonarqubecloud

sonarqubecloud Bot commented Nov 2, 2026

Copy link
Copy Markdown

Comment thread .gitignore Resolved
@github-actions

github-actions Bot commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Unstract test results

Per-group results

Status Group Tier Passed Failed Errors Skipped Duration (s)
⚪ e2e-agent-kv e2e 0 0 0 1 0.1
✅ e2e-api-deployment e2e 3 0 0 0 15.9
✅ e2e-coowners e2e 1 0 0 0 1.2
✅ e2e-etl e2e 1 0 0 0 15.9
✅ e2e-login e2e 2 0 0 0 1.1
✅ e2e-prompt-studio e2e 1 0 0 0 10.7
✅ e2e-smoke e2e 2 0 0 0 0.8
✅ e2e-workflow e2e 1 0 0 0 13.8
✅ frontend unit 651 0 0 0 11.9
✅ integration-backend integration 1458 0 0 27 86.5
✅ integration-connectors integration 1 0 0 7 9.7
✅ integration-workers integration 181 0 0 1 56.5
❌ ui e2e 0 1 0 0 0.0
✅ unit-agent-kv-schema unit 143 0 0 0 1.0
✅ unit-backend unit 2107 0 0 1 38.8
✅ unit-connectors unit 82 0 0 0 8.3
✅ unit-core unit 281 0 0 0 2.3
✅ unit-filesystem unit 1 0 0 0 3.0
✅ unit-platform-service unit 18 0 0 0 1.9
✅ unit-rig unit 120 0 0 0 3.0
✅ unit-runner unit 12 0 0 0 2.0
✅ unit-sdk1 unit 878 0 0 0 29.5
✅ unit-tool-sidecar unit 8 0 0 0 1.1
✅ unit-workers unit 1589 0 0 2 107.7
TOTAL 7541 1 0 39 422.5

Critical paths

⚠️ Critical paths not yet covered

  • workflow-execution-fan-out — Multi-file workflow execution fans out to file-processing workers and rejoins. (declared coverage: no groups declared)
✅ Covered critical paths
  • auth-login — covered by e2e-login
  • adapter-register-llm — covered by integration-backend
  • workflow-author — covered by integration-backend
  • co-owner-manage — covered by integration-backend, e2e-coowners
  • workflow-create-execute — covered by e2e-workflow
  • api-deployment-provision — covered by integration-backend
  • api-deployment-auth — covered by integration-backend
  • api-deployment-run — covered by e2e-api-deployment
  • mcp-server-auth — covered by integration-backend
  • mcp-platform-auth — covered by integration-backend
  • platform-key-whoami — covered by integration-backend
  • prompt-studio-author — covered by integration-backend
  • prompt-studio-fetch-response — covered by e2e-prompt-studio
  • connector-register-test — covered by integration-backend
  • pipeline-etl-execute — covered by e2e-etl
  • usage-aggregate-read — covered by integration-backend
  • usage-token-tracking — covered by e2e-api-deployment
  • callback-result-delivery — covered by e2e-api-deployment

@jaseemjaskp
jaseemjaskp merged commit b41c425 into main Nov 2, 2026
19 checks passed
@jaseemjaskp
jaseemjaskp deleted the UN-4270-v1-scaffold branch November 2, 2026 09:09
praveen-formido added a commit that referenced this pull request Nov 2, 2026
…fold has landed

The scaffold PR #2380 was squash-merged, so every scaffold file both sides
touched conflicted. Main's version is taken for the import checker, the
tsconfigs, the CI workflow and the Biome boundary groups; this branch keeps
the styling policy script and its lint:v1 and tokens:v1 entries, the
tokens.css formatter override, the Styling section of the v1 README and the
27 styling fixture rows. Two fixtures main re-added or changed carried an
unscoped :root selector with a raw hex, which the styling policy rejects, so
they reference a token under .v1 instead.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
athul-rs added a commit that referenced this pull request Nov 3, 2026
Keep the plugin loader layout (plugins/index.ts + .gitkeep) in .gitignore
and point the scaffold's ignore and symlink fixtures at it, since this
branch replaced the placeholder plugins/manifest.ts.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
athul-rs added a commit that referenced this pull request Nov 3, 2026
main's copies of the scaffold files equal the scaffold head merged in the
previous commit, so the conflicts keep this branch's side. The placeholder
plugins/manifest.ts that main reintroduces stays removed: plugins/index.ts
replaces it.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants