fix(observability): downgrade tenant build errors to tagged warning-level captures - #3723
Conversation
…and stop reporting 4xx errors A hosted veryfront.config that the declarative evaluator deterministically rejects (e.g. forbidden-capability: unsupported-call) was re-read, re-hashed and re-sent to the evaluator worker on every request, because only successful evaluations were cached. Each per-request rethrow at the renderer.request boundary also produced a Sentry event even though CONFIG_PARSE_ERROR is a status-400 tenant-content failure. - Add a negative cache next to the positive hosted config cache, keyed by the same source-digest/policy/fingerprint cache key so a corrected config or a new release self-invalidates the entry. Worker-phase and retryable failures are never cached. - Treat client-class (4xx) VeryfrontErrors as expected in captureApplicationError so tenant-content failures no longer flood the error tracker; they remain logged at their throw sites. Fixes VERYFRONT-SERVER-E
…evel captures
Every SSR render failure was captured per request at level=error through
captureApplicationError (ssr.service.ts boundary ssr.render), even when the
error object already discriminated a tenant-content build failure. Tenant
mistakes (a page that does not compile, MDX that does not parse) therefore
surfaced as our own error-level Sentry issues and drove sustained noise.
Classify tenant build errors centrally in captureApplicationError using the
discriminators the error already carries: the module loader's build-failure
tag, toError(createError({ type: "build" })) structured data, VeryfrontError
BUILD-category registry errors, and the render pipeline's buildFailure error
context. Classified captures gain errorClass="tenant-build" and
level="warning" on the shared ApplicationErrorContext; the Sentry policy maps
these to a veryfront.error_class tag and scope.setLevel("warning"). The
events are still captured, so tenant failures remain visible for escalation
analysis, while genuine framework errors keep their error-level capture.
Builds on the client-class (4xx) suppression from fix/sentry-veryfront-server-e.
Refs VERYFRONT-SERVER-2 VERYFRONT-SERVER-3 VERYFRONT-SERVER-S VERYFRONT-SERVER-Q VERYFRONT-SERVER-R
…laming esm.sh for foreign HTML responses An MDX page's "@/" alias import that escaped the loader's alias rewrite fell through the ESM specifier resolver: with a project import map mapping the "@/" prefix it was resolved as a relative URL against the page's own public origin (https://<site>/@/components/...), whose HTML fallback then failed the HTTP module cache with an error that wrongly blamed esm.sh. - resolveSpecifier now pins "@/" specifiers to the project-module transport (/_vf_modules/<path>.js), matching the MDX loader's alias rewrite, instead of routing them to esm.sh or the page origin. - The HTML-response diagnostic only mentions esm.sh for esm.sh hosts; other origins get an unresolved-import explanation, with an explicit hint when the path is an /@/ alias form. Fixes VERYFRONT-SERVER-G
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds a ChangesBuild errors and compiler handling
Tenant failure and module loading
Classified application error reporting
Estimated code review effort: 5 (Critical) | ~120 minutes Merge Risk: 🟡 Moderate · up to The PR keeps tenant build failures captured while downgrading explicitly classified tenant errors to warnings, but the current head still has compile-blocking test duplicates and a metadata write path that can turn an otherwise successful render into a build failure when that write fails; merge should wait for these issues to be fixed. Sequence Diagram(s)sequenceDiagram
participant Compiler
participant ModuleLoader
participant RenderingPipeline
participant ApplicationErrorReporter
participant Sentry
Compiler->>RenderingPipeline: classify source failure
ModuleLoader->>RenderingPipeline: classify tenant import failure
RenderingPipeline->>ApplicationErrorReporter: capture error context
ApplicationErrorReporter->>Sentry: apply error-class tag and severity
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
The tenant build classifier was using the broad BUILD category as a downgrade signal, which also caught framework-side asset optimization and source map failures. The classifier now keeps the existing explicit tenant build discriminators and limits direct BUILD registry matching to tenant-facing slugs. Constraint: PR #3723 must keep tenant content compile failures captured as warning-level tenant-build events while genuine framework errors remain error-level. Rejected: Treat every BUILD-category VeryfrontError as tenant content | asset optimization and source map errors can be framework faults. Confidence: high Scope-risk: narrow Directive: Do not broaden tenant-build classification by category without explicit framework-error regression coverage. Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all src/observability/application-errors.test.ts Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all extensions/ext-observability-sentry/src/policy.test.ts Tested: deno fmt --check src/observability/application-errors.ts src/observability/application-errors.test.ts src/observability/application-error-contract.ts extensions/ext-observability-sentry/src/policy.ts extensions/ext-observability-sentry/src/policy.test.ts Tested: deno lint src/observability/application-errors.ts src/observability/application-errors.test.ts src/observability/application-error-contract.ts extensions/ext-observability-sentry/src/policy.ts extensions/ext-observability-sentry/src/policy.test.ts Tested: deno check src/observability/application-errors.ts src/observability/application-errors.test.ts src/observability/application-error-contract.ts extensions/ext-observability-sentry/src/policy.ts extensions/ext-observability-sentry/src/policy.test.ts Not-tested: full repository test suite
Escaped @/ imports in the HTTP module resolver now reuse the existing import-rewriter extension normalizer, so explicit source extensions land on the same /_vf_modules/*.js shape as the unified alias strategy. The HTML diagnostic copy also avoids punctuation that violates the public-copy rules. Constraint: Root AGENTS.md forbids em dashes in public copy and asks for small reversible diffs. Rejected: Add a new alias helper module | existing normalizeExtension already removes the divergent source-extension behavior without a broad refactor. Confidence: high Scope-risk: narrow Directive: Keep escaped @/ resolver output aligned with AliasStrategy SSR/moduleServerUrl extension normalization. Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all src/transforms/esm/specifier-resolver.test.ts src/transforms/esm/http-cache-helpers.test.ts Tested: deno fmt --check src/transforms/esm/http-cache-helpers.ts src/transforms/esm/specifier-resolver.ts src/transforms/esm/specifier-resolver.test.ts Tested: deno lint src/transforms/esm/http-cache-helpers.ts src/transforms/esm/specifier-resolver.ts src/transforms/esm/specifier-resolver.test.ts Tested: deno check --config deno.json src/transforms/esm/specifier-resolver.ts src/transforms/esm/specifier-resolver.test.ts src/transforms/esm/http-cache-helpers.ts Tested: git diff --check && ! rg -n '—|–' src/transforms/esm/http-cache-helpers.ts src/transforms/esm/specifier-resolver.ts src/transforms/esm/specifier-resolver.test.ts Not-tested: Full repository test suite.
The application error capture change shifted exported observability source anchors, and CI checks generated API reference files with the pinned Deno 2.7.7 toolchain. Regenerating only the stale observability reference keeps the stacked PR narrow while clearing the failing docs check. Constraint: PR #3723 is stacked on this branch, so the fix must avoid broad generated churn. Rejected: Commit docs generated by local Deno 2.7.12 | it rewrote anchors across 42 files and did not match CI's pinned generator output. Confidence: high Scope-risk: narrow Tested: PATH=/tmp/deno-2.7.7-aarch64-apple-darwin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/opt/pmk/env/global/bin:/Library/Apple/usr/bin:/Applications/VMware Fusion.app/Contents/Public:/opt/homebrew/lib/node_modules/@openai/codex/node_modules/@openai/codex-darwin-arm64/vendor/aarch64-apple-darwin/codex-path:/Users/kojiwakayama/.codex/tmp/arg0/codex-arg0xChlGW:/private/tmp/claude-501/-Users-kojiwakayama-Documents-CODE-veryfront-platform-veryfront-code/9f9d23dd-dfdc-4463-9a99-bd5a663046c9/scratchpad/idx09/deno277/bin:/private/tmp/claude-501/-Users-kojiwakayama-Documents-CODE-veryfront-platform-veryfront-code/9f9d23dd-dfdc-4463-9a99-bd5a663046c9/scratchpad/idx13/deno277/bin:/private/tmp/claude-501/-Users-kojiwakayama-Documents-CODE-veryfront-platform-veryfront-code/9f9d23dd-dfdc-4463-9a99-bd5a663046c9/scratchpad/idx26-deno277/bin:/Users/kojiwakayama/Documents/CODE/agent-lab/.venv/bin:/Users/kojiwakayama/.veryfront/bin:/Users/kojiwakayama/.opencode/bin:/Users/kojiwakayama/.local/bin:/Users/kojiwakayama/.antigravity/antigravity/bin:/opt/homebrew/share/google-cloud-sdk/bin:/opt/homebrew/opt/openjdk/bin:/Users/kojiwakayama/.bun/bin:/Users/kojiwakayama/.krew/bin:/Applications/Visual Studio Code.app/Contents/Resources/app/bin:/Users/kojiwakayama/.nvm/versions/node/v24.18.0/bin:/Users/kojiwakayama/.cargo/bin:/Applications/Docker.app/Contents/Resources/bin/:/Users/kojiwakayama/.lmstudio/bin:/Applications/Warp.app/Contents/Resources/bin deno task lint:ci Tested: PATH=/tmp/deno-2.7.7-aarch64-apple-darwin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/opt/pmk/env/global/bin:/Library/Apple/usr/bin:/Applications/VMware Fusion.app/Contents/Public:/opt/homebrew/lib/node_modules/@openai/codex/node_modules/@openai/codex-darwin-arm64/vendor/aarch64-apple-darwin/codex-path:/Users/kojiwakayama/.codex/tmp/arg0/codex-arg0xChlGW:/private/tmp/claude-501/-Users-kojiwakayama-Documents-CODE-veryfront-platform-veryfront-code/9f9d23dd-dfdc-4463-9a99-bd5a663046c9/scratchpad/idx09/deno277/bin:/private/tmp/claude-501/-Users-kojiwakayama-Documents-CODE-veryfront-platform-veryfront-code/9f9d23dd-dfdc-4463-9a99-bd5a663046c9/scratchpad/idx13/deno277/bin:/private/tmp/claude-501/-Users-kojiwakayama-Documents-CODE-veryfront-platform-veryfront-code/9f9d23dd-dfdc-4463-9a99-bd5a663046c9/scratchpad/idx26-deno277/bin:/Users/kojiwakayama/Documents/CODE/agent-lab/.venv/bin:/Users/kojiwakayama/.veryfront/bin:/Users/kojiwakayama/.opencode/bin:/Users/kojiwakayama/.local/bin:/Users/kojiwakayama/.antigravity/antigravity/bin:/opt/homebrew/share/google-cloud-sdk/bin:/opt/homebrew/opt/openjdk/bin:/Users/kojiwakayama/.bun/bin:/Users/kojiwakayama/.krew/bin:/Applications/Visual Studio Code.app/Contents/Resources/app/bin:/Users/kojiwakayama/.nvm/versions/node/v24.18.0/bin:/Users/kojiwakayama/.cargo/bin:/Applications/Docker.app/Contents/Resources/bin/:/Users/kojiwakayama/.lmstudio/bin:/Applications/Warp.app/Contents/Resources/bin deno test --preload=src/testing/preload.ts --no-check --allow-all --unstable-worker-options --unstable-net src/config/loader.test.ts src/observability/application-errors.test.ts Tested: PATH=/tmp/deno-2.7.7-aarch64-apple-darwin:/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/System/Cryptexes/App/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/local/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/bin:/var/run/com.apple.security.cryptexd/codex.system/bootstrap/usr/appleinternal/bin:/opt/pmk/env/global/bin:/Library/Apple/usr/bin:/Applications/VMware Fusion.app/Contents/Public:/opt/homebrew/lib/node_modules/@openai/codex/node_modules/@openai/codex-darwin-arm64/vendor/aarch64-apple-darwin/codex-path:/Users/kojiwakayama/.codex/tmp/arg0/codex-arg0xChlGW:/private/tmp/claude-501/-Users-kojiwakayama-Documents-CODE-veryfront-platform-veryfront-code/9f9d23dd-dfdc-4463-9a99-bd5a663046c9/scratchpad/idx09/deno277/bin:/private/tmp/claude-501/-Users-kojiwakayama-Documents-CODE-veryfront-platform-veryfront-code/9f9d23dd-dfdc-4463-9a99-bd5a663046c9/scratchpad/idx13/deno277/bin:/private/tmp/claude-501/-Users-kojiwakayama-Documents-CODE-veryfront-platform-veryfront-code/9f9d23dd-dfdc-4463-9a99-bd5a663046c9/scratchpad/idx26-deno277/bin:/Users/kojiwakayama/Documents/CODE/agent-lab/.venv/bin:/Users/kojiwakayama/.veryfront/bin:/Users/kojiwakayama/.opencode/bin:/Users/kojiwakayama/.local/bin:/Users/kojiwakayama/.antigravity/antigravity/bin:/opt/homebrew/share/google-cloud-sdk/bin:/opt/homebrew/opt/openjdk/bin:/Users/kojiwakayama/.bun/bin:/Users/kojiwakayama/.krew/bin:/Applications/Visual Studio Code.app/Contents/Resources/app/bin:/Users/kojiwakayama/.nvm/versions/node/v24.18.0/bin:/Users/kojiwakayama/.cargo/bin:/Applications/Docker.app/Contents/Resources/bin/:/Users/kojiwakayama/.lmstudio/bin:/Applications/Warp.app/Contents/Resources/bin deno fmt --check docs/api-reference/veryfront/observability.md src/config/loader.ts src/config/loader.test.ts src/observability/application-errors.ts src/observability/application-errors.test.ts Tested: git diff --check
The tenant capture branch is stacked on fix/sentry-veryfront-server-e, and the base advanced after the tenant-build classifier fix landed. Merge the updated base non-destructively so the PR tests and merge base reflect the current stack without rewriting the branch. Constraint: Do not rebase or force-push the stacked PR branch. Rejected: Rebase onto the updated base | would rewrite the PR branch history. Confidence: high Scope-risk: narrow Directive: Keep #3723 stacked on #3719 until the lead asks to unstack or merge. Tested: pending post-merge verification
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d4d1a5c31
ℹ️ 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".
Generic BUILD_FAILED and BUNDLE_ERROR errors are used by framework cache and bundle infrastructure paths, so slug-only tenant classification was still too broad. The classifier now relies on explicit tenant discriminators or tenant-facing registry slugs that do not also identify framework infrastructure failures. Constraint: CodeRabbit PRRT_kwDOQaPiP86ZYi4U identified generic build and bundle slugs as framework-owned in some throw paths. Constraint: The CI-pinned API reference generator requires both observability and extensions source anchors to be current. Rejected: Keep build-failed and bundle-error in the slug allowlist | misclassifies framework cache and bundle infrastructure errors as tenant-build warnings. Rejected: Commit only observability.md docs output | the pinned docs:api-reference:check still reported extensions.md stale. Confidence: high Scope-risk: narrow Directive: Do not classify generic BUILD_FAILED or BUNDLE_ERROR as tenant-owned without an explicit tenant discriminator at the capture seam. Tested: Red regression in src/observability/application-errors.test.ts failed before implementation for framework BUILD_FAILED false positive. Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all src/observability/application-errors.test.ts Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all extensions/ext-observability-sentry/src/policy.test.ts Tested: PATH=/tmp/deno-2.7.7-aarch64-apple-darwin:$PATH deno task docs:api-reference:check Not-tested: External GitHub CI after push.
normalizeExtension is shared by the alias rewrite path, so it now uses a captured String.prototype.replace through captured Reflect.apply. This keeps escaped @/ source-extension normalization stable even if project code mutates the shared string prototype. Constraint: CodeRabbit thread PRRT_kwDOQaPiP86ZYXsJ requires alias normalization not to call mutable String.prototype.replace through the shared helper. Rejected: Harden the ESM lexer in this PR | the unresolved thread is about the shared extension helper, and parser replace usage is outside this branch's review scope. Confidence: high Scope-risk: narrow Directive: Keep normalizeExtension on captured string intrinsics because alias rewriting imports this helper in long-lived runtimes. Tested: red test first, deno test --preload=src/testing/preload.ts --no-check --allow-all src/transforms/import-rewriter/url-builder.test.ts failed on poisoned String.prototype.replace before the fix. Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all src/transforms/import-rewriter/url-builder.test.ts src/transforms/esm/specifier-resolver.test.ts Tested: deno fmt --check src/transforms/import-rewriter/url-builder.ts src/transforms/import-rewriter/url-builder.test.ts src/transforms/esm/specifier-resolver.test.ts Tested: deno lint src/transforms/import-rewriter/url-builder.ts src/transforms/import-rewriter/url-builder.test.ts src/transforms/esm/specifier-resolver.test.ts Tested: deno check --config deno.json src/transforms/import-rewriter/url-builder.ts src/transforms/import-rewriter/url-builder.test.ts src/transforms/esm/specifier-resolver.test.ts Tested: git diff --check Not-tested: Full repository test suite before commit; pre-push hook will run it before remote update.
The alias extension helper now captures String.prototype.replace, and the generated runtime bundles need to carry that captured intrinsic so CI and local source snapshots stay aligned. Constraint: Generated bundle snapshots changed after the full pre-push verification run. Rejected: Leave generated files dirty after push | a clean branch is required for reproducible CI and review confidence. Confidence: high Scope-risk: narrow Directive: Regenerate these bundles whenever shared import rewriting helpers change bundled runtime code. Tested: deno fmt --check src/build/production-build/templates.ts src/server/services/rsc/endpoints/rsc-bundles.generated.ts Tested: git diff --check Not-tested: Full pre-push after this generated-only commit before this commit; it will run before the next push.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26467889a2
ℹ️ 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".
Separate transform-phase routing from the explicit tenant-source signal used by application-error classification. Legacy build-shaped cache and bundle faults remain error-level while compiler and content failures retain warning-level capture. Constraint: Module loading uses the build-failure marker for both tenant compilation failures and framework infrastructure failures. Rejected: Treat every legacy type=build error as tenant-owned | cache, bundle, and capacity paths emit the same legacy shape. Confidence: high Scope-risk: narrow Directive: Do not infer tenant ownership from the broad build-failure marker or legacy build type. Tested: Application error classifier, render pipeline behavior, Sentry policy, targeted format/lint/typecheck, pinned generated API reference. Not-tested: Full repository suite before commit; enforced by pre-push.
Literal dynamic imports rewritten to /_vf_modules must be fetched and cached before the parent module is persisted, otherwise file-backed execution treats them as filesystem-root paths. The nested import pipeline now scans the existing dynamic import spans, carries their syntax shape through fetch results, and replaces only the quoted specifier for import() calls. Constraint: CodeRabbit thread PRRT_kwDOQaPiP86ZY__J reported escaped alias dynamic imports in cached MDX modules. Rejected: Change alias resolution to emit relative file-cache specifiers | the cache path is only known after nested materialization. Confidence: high Scope-risk: narrow Directive: Keep dynamic nested imports on the source-span scanner; do not rewrite computed import() arguments. Tested: deno test --preload=src/testing/preload.ts --no-check --allow-all src/transforms/mdx/esm-module-loader/module-fetcher/http-fetcher.test.ts src/transforms/mdx/esm-module-loader/module-fetcher/nested-imports.test.ts Tested: deno check src/transforms/mdx/esm-module-loader/module-fetcher/nested-imports.ts src/transforms/mdx/esm-module-loader/module-fetcher/http-fetcher.ts src/transforms/mdx/esm-module-loader/types.ts src/transforms/mdx/esm-module-loader/module-fetcher/nested-imports.test.ts Tested: deno lint src/transforms/mdx/esm-module-loader/module-fetcher/nested-imports.ts src/transforms/mdx/esm-module-loader/module-fetcher/http-fetcher.ts src/transforms/mdx/esm-module-loader/types.ts src/transforms/mdx/esm-module-loader/module-fetcher/nested-imports.test.ts Tested: deno task typecheck Not-tested: End-to-end browser execution before pre-push.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6c5506eb8
ℹ️ 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".
The MDX compiler previously rethrew tenant syntax failures as an ambiguous legacy build error, so the narrowed observability classifier correctly refused to downgrade them. Emit the registered MDX error shape directly and lock the production compiler path with an invalid-MDX regression. Constraint: Legacy type=build is shared by framework infrastructure and cannot safely imply tenant ownership. Rejected: Restore broad legacy build classification | cache, bundle, and capacity failures use the same shape. Confidence: high Scope-risk: narrow Directive: Tenant build ownership must be explicit at compiler/source seams. Tested: MDX compiler regression, application-error classifier, render pipeline behavior, Sentry policy, targeted format/lint/typecheck, pinned API docs. Not-tested: Full repository suite before commit; enforced by pre-push.
The stacked config prerequisite landed on main as a squash commit, so this merge preserves the tenant-error classifier and its regressions while accepting the released config and documentation state. Generated observability references were rebuilt with the CI-pinned Deno toolchain after resolving the overlapping application-error test. Constraint: #3719 merged by squash, so its branch commits are not ancestors of main Rejected: Rebase or force-push the feature branch | rewriting reviewed history would make the repair harder to audit Confidence: high Scope-risk: narrow Directive: Keep framework build failures error-level when changing tenant classification Tested: focused observability, pipeline, MDX compiler, and config suites; pinned API-reference generation; fmt; lint; typecheck; diff-check Not-tested: full pre-push hook runs on the resulting merge commit during push
📦 Client bundle boundary
A server module in a client graph aborts hydration in the browser. New leaks fail CI; known leaks are tracked in |
Non-strict missing-module recovery reused the static import replacement even when the matched span came from a dynamic import, which produced invalid import(from "...") syntax. Extension normalization also captured String.replace but still let RegExp @@replace dispatch through a mutable prototype. The fix keeps dynamic replacements as quoted specifiers and calls the captured RegExp @@replace intrinsic directly. Constraint: PR review requested fixes for unresolved threads PRRT_kwDOQaPiP86ZZdRz and PRRT_kwDOQaPiP86ZZdR2 Rejected: Rewriting the dynamic import call wholesale | source spans intentionally target only the matched specifier Confidence: high Scope-risk: narrow Tested: Pinned Deno 2.7.7 focused nested-imports and url-builder regressions; pinned Deno fmt, lint, and deno check for touched files; pinned Deno generate:manifests:check reached stale pre-existing templates.ts check; git diff --check Not-tested: Full deno task verify after final scope cleanup because generate:manifests:check fails on stale src/build/production-build/templates.ts outside the requested kept file set
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b8878ba66
ℹ️ 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".
Declaration bodies can legally be followed by regex statements, while explicit npm imports can fail at the authored root or inside a transitive dependency. Track declaration-body delimiters in the lightweight export scanner and fingerprint explicit npm roots before assigning tenant ownership. Constraint: Preserve infrastructure failures and transitive package failures at error severity. Rejected: Treat every closing brace as a regex boundary | object expressions require division semantics. Rejected: Mark every npm fetch failure as tenant-authored | transitive dependency failures are framework or upstream concerns. Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep tenant classification tied to the exact authored request fingerprint. Tested: Focused 106-step module/dependency/HTTP cache suite, targeted fmt/lint/check, lint:ci typecheck ratchet, pinned docs check, and full pre-push. Not-tested: Live esm.sh failure behavior outside the mocked 404 and transitive dependency matrix.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91e21cb75a
ℹ️ 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".
The MDX source scanner treated import-from prose inside JSX text as executable source whenever an expression preceded the text. Track JSX text and expression boundaries in the static import scanner, and restrict export from-clause scanning to actual re-export forms. Constraint: The scanner must remain dependency-free and preserve TypeScript value and type re-exports. Rejected: Parse every module with a full JavaScript parser | the existing bounded scanner is used on tolerant and partially transformed source. Confidence: high Scope-risk: narrow Directive: Keep JSX text handling aligned across static import scanner variants. Tested: Focused source-span suite, five related MDX suites, format, lint, typecheck, diff check, and full pre-push (3,852 tests / 28,887 steps plus CWD suites). Not-tested: Browser execution of a tenant MDX document containing the exact prose example.
PR review found two reviewer-blocking edges: symbol tags could be inherited from Error.prototype or accessor-backed, and static import scanning repeatedly walked the remaining source while deciding whether JSX-looking angle assertions had a closing tag. The fix keeps tag reads to own data descriptors, indexes JSX closing tags once per static scan, and preserves the CLI/public export baseline repairs already present in the worktree. Constraint: PR #3723 review requires Symbol.for tenant-build-failure reads to require own data value true without invoking accessors Constraint: JSX closing-tag detection must be bounded for repeated JSX-looking TypeScript angle assertions Rejected: Keep wall-clock performance tests | machine-speed thresholds do not prove the quadratic path was removed Confidence: high Scope-risk: moderate Reversibility: clean Directive: Do not replace descriptor-based symbol reads with property access without re-running the inherited and accessor poisoning regressions Tested: PATH=/private/tmp/deno-2.7.7-aarch64-apple-darwin:$PATH VF_DISABLE_LRU_INTERVAL=1 deno test --preload=src/testing/preload.ts --no-check --allow-all src/rendering/orchestrator/module-loader/index.test.ts src/observability/application-errors.test.ts src/transforms/mdx/esm-module-loader/utils/source-spans.test.ts Tested: PATH=/private/tmp/deno-2.7.7-aarch64-apple-darwin:$PATH deno task fmt:check Tested: PATH=/private/tmp/deno-2.7.7-aarch64-apple-darwin:$PATH deno task lint Tested: PATH=/private/tmp/deno-2.7.7-aarch64-apple-darwin:$PATH deno task typecheck Tested: PATH=/private/tmp/deno-2.7.7-aarch64-apple-darwin:$PATH deno task docs:api-reference:check Tested: PATH=/private/tmp/deno-2.7.7-aarch64-apple-darwin:$PATH deno task test:unit Tested: PATH=/private/tmp/deno-2.7.7-aarch64-apple-darwin:$PATH deno task verify:quick Not-tested: Playwright/e2e binary smoke path
Tenant modules can mutate process globals before later framework error classification runs. Capture the Reflect.getOwnPropertyDescriptor intrinsic inside both tag-reader modules so false own tag descriptors synthesized from a poisoned global cannot convert untagged framework errors into tenant build failures. The tag writers continue using Object.defineProperty because the regression only proves read-time descriptor poisoning creates the false-positive downgrade path. Constraint: Tenant-authored modules can mutate process globals before later error classification. Rejected: Capture Object.defineProperty in tag writers | regressions prove read-time descriptor poisoning is sufficient for the false positives covered here. Confidence: high Scope-risk: narrow Tested: poisoned Reflect red-first regressions, focused touched tests, changed-file deno check, deno fmt --check, deno task lint, direct typecheck entrypoints, deno task test:unit:parallel Not-tested: deno task typecheck wrapper remains blocked before typechecking by stale templates/manifest.generated.ts in the exact PR head
The captured intrinsic shifts public source anchors by one line. Regenerating the API reference preserves the exact generated contract required by the lint gate. Constraint: Generated API references must be produced with pinned Deno 2.7.7 on PATH. Confidence: high Scope-risk: narrow Directive: Regenerate public references whenever observability source anchors move. Tested: Pinned docs generation, API-reference freshness check for all 44 files, and git diff check. Not-tested: No additional runtime behavior; the parent exact head already passed the full pre-push suite.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab0b645f0f
ℹ️ 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".
Root VF module imports now split query and fragment suffixes with the shared specifier suffix helper before fetch and cache resolution, then append the suffix back onto the generated file import. This keeps root dynamic imports aligned with the nested import path behavior without changing the fetched module path. Constraint: PR #3723 review thread identified root dynamic import query and fragment suffixes as the remaining unresolved path. Rejected: Treating query or fragment suffixes as part of the cached module path | fragments were fetched as literal filenames and queries were not restored on the runtime import. Confidence: high Scope-risk: narrow Tested: Deno 2.7.7 focused loader/root writer tests: deno test --preload=src/testing/preload.ts --no-check --allow-all src/transforms/mdx/esm-module-loader/loader-helpers.test.ts src/transforms/mdx/esm-module-loader/module-writer.test.ts Tested: Deno 2.7.7 scanner and nested fetcher tests: deno test --preload=src/testing/preload.ts --no-check --allow-all src/transforms/mdx/esm-module-loader/utils/source-spans.test.ts src/transforms/mdx/esm-module-loader/module-fetcher/nested-imports.test.ts Tested: Deno 2.7.7 HTTP fetcher and alias/static import tests: deno test --preload=src/testing/preload.ts --no-check --allow-all src/transforms/mdx/esm-module-loader/module-fetcher/http-fetcher.test.ts src/transforms/mdx/esm-module-loader/transforms/alias-imports.test.ts Tested: Deno 2.7.7 fmt check for touched files Tested: Deno 2.7.7 lint for touched files Tested: git diff --check Tested: Deno 2.7.7 .husky/pre-push Not-tested: Playwright E2E was not run; this change is isolated to MDX module import rewriting and covered by loader/fetcher unit tests plus the repository pre-push hook.
Tenant code can mutate shared globals and error prototypes before framework failures are classified. Read frontmatter and tenant-context markers only from own data descriptors, capture marker and Set intrinsics at module initialization, and fail closed when proxy inspection throws. Constraint: Framework failures must not be downgraded to tenant warnings by prototype pollution or poisoned globals Rejected: Keep direct property and collection method calls | inherited values and mutable primordials can forge tenant classification Confidence: high Scope-risk: narrow Reversibility: clean Directive: Preserve captured intrinsic and own-data checks at marker classification seams Tested: Focused compiler, observability, Sentry policy, module-loader, and root suffix suites, 78 passed with 86 steps Tested: Changed-file format, lint, typecheck, and diff checks Tested: Full pinned pre-push suite, 3855 unit tests with 28904 steps, 10 cwd tests with 197 steps, and 2 cwd exclusion tests with 2 steps Not-tested: Hosted CI on the pushed commit
Framework severity must depend on provenance stamped at trusted compiler seams, not inherited fields or accessors supplied by an arbitrary error. Normalize esbuild source evidence inside its adapter, then require captured own-data descriptor reads for downstream markers and MDX parser metadata. Constraint: Framework failures must remain error-level under prototype and primordial poisoning Rejected: Read esbuild diagnostic accessors in the compile stage | arbitrary bundler failures could execute getters and forge tenant ownership Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep esbuild accessor reads inside the trusted adapter and require own-data exact-true markers downstream Tested: Focused compiler, observability, Sentry, module-loader, root-suffix, pipeline, and esbuild suites, 88 tests with 150 steps Tested: Changed-file format, lint, typecheck, and diff checks Tested: Full pinned pre-push suite, 3856 unit tests with 28910 steps, 10 cwd tests with 197 steps, and 2 cwd exclusion tests with 2 steps Not-tested: Hosted CI on the pushed commit
The hardened application-error classifier shifted public source locations, so regenerate the observability reference with the CI-pinned Deno toolchain. Constraint: Generated API references must match Deno 2.7.7 output. Rejected: Edit line links manually | regeneration is the repository source of truth. Confidence: high Scope-risk: narrow Directive: Regenerate API references after moving public observability declarations. Tested: Pinned docs:api-reference:check (43 groups, 44 files current), deno fmt --check, git diff --check.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7f53903fc
ℹ️ 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".
Merge the exact main commit that contains PR #3721 without rewriting reviewed history. The resolution keeps its final bounded export-type and import-order scanner while carrying tenant classification, suffix handling, and deferred dependency failures forward. Constraint: PR #3721 was squash-merged, so its original stacked ancestry produced broad textual conflicts against equivalent final code Rejected: Rebase or force-push | would rewrite reviewed PR history Rejected: Choose either scanner wholesale | would drop regressions from the other PR Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep both scanner regression matrices when changing import classification Tested: Scanner and HTTP matrix (451 steps); rendering matrix (194 steps); compiler, observability, and Sentry matrix (190 steps); pinned Deno 2.7.7 fmt, lint CI, typecheck, generated docs, and diff checks Not-tested: Full serialized pre-push hook, which runs after incorporating the latest main commit
Merge current main after the scanner resolution so PR #3723 remains conflict-free without rewriting its reviewed history. The only real overlap now uses the public extension parser from main, preserving JSONC project detection while removing the duplicated parser carried by the stacked branch. Constraint: Main advanced with PR #3388 after the exact PR #3721 merge point Rejected: Retain both parser implementations | leaves dead code and diverging JSONC behavior Rejected: Rebase or force-push | would rewrite reviewed PR history Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep generate project detection aligned with the shared extension manifest parser Tested: Generate unit and integration suites (70 steps), build command/error suites (29 steps), pinned Deno 2.7.7 fmt, lint, typecheck, generated manifests, generated API docs, and diff checks Not-tested: Full serialized pre-push hook, queued after the active PR lanes
Coverage instrumentation exposed repeated backward comment scans for plain keyword-shaped identifiers. Use the immediate non-whitespace character for the common no-comment case and retain the comment-aware fallback where adjacent trivia can contain a block or line comment. Constraint: Comment-separated member names must retain their existing parsing behavior Rejected: Raise the 750ms regression threshold | masks quadratic scanner work Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep the fallback for block comments and line comments across line terminators Tested: Exact coverage shard 1/8 (397 tests, 3763 steps); focused scanner coverage (243 steps); scanner/HTTP matrix (451 steps); fmt, lint, check, diff Not-tested: Hosted CI after this commit
Bring the merged jsdom and React scheduler harness drains into the tenant-error branch so its full leak-sanitized gate can complete without bypasses. Preserve the existing tenant classification hardening, scanner reconciliation, and linear member-name scan. Constraint: PR #3723 must integrate current main non-destructively after #3751 merged Rejected: Rebase or force-push the branch | would rewrite reviewed history Confidence: high Scope-risk: moderate Reversibility: clean Directive: Preserve both the mainline Popover and Field drains and the branch source scanner regressions in future conflict resolution Tested: Pre-merge synthetic merge-tree completed without conflicts Not-tested: Post-merge focused and full pinned gates pending
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ea69dbb34
ℹ️ 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".
| openBraces.push( | ||
| opensDeclarationBody || | ||
| code[previousTokenIndex] === ")" && | ||
| controlConditionCloseParens.has(previousTokenIndex), | ||
| ); |
There was a problem hiding this comment.
Track arrow-body closures before regex literals
When a cycle target contains valid code such as const fn = () => {}\n/export default/.test(source) but has no default export, this stack marks only declaration and control-statement bodies. The slash is consequently treated as division, hasDefaultExport reads the regex contents as a real declaration, and the generated cycle alias re-exports a nonexistent default, causing module instantiation to fail. Track arrow-function body closures as regex-prefix boundaries and add a focused cycle-alias regression.
AGENTS.md reference: AGENTS.md:L11-L13
Useful? React with 👍 / 👎.
Refs VERYFRONT-SERVER-2, VERYFRONT-SERVER-3, VERYFRONT-SERVER-S, VERYFRONT-SERVER-Q, VERYFRONT-SERVER-R.
#3719 is merged. This follow-up targets
maindirectly.Problem
SSR render failures were captured at error level even when the failure came from tenant-authored source. The existing
buildFailuremarker is intentionally broad and also covers framework cache, bundle, filesystem, and capacity failures, so it cannot safely determine observability severity.Fix
captureApplicationErrornow classifies tenant build errors centrally, tags them withveryfront.error_class=tenant-build, and captures them at warning level. Tenant errors remain captured for escalation analysis.The render pipeline keeps two independent signals:
buildFailurepreserves existing SSR and custom-error-page behavior for transform failures.tenantBuildFailureaffects observability severity only when a source seam has explicit tenant evidence.The classifier accepts explicit tenant tags and a narrow allowlist of tenant-facing BUILD slugs. It does not infer tenant ownership from the BUILD category, legacy
type="build", or the broad module-loader marker. Framework cache, bundle, filesystem, capacity, asset optimization, sourcemap, and ambiguous static-generation failures remain error-level.Source-specific handling covers MDX and Markdown parse/frontmatter failures, TypeScript and JavaScript source diagnostics, unresolved tenant imports, side-effect imports, and cached transitive unresolved-import evidence. The unresolved-import sidecar is identity-bound and must persist before reusable cache pointers are published.
The latest review fixes classify a missing authored bare package at the specifier-resolution seam. The HTTP 404 carries a sanitized display URL plus an opaque fingerprint of the exact normalized request, and the fingerprint must match the first effective root package request before it receives
tenantBuildFailure: true. A 404 from a transitive package dependency remains framework-level, as do direct HTTP 404s and framework network, cache, response-body, HTML, and policy failures.Safety invariants
true, so inherited values, accessors, prototype pollution, proxies, and poisoned globals cannot downgrade untagged framework errors.import-resolution-errorfailures remain error-level unless a tenant-source seam explicitly reclassifies them.setLevelremains optional.inorinstanceofdo not terminate lexical scanning early.npm:roots use the same exact-request fingerprint rule as authored bare packages; their transitive 404s remain framework-level.Verification
Exact local head:
5ea69dbb345b7f21190fd844420493969cd21cd8./_vf_modulespath and preserve the suffix onimport.meta.url.npm:roots, and transitive dependency safeguards.8da293cca7c56a58757cccae7548318eb4f42e42integrates exactmain743e07da96d78fbbbf14544ca10d0e68580bbf22, preserving both fix(transforms): keep @/ alias imports off the page origin and stop blaming esm.sh for foreign HTML responses #3721 final export-type/import-order scanner regressions and this PR tenant-classification/scanner regressions. Lore merge commit81c7b590f09e9d07cd9243cd3b18c3dcd3dcd95fintegratesmain78e09f1f2ce1df6699b758d5b65481440d2803e9and keeps generate project detection on the shared extension manifest parser.f972e7d731844161de3c5a10f45050eac8e3d6f8keeps the member-name scanner linear on comment-free identifier runs while retaining the comment-aware fallback and every prior scanner regression.5ea69dbb345b7f21190fd844420493969cd21cd8integrates exact currentmainf30d38007ea52ee0d6f0594be3b03562d7a8286a, including the merged test(ui): contain deferred jsdom timers in harnesses #3751 Popover and Field harness stabilization, with no conflicts while preserving the fix(transforms): keep @/ alias imports off the page origin and stop blaming esm.sh for foreign HTML responses #3721, feat(ui): add UI primitives and an opt-in adapter slice #3388, and fix(observability): downgrade tenant build errors to tagged warning-level captures #3723 regression histories. That integratedmainis a direct ancestor of the exact head. After the push, fix(login): report an existing session instead of re-prompting #3742 advancedmainto71356afcf94da86bc63997ef6eca397090f8a9d2; a fresh synthetic merge is conflict-free with tree5bf242407681529e0d11f493b1909b4f0d84276aand no overlapping fix(observability): downgrade tenant build errors to tagged warning-level captures #3723 files, so the exact green head remains mergeable without another branch mutation.veryfront/observability.mdcarries the final source anchors.git diff --checkpasses.Exact-head GitHub CI is terminal green on the final merged-main commit: 28 checks pass, 6 are intentionally skipped, 0 are pending, and 0 fail. The fully paginated review audit reports 70 review threads with 0 unresolved, 124 reviews, 145 inline review comments, and 56 issue comments. The declaration-block thread was inspected and resolved after the pushed-head exact repro and regression evidence.
Known follow-ups