feat(a11y): Accessibility Studio portlet + AI agent platform (@dotcms/ai, MCP server, agent UI)

[fmontes]

This PR delivers the AI agent platform for dotCMS and its first agent — the Accessibility Studio. Tracks #36640.

What this is

A new admin surface (agents route) where AI agents assist authors, plus the shared plumbing agents stand on. Agent #1 is the Accessibility Studio: pick a page, run a real axe-core scan, let the agent fix the violations it can (streaming its work live), review the working-vs-live diff, then publish or discard the batch.

The design is deliberately agent-agnostic — the streaming-log UI, the SSE transport, and the wire contract are reusable, so agent #2 is a registry entry plus a presenter, not a rebuild.

High-level flow

pick a page  →  scan (axe-core)  →  agent fixes (SSE, live)  →  review diff  →  publish / discard

The fix run makes two passes — a deterministic pass (logs one row per violating element) and an agentic pass (fixes carry no rows). The report accounting reflects both: counts derive from the before/after rescan, not from row counts.

The agentic loop is not in this repo. It runs in a separate service (microservice-accessability#12) on the @dotcms/ai SDK published from libs/sdk/ai. What this PR adds on the loop's behalf is the A11yAgentResource proxy the Studio streams from — see @dotcms/ai SDK for how the two consumers differ.

Changes

Frontend — dot-agents portlet (new)

• Agents shell + registry-driven landing (agents route in app.routes.ts); a11y is the first registered agent.
• Accessibility Studio: page list → run screen (scan/fix/review/publish) → working-vs-live Monaco diff. Two route-scoped SignalStores (a11y-page-list.store, a11y-run.store) so each screen owns independent, per-navigation state. See Agents portlet below for the listing/detail pattern and how agents register.
• Live scan: axe-core score/severity widget, phase-aware violation markers drawn inside the preview iframe, and an axe "incomplete" Needs-your-review section.

Frontend — @dotcms/ai-ui (new shared kernel)

• Agent-agnostic render kernel: AgentMessage view-model + AgentMessagePresenter<T> seam, and the dot-agent-message / dot-agent-thinking / dot-agent-activity-log components. The a11y A11yAgentPresenter is the first consumer.
• Supporting wire contract in @dotcms/dotcms-models (AgentRunStep, AgentStreamEvent<T>) and a generic SSE transport in @dotcms/data-access.

flowchart TD
    portlet["dot-agents portlet<br/>shell + agent registry"]
    a11y["a11y Studio<br/>page list · run · diff"]
    stores["route-scoped stores<br/>a11y-page-list · a11y-run"]
    presenter["A11yAgentPresenter"]
    aiui["@dotcms/ai-ui<br/>message · thinking · activity-log"]
    da["@dotcms/data-access<br/>DotAgentRunService (SSE)"]
    models["@dotcms/dotcms-models<br/>AgentStreamEvent · AgentRunStep"]
    portlet --> a11y --> stores
    a11y --> presenter --> aiui
    stores --> da
    aiui --> models
    da --> models
    stores -. "POST /agents/a11y" .-> backend["A11yAgentResource"]

Loading

Backend — A11yAgentResource (new)

• REST proxy at /api/v1/agents/a11y: authenticates the backend user, mints a short-lived JWT, resolves the page identifier to a full agent payload, and relays to the external agent service — JSON /fix, SSE /fix/stream, /stop, /active-run. Agent base URL + service token come from the dotPageScanner-config App secret, never from request input.

Backend — making our own API usable by agents

Why this is in an agent PR: building the MCP server and the a11y agent meant being the first heavy non-human consumer of our own write endpoints, and that surfaced a pattern. A human hitting a 500 opens the logs, asks someone, and retries. An agent can't — it only sees the response, so an opaque error is a dead end it cannot recover from, and a missing description is a parameter it will never pass. Every fix below is one of those dead ends, found by watching an agent fail on it:

• Unknown field → a fixable 400. A new auto-discovered @Provider, UnrecognizedPropertyExceptionMapper, turns Jackson's class-name-and-JSON-pointer dump into Unrecognized field 'notARealField'. Valid fields are: [body, drawed, theme, title] — the agent reads the valid list and retries correctly instead of guessing. Full detail still goes to the server log.
• 500 → 400 on bad input. TemplateResource.fillTemplate null-body / unset-themeFolder guards; a malformed request now says so instead of looking like our bug.
• Velocity failures become structured. /api/vtl/dynamic returns {errors:[{message, errorType, templateName, line, column}]} on parse/eval failures, and non-fatal warnings (undefined references — Velocity is non-strict, so these silently render empty) ride along in X-Dot-Velocity-Warnings, length-capped so a big warning set can't blow Tomcat's header limit and turn a 200 into a failure. This is what lets an agent fix its own VTL instead of shipping a blank page.
• Documented what was undiscoverable. Explicit host_id @QueryParam on PageResource and typed @Schema request-body descriptions across the write forms — an agent reads the spec to decide what to send, so an undocumented parameter effectively does not exist. openapi.yaml regenerated to match.

These are small, but they're the difference between an agent that self-corrects and one that stalls. They also make the endpoints friendlier for humans and integrators, which is why they're worth keeping regardless of the agent work.

Agents portlet (libs/portlets/dot-agents)

Every agent is the same shape: a listing of things to work on, and a detail screen that works on one of them. The portlet provides that shape once, so an agent is a registry entry plus its own two screens.

Two levels of listing → detail. The pattern repeats, which is what keeps agent #2 cheap:

Level	Listing	Detail
Portlet	agent gallery (agents)	one agent, full-screen (agents/{id})
Agent (a11y)	page list (agents/a11y)	one page's run (agents/a11y/{path})

agents                        → gallery: one card per registered agent
  agents/a11y                 → listing: pages you can scan
    agents/a11y/blog/post/x   → detail:  scan · fix · review · publish

How agents are registered

agent-registry.ts is the single extension point. Everything else — gallery cards and child routes — derives from one DOT_AGENTS array, so adding an agent touches no shell, landing, or routing code:

export const DOT_AGENTS: readonly AgentDefinition[] = [
    {
        id: 'a11y',                          // URL segment + i18n key stem
        labelKey: 'agents.a11y.label',       // card title
        descriptionKey: 'agents.a11y.description',
        icon: 'accessibility_new',           // Material Symbols ligature
        iconColor: 'blue',                   // dot-color-icon accent
        status: 'available',
        loadChildren: () =>                  // the agent's own routes, lazy
            import('./agents/a11y/a11y.routes').then((m) => m.dotAccessibilityStudioRoutes)
    },
    { id: 'geo-fixer', /* … */ status: 'coming-soon' }   // no loadChildren → no route
];

lib.routes.ts reads that array and derives the routes:

const agentRoutes = DOT_AGENTS.flatMap((agent) =>
    agent.loadChildren ? [{ path: agent.id, loadChildren: agent.loadChildren }] : []
);

status is the whole roadmap mechanism: coming-soon agents have no loadChildren, so they render a disabled card and register no route — no dead links, and a planned agent is announceable in one entry. status: 'available' plus a loader is what turns it on.

To add agent #2: build its UI under src/lib/agents/{id}/ with its own {id}.routes.ts (listing at '', detail below it), then add one DOT_AGENTS entry. The gallery card and the agents/{id} route appear automatically.

flowchart TD
    registry["agent-registry.ts<br/>DOT_AGENTS (single source)"]
    landing["gallery landing<br/>one card per entry"]
    routes["lib.routes.ts<br/>agents/{id} per entry"]
    shell["agents shell<br/>router-outlet"]
    a11y["a11y root<br/>router-outlet"]
    list["page list<br/>A11yPageListStore"]
    run["run screen<br/>A11yRunStore"]
    registry --> landing
    registry --> routes
    routes --> shell
    shell --> a11y
    a11y -- "''" --> list
    a11y -- "'**'" --> run
    list -- "row via router state" --> run

Loading

Why the stores are route-scoped

Each level's hosts are thin <router-outlet> wrappers holding no state (DotAgentsShellComponent, DotA11yRootComponent); state lives in a SignalStore provided at the screen component:

• A11yPageListStore — provided at the page list. Owns search, pagination, the page query.
• A11yRunStore — provided at the run screen. Owns one page's scan/fix/review/publish lifecycle.

Because the store is provided at the component and not the root, navigating to a different page destroys and recreates it: fresh run state per page, no manual reset, and list state can never leak into a run. The run store also has an onDestroy that tears down in-flight SSE/scan subscriptions, so navigating away mid-run aborts the streams rather than leaking them.

The page list hands the selected row to the run screen in the navigation's state, so the detail screen needs no lookup of its own. The URL still carries the page's readable path (agents/a11y/blog/post/hello) for display — a wildcard route, since a page path is multi-segment and cannot be a single Angular param. Trade-off worth knowing: the run route is reachable only through the list, so a refresh or pasted run URL bounces back to the listing.

What is agent-specific vs shared

The agents/a11y/ folder is the only a11y-aware code. Everything an agent needs to stream its work is shared and agent-agnostic:

• @dotcms/ai-ui — the activity-log components, driven by an AgentMessage view-model
• AgentMessagePresenter<T> — the seam each agent implements (A11yAgentPresenter maps a11y phases/results to log rows); this is the one file agent Test Branch and Commit #2 writes to get a live log
• DotAgentRunService (@dotcms/data-access) — the generic SSE transport, provided per run route rather than at the root

@dotcms/ai SDK (libs/sdk/ai)

The execution layer under everything above. Most CMSs hand an AI a fixed menu of tools — it can only do what the vendor pre-built. @dotcms/ai inverts that: the model writes code, and the runtime runs it in a sandbox against the whole dotCMS API, with auth and policy owned in one place. The ceiling isn't a tool list — it's the API itself. There's no LLM inside; it's the layer beneath a model/agent framework, and it's what dotCMS's own MCP server and first-party agents run on (we ship on it, not just publish it).

Two consumers, two verbs. The runtime is deliberately not sandbox-only. createRuntime exposes both, and the rule is about authorship: request is the default — use it when you wrote the call; run is for code you did not write (a model did) and therefore must confine. Both flow through one shared request core, so the allow-list and auth injection apply identically and the two verbs cannot drift.

Consumer	Verb	Why
MCP server (apps/mcp-server, this PR)	sandboxed run	the model writes the code, so it must be confined
a11y agentic loop (microservice-accessability#12)	direct request	the loop's own tool code is trusted; the model never gets a request tool

The agentic loop lives in a separate service, not in this repo — the A11yAgentResource proxy added here is what the Studio talks to, and that service is what runs the scan → fix → re-scan loop. It consumes @dotcms/ai as a published npm package (1.5.6-beta.1), which is why the SDK's public surface and its versioning matter beyond this repo: src/agents/dotcms/client.ts wraps createRuntime(...).request as its only transport, then hands the model a curated tool set (locateSources, readAsset, grepAssets, editAsset, solveContrast, rescan) — never a generic request tool, never the bearer token. Same principle as the sandbox path, enforced at the tool boundary instead of by confinement.

Governed by construction — safety is the shape of the runtime, not a setting:

• Your token never enters the sandbox — auth is injected host-side; executing code can't read it.
• Adapters are the only door out — sandbox code reaches the network/host only through an adapter you grant; direct fetch / require / process.env are removed.
• You decide the surface — an allow-list (or typed defineAdapter operations) bounds what any code can reach. Expose scan and read; never expose delete.

Public surface — four subpaths: @dotcms/ai/runtime (the front door — one runtime, two verbs), @dotcms/ai/sandbox (the generic execution engine, a lint-enforced zero-dotCMS boundary), @dotcms/ai/adapter (the dotCMS wiring — auth, context, operations), and @dotcms/ai/spec (the filtered OpenAPI spec agents query).

flowchart LR
    code["model-written code"]
    subgraph runtime["@dotcms/ai/runtime (front door)"]
        sandbox["sandbox<br/>confined worker · no fetch/require/env<br/>(zero-dotCMS boundary)"]
        adapter["adapter<br/>allow-listed operations"]
    end
    token["auth token<br/>(host-side only)"]
    api["dotCMS API"]
    code --> sandbox
    sandbox -- "only door out" --> adapter
    adapter --> api
    token -. "injected host-side,<br/>never enters sandbox" .-> adapter
    spec["@dotcms/ai/spec<br/>filtered OpenAPI"] --> code

Loading

What this PR changes:

• Sandbox: new format-result (structured, size-capped result formatting reused by the MCP execute tool) and worker-harness helpers; createSandbox gives each run() its own AbortController so a timeout aborts in-flight host work.
• Adapter: binary responses (e.g. /api/v2/assets, /dA) return a { __dotcmsBinary, contentType, base64, byteLength } envelope capped at 25 MB (checked against Content-Length before buffering); user-supplied fetch URLs are SSRF-guarded (loopback / link-local 169.254.0.0/16 incl. cloud metadata / RFC-1918 / IPv6 unique-local rejected).
• Spec generation: rewritten as a testable spec-transform (extracted from the monolithic generate-spec script) — $ref-based output keyed by schema name, with request/response schemas kept and context caps applied, so agents resolveRef(name, depth) instead of hand-walking refs.

MCP server (apps/mcp-server)

The MCP server exposes dotCMS to AI agents as tools, running on the @dotcms/ai SDK above. This PR turns page authoring from a single fragile call into a three-step, verify-as-you-go workflow, and hardens the general-purpose tools around it.

New page-authoring tools — the intended flow is page_create → page_place_content → page_verify:

• page_create — creates and publishes a blank page in one safe call. Splits urlPath into parent folder + leaf (creating the folder idempotently) to avoid the silent /index URL-collapse trap; resolves any HTMLPAGE base-type content type instead of hard-coding htmlpageasset; validates user-added required fields before firing; resolves the site to its identifier and sends it as contentHost (fixes the root-page host is null NPE).
• page_place_content — populates a page's slots with contentlets after it exists. This is the step that turns a blank page_create result into a real page.
• page_verify — confirms a page actually renders: catches a blank slot, a swallowed VTL error, a cache-stale page, or an unpublished edit — the failure modes that a create-and-publish call reports as success but a human would see as broken.

flowchart LR
    agent["AI agent"]
    create["page_create<br/>blank page + folder"]
    place["page_place_content<br/>fill the slots"]
    verify["page_verify<br/>does it render?"]
    sdk["@dotcms/ai runtime + adapter"]
    agent --> create --> place --> verify
    verify -. "empty slot / VTL error /<br/>cache-stale → fix" .-> place
    create --> sdk
    place --> sdk
    verify --> sdk
    sdk --> api["dotCMS API"]

Loading

Supporting tool + SDK hardening:

• assets-transfer reworked (binary-safe transfer path, expanded coverage).
• execute (JS sandbox, not VTL): timeout default raised to 45s, results routed through the SDK's formatSandboxResult.
• search reframed as a curated OpenAPI-spec query — a spec global with a resolveRef(name, depth) helper and a ~25k-char output cap — instead of hand-walking $refs.
• upload_assets accepts string booleans without the z.coerce "false" → true trap and uploads empty files as-is.

(The binary-envelope, SSRF guard, and spec query these tools rely on come from the @dotcms/ai SDK — see above.)

Reviewer notes

• Scope: the branch diff vs main is large only because main was merged in (unrelated trunk work). The coherent unit is the agent platform + Studio + authoring/OpenAPI hardening described above.
• Highest-risk backend items: UnrecognizedPropertyExceptionMapper is a new @Provider and JAX-RS auto-discovers it, so it changes the 400 body on every Jackson-deserialized write endpoint in the product, not just the ones in this PR. Status stays 400 and full detail still goes to the server log, but anything asserting on the old raw Jackson message will see different text. Also the TemplateResource.fillTemplate guards.
• Security: A11yAgentResource uses an App-secret-derived URL/token and a minted JWT (never the caller's credentials, never request-supplied); the @dotcms/ai adapter rejects loopback/link-local/RFC-1918 URLs (SSRF) and caps binary bodies at 25 MB.
• Screenshots: please attach recordings of the page list, live scan + marker overlay, and the agent fix stream before review — the description is diff-derived and can't capture the UI.

🤖 Generated with Claude Code

[oidacra]

This errorType badge is a hand-rolled span with hardcoded severity coloring (bg-red-100 / text-red-700). Since it's a read-only status indicator, would <p-tag severity="danger" [value]="detail.errorType" /> be preferable, so the color comes from the theme preset rather than a literal Tailwind pairing?

[fmontes]

Fixed in 09ee788fb7 → <p-tag severity="danger" [value]="detail.errorType" styleClass="font-mono" />. Kept the mono face since it renders an error type token; the color now comes from the theme preset.

[oidacra]

Same as the error badge on the page component: this warning.type span hardcodes bg-yellow-100 / text-yellow-800. Would <p-tag severity="warn" [value]="warning.type" /> be a better fit for a read-only status label?

[fmontes]

Fixed in 09ee788fb7 → <p-tag severity="warn" [value]="warning.type" styleClass="font-mono" />, same shape as the error badge.

[oidacra]

I don't see a test that unsubscribes from run() mid-stream and asserts the request is actually aborted.

That teardown (() => controller.abort()) is the only thing stopping an in-flight agent stream when the user navigates away or restarts a run, so a refactor that dropped the signal from fetch or moved the abort call would leak requests with this suite still green. Worth asserting fetchMock.mock.calls[0][1].signal.aborted after unsubscribing?

[oidacra]

The failure coverage here stops at the ok: false response. There are two other error paths in the service - fetch itself rejecting (network drop before any response) and reader.read() rejecting mid-stream after some events have been emitted - each with its own try/catch calling subscriber.error.

Would be worth covering both, since a regression that swallowed either would turn a dropped connection into a silently stalled stream.

[oidacra]

The chunk-boundary tests here are good. Is there also a case for a frame whose data: line isn't valid JSON?

#parseFrame deliberately catches and returns null to drop it, but nothing pins that down - a change that let the parse error propagate would take down the whole stream on one bad frame from a flaky backend, and this suite would still pass.

[oidacra]

This spec covers splitIncludePatterns and includeMatcher only, but the production diff for assets-transfer.ts is +214/-35 and reworks real transfer behavior that has no coverage here: the 0-byte skip being removed (files now upload as-is), the new retry-with-'\n' fallback, the new final re-check pass in verifyLive that fixes a false-negative, and the new totalSeen / zero-match warning branch. Neither uploadAssets nor downloadAssets is invoked with a fake runtime.

Would it be worth adding a few tests around those paths, given they're the behavior changes the rework is for?

[oidacra]

stopAgent() swallows a failed /stop via catchError(() => EMPTY), but every test here drives agentService.stop through the success mock. Would a case with throwError be worth adding, to pin down that a failed stop leaves the store in a sane phase and doesn't take down the stream?

[oidacra]

rescanPreviewDuringFix() fires on every progress frame and swallows scan failures via catchError(() => EMPTY), but the progress tests only exercise the success path.

Worth a case where the mid-fix rescan errors, asserting the phase stays fixing and previewRevision doesn't advance? That's a named failure mode in the code with no coverage.

[oidacra]

onLazyLoad() converts the PrimeNG table event into a 1-based page number with Math.floor(first / rows) + 1 and calls store.setPagination, but nothing in this spec exercises it.

Off-by-one page math tends to regress quietly - worth a test asserting { first: 25, rows: 25 } maps to page 2, plus the first: 0 boundary?

[oidacra]

These marker-visibility tests assert on the showMarkers() computed, but the behavior that matters is the effect calling markerService.render(...) with show ? groups : [] for each of the two frames. A11yMarkerService is mocked and never asserted on.

Would asserting the render calls catch more? An inverted ternary or the wrong groups passed to the wrong frame would currently pass.

[oidacra]

The assertions here select by CSS class and tag (.material-symbols-outlined, dot-color-icon, .w-0\.5, .pi-spinner) rather than byTestId, and the template has no data-testid attributes.

Since those are styling and icon-font details, a purely visual refactor would break these tests without any behavior change - worth adding testids to the icon, connector and chip elements?

[oidacra]

Same note: these select on i.pi-spinner and .agent-shimmer rather than byTestId. Adding data-testid to the spinner and shimmer label would decouple the tests from the icon library and the local shimmer class name.

[oidacra]

The splitUrlPath coverage here is thorough on percent-decoding, ./.. collapsing and query/fragment stripping. One case that looks absent: a path beginning with //.

new URL('//books/index', 'http://_').pathname is /x-style scheme-relative parsing and returns /index, silently discarding the first segment as a host - so //books/index would resolve to folder / rather than /books. Worth pinning down the intended behavior (error vs documented equivalence), given this function exists to prevent silent misplacement?

[fmontes]

Good catch, and it was worse than a missing test — //books/index really did resolve to /books... no: it resolved to /index. Fixed in cf139e1313.

To the URL API a leading // is scheme-relative, so new URL('//books/index', 'http://_').pathname parses books as a host and returns /index — the first segment silently discarded, a page path resolving to a different page. /// did not parse at all. Leading slashes are collapsed before parsing now, which is a real concern rather than a theoretical one, since //host/path is how dotCMS writes a host-qualified path elsewhere.

This also turned out to affect more than splitUrlPath: page_verify and page_place_content were interpolating the raw caller path straight into the request URL with no normalization at all. That is now shared in a new lib/page-path.ts — details in the top-level comment.

[oidacra]

This package-private constructor is here for unit tests, but there's no test file for this resource anywhere in the repo.

Given the class relays SSE, mints tokens, and reads App secrets, would some coverage of the config-resolution and error-relay paths be worth adding while the seam is fresh? This is currently the only acceptance criterion on the issue with no automated test behind it.

[fmontes]

Done in 99b54608a2 — A11yAgentResourceTest, 12 tests, using the package-private seam and mirroring PageScannerResourceTest.

Covers: App not configured and half-configured (missing apiUrl or apiAuthToken) across all four endpoints; request validation; that a malformed request mints no token and never touches App secrets; that /fix/stream reports a config failure in-band with a closed stream; and that the App auth token never appears in an error body — the proxy is the auth boundary, so a relayed error is where a secret would most plausibly escape.

Config resolution and the error-relay paths are covered; the happy-path forward still is not, since that needs a stubbed HttpClient exchange.

[oidacra]

This comment says the wildcard route captures the path and "the run screen reads it back and rehydrates the page", but the run component reads the row from location.getState() and bounces back to the list when it's absent - its own comment notes the URL "can't supply identifier/host/language".

Since the two comments disagree, someone reading this file would reasonably conclude deep-linking to a run works. Worth rewording to match the actual bounce behavior?

[fmontes]

Fixed in 09ee788fb7. You were right that the two comments disagreed, and the route comment was the wrong one — the URL is display and history only.

Reworded to say so explicitly: the page is handed over in navigation state because the path alone cannot supply identifier, host and language, so a cold load (new tab, refresh, pasted link) has no state and bounces to the list.

[zJaaal]

Consolidated review: frontend and MCP server

This single comment replaces 45 inline threads plus 11 shorter notes of my own, removed to keep the PR readable. Everything is here with file:line, the concrete failure, and the fix direction, so it can be worked top to bottom.

Scope: TypeScript only. core-web/apps/mcp-server, the dot-agents portlet, libs/ai-ui, libs/sdk/ai, and the touched edit-ema and data-access files. Reviewed at HEAD (b142e43).

Organised around the four themes that recur across this PR:

Theme	Findings	Worst site
1. Error serialization	7 + 6 minor	mcp-server/src/lib/runtime.ts:26
2. Missing try/catch	8	mcp-server/src/lib/assets-transfer.ts:227
3. await vs promise	5 + full loop inventory	assets-transfer.ts:359/369/384
4. Type duplication	6 + 5 nits	a11y/models/a11y-groups.ts:13
5. Other frontend defects	13	a11y/store/a11y-run.store.ts:665

Deliberately out of scope: the Java backend, openapi.yaml versus its annotations, and a systematic test-coverage audit. There are separate findings in A11yAgentResource.java (a missing permission check, a Host-header-derived callback URL sent with the caller's JWT, a token TTL shorter than the runs it authenticates, and internal topology in a relayed error message). Those want a backend reviewer rather than this thread; ask and I will hand them over.

---

1. Error serialization

The test applied at every throw site: given only this error string, can the calling model fix the call and retry, without asking a human and without guessing? For an MCP tool a vague error is a functional bug, not a cosmetic one.

mcp-server/src/lib/runtime.ts:26 — MEDIUM. errorMessage is the root cause of every unactionable tool error.
@dotcms/ai/runtime already exports DotCMSError, ValidationError, PolicyError, HttpError, TimeoutError, AbortError, isDotCMSError and serializeError (sdk/ai/src/runtime.ts:206-218), and requestCore already throws them (request-core.ts:305,313,330,347,369,403) carrying code, status, statusText, body, detail() and toJSON(). errorMessage flattens all of it to .message. All five non-sandbox tools funnel through it (page_create.ts:101, page_place_content.ts:146, page_verify.ts:99, upload_assets.ts:103, download_assets.ts:73), so the model can never tell retryable from terminal: a 429 on file 3 of 200 looks identical to a permanent 403, so it either abandons a transfer that would have succeeded or retries one that cannot.
Fix: one toolFailure(operation, error, extra) helper here, reusing the SDK types. This is also where the [MCP Server - <operation>]: <error> prefix convention belongs, owned by one helper instead of template strings at each site. Critically, retryable must be a field on the result, not just a type, because MCP hands the model a string and instanceof is unavailable on the far side. Do not add a parallel error hierarchy: formatSandboxResult is the intended layer and execute.ts:136 and search.ts:96 already use it.

mcp-server/src/tools/page_verify.ts:99, plus page_create.ts:101, page_place_content.ts:146, upload_assets.ts:103 — MEDIUM. Uncapped error body, and no request timeout.
HttpError.message is HTTP ${status} ${statusText}: ${body} where body is the full uncapped response.text() (request-core.ts:402; the 25MB caps only cover the binary and remote-fetch paths). A dotCMS 500 returns its HTML stack-trace page, tens to hundreds of KB, handed straight to the model. lib/assets-transfer.ts:222 makes it worse by storing one full message per failed file, so 200 files against a broken instance yields 200 copies of that HTML in one manifest. Amplifiers at lib/page-place-content.ts:437 and lib/page-create.ts:437.
No timeout: lib/runtime.ts:17 passes timeout: opts?.timeout, but that only bounds run(). request() (sdk/ai/src/runtime.ts:111-118) forwards only opts?.signal, requestCore adds none, and no lib tool ever passes a signal. So a wedged instance hangs the MCP call forever, and TimeoutError, the one unambiguously retryable code, can never be produced. execute (45s) and search (10s) are bounded only because they go through run().
Fix: reuse the existing 25k cap, and thread an AbortSignal with a default deadline through dotcms.request for all four.

mcp-server/src/lib/assets-transfer.ts:121, :159, :295, :300 — MEDIUM. Four throws hold the value, and sometimes the entire fix, in scope and echo none of it.
:121 'Asset is missing identifier or path' has asset, input.path, assetPath and identifier all in scope, and rel falls back to '(unknown)' at :114, so the manifest can say {path:'(unknown)', error:...}, unmappable in a batch of hundreds. :159 'Destination file already exists' is the highest-value one: it fires for every already-present file on any re-run and never names overwrite: 'overwrite', the caller-controlled value that resolves it. That is a constraint a model only discovers on its second run, so it belongs in the tool description too. :295 and :300 omit the request path, so "wrong endpoint" or "path is a folder" is indistinguishable from "the asset is genuinely 0 bytes".
Note: this same file already sets the right standard. zeroMatchWarning (:478-500) echoes the input, explains the //host parse, states the corrected value, and marks it not-a-success. Copy that shape.

**mcp-server/src/lib/page-place-content.ts:431 — MEDIUM. The docblock promises a 400 branch that does not exist, and the 409 is sniffed by regex.** :412-416claims it translates "the two documented non-200 outcomes ... a 400 usually means a contentlet's type is not allowed in its container." The only branches are the 409 at:433and the generic fallthrough at:440. Placing a Bannerinto aBlog-only container yields Failed to save page content: HTTP 400 Bad Request: . That is the single most common failure of this tool in a placement loop, and the model cannot tell the fix is "different container or different type", so it retries identically. :433matches/\b409\b|net content loss|conflict/iagainst the message whenerror instanceof HttpError && error.status === 409is available: it over-matches any body containing "409" or "conflict" and under-matches a 409 saying neither.causeis dropped at both:434and:440, which is why codeandstatusnever reach the boundary. *Secondary:*:333-335names the language but enumerates none, and never mentionsvariantName despite it being an argument (:315-319) that silently enters the query (:328`).

**mcp-server/src/lib/page-verify.ts:385-409 — MEDIUM. extractStatusfabricates a verdict, not just a status.**renderPagereturns a synthetic{status, body:{}}instead of rethrowing, andbuildManifest turns it into a confident diagnosis (:320): *"Render returned HTTP nnn. The page did not render, check the path, site, and that the page exists."* The fallback is message.match(/\b(\d{3})\b/), so connect ETIMEDOUT 10.0.0.5:443becomes status443and a definitive statement that the page is broken. The model then "fixes" a page that is fine. *Fix:*if (isDotCMSError(error) && error.code === 'HTTP') return {status: error.status, body:{}}; throw error;which deletesextractStatusentirely. Real cases are already covered byHttpError.status`.

**mcp-server/src/lib/page-create.ts:433-438 — MEDIUM. JSON.stringify(undefined)prints the literal textundefined.** `` Response entity: ${JSON.stringify(responseEntity(response))}``. An absent entity is exactly the condition that triggers this branch, so the model readsResponse entity: undefined; when the entity is present it dumps an uncapped response blob instead. *Fix:* responseEntity(response) ?? '(no entity in response)'`, and name the next step.

**libs/sdk/ai/src/sandbox/format-result.ts:41 — MEDIUM. The serializer itself can throw, and it takes the logs with it.** postMessageuses structured clone, which supports circular references andBigInt; JSON does not. *Failure:* model code builds a tree from a flat folder or page list with parent back-pointers, the ordinary way. The worker reports success: true, structured clone transfers it intact, then :41throwsTypeError: Converting circular structure to JSONout offormatSandboxResultto the tool caller.result.logsare attached at:42-43**after** the stringify, so everyconsole.logthe model used to debug is discarded at the worst possible moment. It presents as "the tool breaks only when my code succeeds". *Fix:* try/catch with aWeakSetreplacer plus BigInt to string, and buildlogs` before the stringify.

Minor, same class:

• tools/execute.ts:123 and tools/search.ts:82 call createRuntime outside any try/catch, so a config failure escapes as an MCP protocol error rather than a tool result. execute.ts has no try/catch around the handler body at all.
• All five lib tools: runtimeFromEnv can throw createRuntime: token is required when AUTH_TOKEN/DOTCMS_URL are unset. The model reads that as an input problem, while upload_assets.ts:58 and download_assets.ts:36 explicitly tell it "you do NOT need a dotCMS token, never go looking for them". A server misconfiguration therefore pushes the model toward exactly the credential-hunting the descriptions forbid. Label it terminal and not the caller's fault.
• lib/runtime.ts:19-21: a context-load failure only console.errors, so the model sees Available sites: (none found) (page-create.ts:226) and concludes the instance has no sites rather than "context load failed, retry".
• lib/page-place-content.ts:123-126: correctly atomic (it fails before any write) but throws on the first bad slot address only, so three bad addresses cost three round trips. Accumulate them.
• lib/page-place-content.ts:340: Page "x" resolved but has no identifier. is terminal with no next action; say "report this, do not retry".
• lib/assets-transfer.ts:66, :511-521: mkdir/access throw raw Node errors (EACCES: permission denied, mkdir '/x'), legible but unlabelled, so the model cannot tell a local-disk problem from a dotCMS one.

---

2. Missing try/catch

The damage from an unguarded await is rarely the thrown error. It is the state left behind, and the report that never reaches the caller.

**mcp-server/src/lib/assets-transfer.ts:227 — MEDIUM, the worst of this class. An unguarded verifyLivediscards the report of completed writes.**verifyLivehas four unguardedawait sites (:360, :370, :385, and :393with no error handling at all), and it is itself awaited at:227with no try/catch. *Failure A, the loop at:369-375:* round 0 finds 50 assets not live and starts re-firing PUBLISH. Asset #3 returns 400 (locked by another workflow, or the token lacks PUBLISH on that folder). Assets #4 to #50 are **never attempted** even though the code already knows they are not live, pendingis never updated, and the function throws. Committed state: #1 and #2 republished, #3 to #50 not, and nothing in the manifest records which. Same shape in the final pass at:384-388. *Failure B, the bigger one:* upload a 120-file theme with the default publish:true, verify:true. All 120 upload and publish, then one flaky liveness GET fails, the throw propagates out of uploadAssets, and the handler (tools/upload_assets.ts:102-104) returns a bare Error: HTTP 400 .... files[](120 paths and identifiers),failures[]andwarnings[]are all discarded. 120 assets are live in dotCMS and the model is told the operation failed, so its next move is to re-upload everything. A read-only verification nicety nullifies the report of a completed write. *Fix:* guard:227` so verification can only ever downgrade the manifest, never replace it with an exception, and give each loop the shape set out in §3.

**mcp-server/src/lib/assets-transfer.ts:354 — MEDIUM. A silent filter can make "verified" mean "nothing was checked".** files.filter((file) => file.identifier)silently drops every file whose identifier was not parsed, and the cast at:326 (as Promise<{entity?:{identifier?:string}}>) is unchecked. If /api/v2/assets/publishreturns 200 with an envelope that does not match, every identifier isundefined, all files are filtered out, the round loop never runs, and the manifest reports count: 120, failures: [], notLive: [], warnings: []`, indistinguishable from a fully verified publish when nothing was verified at all.
Fix: a file with no identifier must be a warning, not a silent skip.

**mcp-server/src/lib/page-create.ts:126 — MEDIUM. A committed folder write sits before an unguarded fire.** ensureFolder (:126) persists, then the PUBLISH fire (:139) is unguarded. template is the one input never pre-validated (siteandcontentTypeare resolved against cached context with candidate lists at:227and:258); the schema only says "the template UUID, not its name" (tools/page_create.ts:19-22). *Failure:* page_create({urlPath:"/books/index", template:"My Template"}), the exact trap the tool description warns about. /booksnow exists on the site, the fire 400s, and the handler returns onlyError: HTTP 400 .... An empty /booksthat nothing mentions; corrected retries accumulate more stale folders, and a retry with a different leaf operates on folder state the caller does not know exists.languageId (tools/page_create.ts:37, .int()with no.positive()) and cacheTtl (:38) are unvalidated the same way, so they too can only fail after the folder write lands. *Fix:* validate templatebeforeensureFolder, and on failure report the inputs, that folder was created, and that re-running is safe becausecreatefolders` is idempotent. That last sentence is what makes it recoverable.

**dot-agents/.../a11y/store/a11y-run.store.ts:578 — HIGH. The fix SSE subscription has no completehandler, so the run wedges infixingforever.**.subscribe((event) => ...)supplies onlynext. catchErrorcovers errors, but the store leavesfixingonly on adone, abortedorerrorframe, andDotAgentRunServicecallssubscriber.complete() whenever the fetch body ends (dot-agent-run.service.ts:117). A body ending without a terminal frame is routine: agent pod restart, load-balancer or ingress idle timeout on a multi-minute run, and the relay closing its output cleanly on an upstream socket drop, which produces no error frame. *Failure:* the agent dies 90 seconds in. Phase stays fixing, the "still working" pill shimmers indefinitely, and the only control is "Stop agent", which is itself a silent no-op (next finding), so there is no way back to scannedshort of navigating away and redoing the whole scan. *Related:*#parseFrameonly consumes frames terminated by\n\n (dot-agent-run.service.ts:106-115), so a final frame without the trailing blank line is dropped. *Fix:* add complete, treat "completed while still fixing" as abnormal (restore phase: 'scanned', set fixError`), and flush buffered partial frames.

**.../a11y/store/a11y-run.store.ts:647 — MEDIUM. stopAgent()swallows every failure.**.pipe(take(1), catchError(() => EMPTY)).subscribe(): no signal, no state change, and not held in a SubscriptionSlot, so teardown()cannot cancel it. If the agent is unreachable or the run id was already discarded (404 or 502), nothing happens, the phase staysfixing, and each further click fires another silent POST. This is what makes the missing complete` handler unrecoverable: the only remaining control is guaranteed to fail silently exactly when it is needed.

**.../a11y/services/dot-page-sources.service.ts:165 — MEDIUM. A swallowed fetch error renders as a full-file deletion.** fetchTextmaps any error toof('')`, so a 502 on the working version renders as a real diff showing the entire file deleted: large red minus-N badge, Monaco showing the full deletion. The user concludes the agent wiped a template and reaches for Discard, which is itself a no-op that reverts nothing (§5). This is the clearest case in the PR of a caught error being worse than an uncaught one.
Fix: propagate, mark the file "could not load", and exclude it from both the diff and the count.

**.../a11y/store/a11y-run.store.ts:203 — MEDIUM. A terminal frame with no scanthrows inside the computeds and blanks the run screen.**:203readsreport.scan.before.violations; :212guardsreportbut notscan. unwrapReport (services/dot-a11y-agent.service.ts:83-87) falls back to payload as FixReport, so any truthy terminal payload becomes report. *Failure:* "Stop agent" produces abortedwith a partial payload carrying noscan(a documented shape;FixReport.statusexists for status-only frames), sobeforeCount, afterCount, openCount, fixedCountandreportedCountall throw during change detection. Score widget, footer and donut die together, leaving a blank pane until reload, and losing the run the user was gracefully stopping. *Fix:* guardscan, and have unwrapReport validate ('report' in payload) instead of double-casting at :84and:86`.

**libs/sdk/ai/src/sandbox/worker-harness.ts:96 — MEDIUM. resolveRefhas a caller-controlleddepthwith no bound and no cycle detection.** Termination relies solely ondepth, and expandcopies the whole subtree per$refhop with no memo and no visited set.depthis model-chosen, and asking for a bigger number is exactly what a model does when depth 2 looks truncated. The committedopenapi.yamlis **not** acyclic, contradictingspec-transform.ts's "naturally acyclic" claim: self-refs FolderView, MultiPart, Permissionable, the cycle BodyPart → MultiPart → BodyPart, and fan-out 10 (PageView, EmptyPageView). *Failure:* resolveRef('PageView', 12)branches multiplicatively, blows pastmaxOldGenerationSizeMb: 256, and the worker is killed. The user sees an opaque sandbox death rather than "depth too large", and the accumulated logsdie with it. TheformatSandboxResult25k cap does not help: it applies to the result string after the graph is already built in worker memory. *Fix:* clampdepth (Math.min(depth, 5)), carry a Set` of names along the expansion path, and add a node-count budget that throws a clear error.

---

3. await vs promise

Scope swept: all of mcp-server/src, the a11y stores and services, and libs/sdk/ai/src. Nine real await-loops, eight of them in assets-transfer.ts. Zero in page-create.ts, page-place-content.ts or page-verify.ts (single-request, or resolve-then-one-POST). Zero in the a11y stores, which are observable-based, so their concurrency defects are the overlapping-request ones below rather than loop fan-out.

Promise.all is not the answer for any of the nine, for two independent reasons. It fails fast and discards settled siblings, and in this file the settled siblings are the manifest (files[], failures[], notLive[]), so it converts "wrong report" into "no report", the same cliff as the unguarded verifyLive in §2. And it does not bound fan-out.

#	Site	Iterates	Committed state on mid-loop failure	Class
1	assets-transfer.ts:112	remote assets to local disk	Yes (files written), but download at :80-90 already try/catches per item	3, bounded concurrency
2	assets-transfer.ts:207	local files to dotCMS uploads	Yes (assets created); per-item try/catch at :208-223	2, sequential
3	assets-transfer.ts:252	_search pages	No (precedes writes)	Inherently sequential (offset depends on the prior page); its problem is the missing bounds
4	assets-transfer.ts:356	up to 3 verify rounds	n/a	Inherently sequential (round N+1 consumes round N's notLive)
5	assets-transfer.ts:359	pending to isLive GETs	No (pure reads)	1, allSettled
6	assets-transfer.ts:369	notLive to PUBLISH fires	Yes (publishes fired)	2, sequential
7	assets-transfer.ts:384	pending to final isLive GETs	No (pure reads)	1, allSettled
8	assets-transfer.ts:419	local dir walk (readdir recursion)	No (pre-write)	Leave sequential: local FS, parallel recursion risks EMFILE for no gain
9	sdk/ai/src/adapter/request-core.ts:374	formData fields	No (throws before fetch)	Leave sequential: 1 to 3 fields, and form.append order should stay deterministic

Category 1, allSettled (#5, #7). Pure GETs on distinct identifiers with no interdependence. Cost today for a 120-file theme: up to 3 × 120 sequential isLive GETs plus the 120 of the final pass, so up to 480 sequential round trips, of which only the last round's results matter. Largest latency win in the file, and allSettled also supplies the per-item isolation these two currently lack, since a rejected read today throws out of verifyLive entirely.

Category 2, sequential plus per-iteration try/catch (#6, #2). #6 fires workflow actions against content dotCMS is concurrently versioning and indexing, so keep it sequential. The per-item reporting is the fix, not the concurrency: a per-item failure becomes a per-item manifest entry instead of a thrown exception. #2 is the same category less obviously: uploading to /api/v2/assets implicitly autocreates the parent folder tree, so two in-flight uploads sharing a not-yet-existing parent race on folder autocreation. Only safe if grouped by folder or with folders pre-created, which is not worth it.

Category 3, bounded concurrency (#1). Genuinely independent, but the array is unbounded (enumerateAssets has no cap, and /application can enumerate tens of thousands). allSettled over that is one simultaneous GET and one open file handle per asset, so socket exhaustion and EMFILE. A realistic theme is 100 to 500 files, so a cap of 5 to 8 captures nearly all the win.

No helper exists. Searched core-web/libs, core-web/apps and the package manifests for p-limit, p-queue, p-map, promise-pool, mapLimit, parallelLimit, withConcurrency, and any exported chunk/batch/pool util: zero hits, and no such dependency declared. So this is a roughly 12-line local helper (preferably in lib/runtime.ts, which already owns cross-tool plumbing) or a new dependency. If you write one, note on the line that its inner Promise.all is safe only because each worker catches everything and never rejects, otherwise someone will later "simplify" it into the fail-fast form this whole inventory argues against.

Related concurrency defects outside the loops

mcp-server/src/lib/assets-transfer.ts:216 (code at :252) — MEDIUM. Unbounded pagination, including a path that never terminates.
No max-pages guard, no max-assets guard, no "offset made no progress" check; the only exit is :279.
Scenario A: download_assets on /application of a large site returns 40k assets, all accumulated into assets[] then downloaded one at a time, with no cap on either.
Scenario B: if the _search body's offset is ignored or clamped by the backend, page.length is always SEARCH_LIMIT, the exit never fires, seen de-dupes so assets stops growing, and the loop spins forever issuing identical POSTs. Combined with the missing request timeout from §1, nothing ever breaks the cycle: the MCP call hangs indefinitely and the instance takes sustained identical-query load.
Fix: cap total assets and pages, and break when an iteration adds no new identifiers to seen. That one check kills scenario B regardless of backend behaviour.

**.../a11y/store/a11y-page-list.store.ts:127 — MEDIUM. Overlapping searches are never cancelled.** No SubscriptionSlot, no switchMap, no takeUntilDestroyed, no onDestroy. *Failure:* type "blog" then click page 2. Two _searchPOSTs; if page 1 resolves last it overwritespagesandtotalRecordsand setsloadedwhile the paginator shows page 2, so table and paginator disagree and a row click opens a page the user did not select. It also leaks: navigate away mid-search andpatchStateruns against a destroyed store. *Fix:*switchMap, or the SubscriptionSlotthe run store already uses, pluswithHooks({onDestroy}), mirroring a11y-run.store.ts:691-693`.

**.../a11y/a11y-diff/a11y-diff.component.ts:114 — MEDIUM. The diff refetches per SSE frame with no supersession.** Keyed on previewRevision(), which is bumped on **every** progress frame (a11y-run.store.ts:410-413). loadDiff() (:131-157) has takeUntilDestroyedbut nothing supersedes the previous load, andp-accordion-contentusesmountOnEnter=falseso it refetches even while collapsed. Per frame that is one_render-sourcescall plus two per source file, so dozens of overlapping requests per frame. *Failure:* a stale response writes the olderfiles.set()andchangedCount.emit(), so the Files badge goes stale and, because the Publish bar is gated on changedFileCount, hasChangedFiles()` can flip back to false after the agent wrote files, blocking publish until request ordering happens to favour it.

.../a11y/store/a11y-run.store.ts:392 (called from :595) — LOW-MEDIUM. A full axe rescan fires on every progress frame.
SubscriptionSlot.set aborts only the client HTTP request; the backend headless render already launched and keeps running. A chatty agent emitting progress every few seconds queues many concurrent full-page renders per run.
Shared root cause with the finding above: previewRevision is too hot to be a refetch key. Debounce it, or rescan only on terminal and rate-limited frames.

---

4. Type duplication

**.../a11y/models/a11y-groups.ts:13 — MEDIUM, the one to fix first. Three types and two functions copied from edit-ema, and the copy has already drifted.** A11yFindingType :13, A11yGroupItem :16, A11yGroup :24, buildA11yGroups :39andmapRules :50mirroredit-ema/.../dot-page-scanner-report/models.ts:9/:14/:24anddot-page-scanner-a11y-report.component.ts:48/:61. The header at :5-7is explicit that this is a replication because edit-ema'smodels.tsis not exported. *The drift is already here:*A11yGroup.impactisAxeImpactin edit-ema butAxeImpact | nullhere, andAxeImpactalready includesnull, so the | nullis redundant and signals the author was unsure which contract was authoritative. Same redundancy atmodels/a11y-severity.ts:45. *It is already costing you:* the target.joinbug (§5) exists in **both** copies and has to be fixed twice. *Fix:* the blocker named in that header comment is a one-line change. Addexport * from './lib/dot-page-scanner-report/models';toedit-ema/ui/src/index.ts(precedent atindex.ts:15). This file already imports AxeImpact, AxeRuleandPageScannerA11yResponsefrom@dotcms/portlets/dot-ema/uion line 1, so **no new dependency edge is created**. Then delete the local types, movebuildA11yGroupsandmapRules` next to the shared models, and have the edit-ema component call the shared function.

**mcp-server/src/lib/page-create.ts:194, :201, :187 — three local types duplicate types already in scope.** ContentTypeSummaryis re-exported from@dotcms/ai/runtime (sdk/ai/src/runtime.ts:233-240), **the module this file already imports on line 1**, and it is the actual type of the context.contentTypesthe file already consumes at:248, :263and:270. So the file reads a ContentTypeSummary`, then hand-copies three of its six fields into a parallel local interface.

type ContentTypeDefinition = Pick<ContentTypeSummary, 'id' | 'variable' | 'baseType'> & { fields: ContentTypeField[] };
type PageEntity = Partial<Pick<DotCMSBasicContentlet, 'identifier' | 'inode' | 'live'>> & { contentlets?: PageEntity[] };

Pick rather than extends on purpose: fetchContentTypeDefinition never populates name, so inheriting it would be a lie. DotCMSBasicContentlet lives at sdk/types/src/lib/page/public.ts:353, and @dotcms/types is a pure-type entrypoint, so it resolves at zero runtime cost.
For the field half, DotCMSContentTypeField (dotcms-models/src/lib/dot-content-types.model.ts:540) already has all four fields: Partial<Pick<DotCMSContentTypeField, 'variable'|'required'|'fixed'|'defaultValue'>>. Caveat before wiring it: the @dotcms/dotcms-models barrel pulls Angular in transitively (lib/dot-action-menu-item.model.ts), which you do not want in the MCP server, so deep-import that one model file or skip this half and take the ContentTypeSummary win, which needs nothing new.
Why this matters beyond tidiness: these are response shapes for endpoints the repo already types. A hand-written local copy cannot drift-check against the server contract, so an upstream type change fails silently here instead of at compile time.

**mcp-server/src/lib/page-verify.ts:155 and page-place-content.ts:302 — duplicated inside the PR, and both restate types @dotcms/typesships.**LayoutRowis byte-identical in both files.isLive is implemented twice (page-create.ts:510, assets-transfer.ts:393): two implementations of one liveness probe, same endpoint and same ?depth=0, with two separately hand-written response shapes. The envelopes RenderResponse (:139) and PageJsonResponse (page-place-content.ts:290) overlap, and their container maps (RenderedContainer:148versusRawContainer:298) differ only by inode. That difference is the divergence already starting: two tools parse the **same** /api/v1/page/jsonresponse through two independently maintained shapes. Canonical types, zero runtime cost:DotCMSPageAsset (sdk/types/src/lib/page/public.ts:41), DotPageAssetLayoutRow :81, DotPageAssetLayoutColumn :134, DotCMSColumnContainer :153, DotCMSPage :517, DotCMSLayout :639. *Recommendation:* do **not** swap in DotCMSPageAsset at full strictness. The local shapes make everything optional deliberately, because the payload is unproven at compile time, and that defensiveness is correct. Keep the optionality but derive it (DeepPartial), at minimum reuse Pick<DotCMSColumnContainer, 'identifier'|'uuid'>for the innermost element, and hoist the oneLayoutRow` into a shared module so the two files cannot diverge.

*.../a11y/services/dot-page-sources.service.ts:174 — MEDIUM. Claims to mirror getFileVersionbut inverts its precedence.** The docblock at:172says *"MirrorsgetFileVersionin@dotcms/utils`." It does not:

// libs/utils/src/lib/shared/contentlet.utils.ts:29
contentlet['assetVersion'] || contentlet['fileAssetVersion'] || null
// here, :176-179
contentlet['fileAssetVersion'] || contentlet['assetVersion'] || contentlet['fileAsset'] || null

They agree when only one key is set, which is why this passes casually, and disagree when both are set. Then the diff viewer fetches a different version of the file than every other admin surface, so a line the user sees in the editor is missing from the diff for no discoverable reason. That is a bad failure mode in the one panel whose job is to be the trustworthy account of what the agent changed before publish.
Fix: import and use getFileVersion. If the extra fileAsset fallback is genuinely needed, add it to the util so both callers share one fixed precedence. A local helper whose comment says "mirrors X" is the thing most likely to silently stop mirroring X.

**.../a11y/store/a11y-page-list.store.ts:112 — LOW-MEDIUM. The _searchenvelope is redeclared inline.**get<{ jsonObjectView: { contentlets: DotCMSContentlet[] }; resultsSize: number }>restates a shape the repo declares twice:ESContent (dotcms-models/src/lib/dot-es-content.model.ts:3, exported from @dotcms/dotcms-models, and already used exactly this way at edit-ema/.../palette/utils/index.ts:304) and DotContentSearchResponse (data-access/.../dot-content-search.service.ts:33, declared on the very service being injected here). This store already imports DotCMSContentlet, so get({...})needs no new dependency. The inline copy omitscontentTookandqueryTook`, so wanting query timing later means a second partial copy rather than widening one shared type.

**mcp-server/src/lib/page-create.ts:504 and :507 — the ascasts defeat the narrowing this file was rewritten to use.** Correction to an earlier reply on this file: four casts remain at HEAD, not zero.:456and:462are fine, being the insides ofresponseEntityandasRecord, and the docblock at :444-450justifies them well. But:504 (asRecord(contentlets[0]) as PageEntity | undefined) and :507 (entity as PageEntity) assert that identifier, inodeandliveexist with the right types off aRecord<string, unknown>when nothing checked them. That is precisely the "type-check a lie, letundefinedsurface in the caller" failure the:444-450docblock argues against, soextractPageEntityre-opens the holeresponseEntity/asRecord/optionalStringwere added to close. *Fix:* read the fields withoptionalString(entity, 'identifier')and an equivalentoptionalBooleanforlive, returning a real PageEntitybuilt from checked values.:504is the same fix applied tocontentlets[0]. The same unchecked-envelope pattern still lives in page-verify.ts:383 (as RenderResponse), page-place-content.ts:329 (as PageJsonResponse), and assets-transfer.ts:326, :397, :712, :729. :729is the sharpest:candidate as AssetContentlet[]asserts the *element* type after only anArray.isArray` check.

Smaller type nits:

• dot-a11y-agent.service.ts:84,86: payload as {report?: FixReport} then payload as FixReport, with no runtime check. A 'report' in payload guard makes it honest, and this is the enabler for the a11y-run.store.ts:203 crash in §2.
• dot-page-sources.service.ts:213: (view.theme as ThemeSourceView | undefined) is a no-op cast, since view.theme is already non-optional (page-render-sources.models.ts:80). Either the model field should be optional or the cast should go.
• dot-velocity-playground.utils.ts:182: parsed as VelocityWarning[] off a JSON.parse of the X-Dot-Velocity-Warnings header, guarded only by Array.isArray; element shape unvalidated.
• a11y-diff-viewer.component.ts:180 plus :159/:162/:166/:171: hand-rolled Monaco interfaces behind window as unknown as .... Precedent-consistent with dot-velocity-playground/.../register-velocity.ts:1, but monaco-editor ships IStandaloneDiffEditor and ITextModel and both can be import type-d. Nit.
• StudioPageRow (accessibility-studio.models.ts:84) is a deliberate view model, but three fields are pure renames of DotCMSContentlet (path←url, type←contentType, hostId←host) that cost a mapper and buy nothing, and hostId/hostName sitting adjacent invites the host versus hostName mix-up the source type already disambiguates.

---

5. Other frontend defects

Outside the four themes, but found along the way and worth fixing.

.../a11y/store/a11y-run.store.ts:665 and :677 — HIGH. publish() and discard() only patchState; no backend call exists anywhere in the feature.
Failure: the user scans, the agent fixes the working version, they click "Apply these changes" (a11y-run.component.ts:725). The UI moves to the published phase and reports success, but the working version is never promoted, so the live page keeps every violation while the user believes it is fixed. discard() is worse than a no-op: it advertises a revert, but the working version still holds the agent's edits, so the next editor to publish that page ships changes the user explicitly discarded.
Fix: wire these to the real publish or version-revert endpoints, or disable the buttons and state that promotion is manual. As written, the feature's primary call to action does not do what it says.

**edit-ema/ui/.../dot-page-scanner-report/dot-page-scanner.service.ts:121 — HIGH. A dev-only hack ships in a lib UVE loads in production.** scanUrl.replace('4200', '8080')runs unconditionally, under a comment block saying it must be reverted before merge.String.prototype.replacewith a string pattern rewrites the first occurrence anywhere, not just a port:/products/4200-seriesbecomes/products/8080-series, ?id=a4200f31-...becomes?id=a8080f31-..., and a real host on port 4200 is silently retargeted. The results look legitimate. *Note:* this is the missed sibling of a hack already fixed in a11y-run.store.ts, which now parses the URL and compares url.port === DEV_SERVER_PORTwith a comment naming thedev4200.example.comfalse positive. The fix landed in one of the two places. *Fix:* passscanUrl` through and let the caller or the proxy config resolve the env-aware URL, as the comment itself suggests.

.../a11y/a11y-run/a11y-run.component.ts:311 — HIGH. "Back to page list" is a no-op on multi-segment paths.** The run route is path: '' (a11y.routes.ts:23), so route.snapshot.urlholds all remaining segments, and Angular's..removes one URL **segment**, not one route level. From/agents/a11y/about-us/indexthe back arrow yields/agents/a11y/about-us, which matches again.data: { reuseRoute: false }is **not** inherited by thesnapshot (itsrouteConfig.path !== '', and the parent route has a component so getInheriteddoes not merge parent data), soshouldReuseRoutereturns true, the component instance is kept, and the screen does not change. *Same defect on cold load:* a run URL opened in a new tab has nohistory.state.row (:227), so the constructor calls toPageList(), stays on **, and renders an empty studio with selected === nullwhere Scan silently no-ops. *Fix:*this.router.navigate(['/agents/a11y']). Also handle the returned promise; this and a11y-page-list.component.ts:81` both discard it.

**apps/dotcms-ui/src/app/app.routes.ts:193 — MEDIUM. agentsis the only lazy portlet route with noMenuGuardService.** Answering the question on the old thread: authentication is **not** bypassed, portlet authorization is. MenuGuardService enforces that the portlet is in the user's layout and granted to their role, and every sibling carries it (pages/edit-page, content-types-angular, forms, templates, containers, categories, apps, personas). *Failure:* any authenticated backend user, including a role never granted Accessibility Studio, types /agents/a11y` and gets the full portlet with scan and fix actions. Removing it from a role's layout hides the menu entry but does not block URL entry.
Caveat: the backend half of this needs a backend reviewer, so treat the guard as necessary but not sufficient on its own.

**mcp-server/src/tools/page_place_content.ts:71 — MEDIUM. languageIdis bounds-checked but never validated against the languages already cached.**int().positive()is the only check (same attools/page_create.ts:37, tools/page_verify.ts:23) even though dotcms.loadContext()already returnslanguages, and dotCMS silently falls back to the default language for an unknown id. *Failure:* page_place_content({path:'/about-us', languageId: 12, slots:[...spanish contentlet ids]})where language 12 does not exist.loadPageSlots (lib/page-place-content.ts:326) reads the **English** page, the merged container map is computed from English, and the POST writes to **English**, replacing its content with Spanish contentlets. The manifest returns languageId: 12andwarnings: [], so the model reports "placed on the Spanish page" while English is what changed. *Fix:* resolve languageIdagainstloadContext().languagesbefore any request and fail with the available list, the waysiteandcontentTypealready are. If a fallback is ever intentional, the manifest must report the language actually written. This is the same family as the hardcoded?? 1default atpage-create.ts:151`, and the two compound.

**mcp-server/src/lib/page-place-content.ts:278 — MEDIUM. Bidirectional substring container matching resolves to the wrong container.** containerMatchesaccepts exact, suffix, and "either string contains the other";findContainer (:401) returns the first hit while iterating the layout's container map. *Failure:* a layout has both .../containers/default/and.../containers/default-banner/. A caller asks for .../default/; if default-banneris iterated first, the containment branch matches it and wins. Inmergemode the tool reads the banner's contentlet list and writes the merged result back under it, so content lands in the wrong container and the banner's own contents can be replaced. Object iteration is insertion order, so which container wins depends on layout authoring order, making it non-deterministic from the caller's side. *Fix:* collect all matches, prefer exact then suffix on a/` boundary, and fail loudly with the candidate list when more than one still matches.

**mcp-server/src/lib/page-verify.ts:346 — MEDIUM. A clean bill of health for a page with zero slots.** collectWarnings (:274) iterates slots, so []yieldswarnings: [], and this line interpolates slots.lengthinto a success sentence. Legacy or advanced templates, and any response whereentity.layout.body.rowsis absent whileentity.containersis populated, giveslots: [] (:180-191). With a non-empty page.rendered: "Page rendered successfully in LIVE mode, all 0 slot(s) produced content."Every slot could be blank and the tool built to catch blank slots declares success. *Mirror image:*resolveSlot (lib/page-place-content.ts:232-237) throws The page has 0 slot(s): (none)for that same page, which reads as "no containers" rather than "the layout could not be parsed". *Fix:* a distinct branch forslots.length === 0`.

**mcp-server/src/lib/page-create.ts:147 — worth a look, with a caveat. extraFieldsis spread into the fire body with no key deny-list.** The comment at:145-146promises the typed fields win on "the keys they own".identifierandinodeare not among them, andassertRequiredFieldsSatisfied (:310) only validates *missing* values, never unexpected keys. On the evidence I could read, MapToContentletPopulator.processIdentifier (dotCMS/src/main/java/com/dotcms/rest/MapToContentletPopulator.java:1014-1026) treats an identifier in the map as "load the existing contentlet, copy properties onto the incoming one, blank the inode", which would make the PUBLISH fire save a new live version of that existing content. **That backend half needs a backend reviewer to confirm**, so take it as a question rather than a claim. The TypeScript-side fix stands regardless of the answer: reject reserved keys outright rather than relying on spread order (identifier, inode, contentType, stInode, live, working, deleted, modUser, owner). Note also that destructiveHint: false (tools/page_create.ts:74`) means the MCP client will not prompt the user.

**mcp-server/src/lib/page-place-content.ts:321 — LOW-MEDIUM, latent. Unencoded path interpolation allows ..retargeting.** Here and at:327(alsopage-verify.ts:114-116,381) uriis only leading-slash-normalised then interpolated into ``/api/v1/page/json${uri}``.requestCore policy-checks the **raw** string (sdk/ai/src/adapter/request-core.ts:328) and normalises afterwards (:351), so /../../../../api/v1/users/currentwould pass an/api/v1/page/prefix allowlist and then resolve elsewhere. **Latent, not live:** noallow policy is configured today (lib/runtime.ts:13-22), but it becomes a real bypass the moment one is added, which is the natural next hardening step for this tool surface. Already wrong today with no policy involved: page_verify({path:'/a/../b'})renders/bwhile the manifest reports/a/../b, and a #silently truncates. *Fix:* reusesplitUrlPath (lib/page-create.ts:365-403`), which already does this correctly in this same PR, and report the normalised path.

*.../a11y/store/a11y-page-list.store.ts:44 — MEDIUM. No languageIdclause, so multilingual sites list every page twice.**buildPagesQueryemits+working:true +(urlmap: OR basetype:5) +deleted:false +conhost:. toPageRowcarrieslanguageId but the table renders only title, path, type, status and date (a11y-page-list.component.html:63-110). On demo (en-us plus es) every page appears twice, identically, the "N of M" count doubles, and which row the user clicks is arbitrary, so someone fixing the English page can silently scan and fix the Spanish one. openSelectedPage (a11y-run.store.ts:443-448) dedupes on identifier` only, compounding it.

**.../a11y/store/a11y-page-list.store.ts:55 — MEDIUM. The Lucene escape does not handle whitespace.** q.replace(/[+-&|!(){}[]^"~?:\/]/g, '\$&') (:54) escapes metacharacters but leaves spaces. Searching about usproduces+(title:about OR path:about us OR urlmap:about us): the whitespace terminates the field-qualified term, so us*becomes a bare token against the default field inside theORgroup. The picker returns any content whose default field starts withus(users, USA, usage), and the intendedpath/urlmap` wildcard is never applied to the full phrase.

**.../a11y/models/a11y-groups.ts:59 — LOW-MEDIUM. The axe targetarray is joined into a selector list.**node.target?.join(', ')builds a CSS selector *list* while the comment says "First CSS selector". axe'stargetis an ancestor path for frames and shadow roots, so['iframe#promo','button.cta']makesquerySelectorreturn theiframe` and the overlay outlines the whole embed instead of the button.
Fix: take the last element. Present in the edit-ema copy too, so fix both (see §4).

**libs/ai-ui/.../dot-agent-activity-log/dot-agent-activity-log.component.ts:85 — MEDIUM. Force-scroll fights the user.** The afterRenderEffecttracksworkingText(), which changes every 5 seconds during a run (a11y-run.component.ts:569-581), then sets scroller.scrollTop = scroller.scrollHeight`. The scroller is the nearest scrollable ancestor, which in the studio is the whole scanner pane containing the donut, legend and issue list. Scroll up to read the score ring mid-run and it snaps back about 5 seconds later, for the entire run.
Fix: only auto-scroll when already pinned to the bottom, measured before the write, and do not treat a cosmetic reassurance string as a scroll trigger.

**mcp-server/src/lib/assets-transfer.ts:337 — LOW. The 0-byte fallback uploads a 1-byte file and reports success.** Buffer.from('\n')` is uploaded instead of the empty file, and the manifest reports it uploaded with no warning. For an empty VTL or CSS partial the remote asset differs from the source and the caller cannot see it.

---

6. My earlier notes, resolved

Note	Resolution
assets-transfer.ts:360 and :374, "we should do Promise.all or any other thing, this is not handling errors per request"	Answered in §3. Promise.all is the wrong tool for both: allSettled for the liveness GETs (#5, #7), sequential plus per-iteration try/catch for the PUBLISH loop (#6).
assets-transfer.ts:185, "what happens if this fails?"	The upload loop at :207 does have a per-item try/catch at :208-223, so it is the better-behaved half. The unguarded paths are verifyLive at :227 and the four sites inside it.
assets-transfer.ts file-level, "double check the error handling and promise handling"	Covered by §1, §2 and the loop inventory. Also worth knowing: this file has zero tests for downloadAssets/uploadAssets, failures[], or the verifyLive throw path. assets-transfer.spec.ts covers only splitIncludePatterns and includeMatcher, so the file with the weakest error handling has no error-path coverage.
page-create.ts file-level, "we are awaiting without catching errors"	The specific damage is ensureFolder at :126, a committed write before the unguarded fire at :139. See §2.
page-create.ts:151, "the default language is not taken into account"	Confirmed, and worse than it looks: the default is hardcoded to ?? 1 and a wrong explicit languageId is silently redirected to the default, so the two compound. See tools/page_place_content.ts:71 in §5.
page-create.ts:206, "review this types, maybe we can reuse something"	Yes, three of them, and one is already imported into the file. See §4.
page-create.ts:227, "should we mark this error as [MCP Server - resolve site]: <error>"	Endorsed, and it should be owned by one helper rather than template strings per site. But formatting is the smaller half: see runtime.ts:26 in §1, where retryable needs to be a field on the result, since MCP hands the model a string and instanceof is unavailable on the far side.
page-create.ts:89, "check if we don't have anything similar on the repo"	Not fully answered. No equivalent helper was found for the folder-ensure path, but this deserves a targeted search before closing. Related and confirmed: splitUrlPath (:365-403) already exists in this file and should be reused by page-place-content.ts:321 and page-verify.ts:114-116 instead of string concatenation.
page-create.ts:375, "why underscore?"	Still open, author's call. No functional issue found; it reads as a convention not used elsewhere in the file.
dot-page-scanner.service.ts:121, "we can add this to the proxy"	Agreed as the direction: the env-aware URL belongs to the caller or the proxy config, not a blind replace in a shared lib. See §5.

---

7. Checked and verified clean

Stated so this reads as a review rather than only a defect list.

• Critical Rules: clean on the frontend side. No hardcoded secrets: greps for api[_-]?key|secret|password|token assigned to 16+ character literals across all changed non-lock files returned nothing. No pom.xml changed at all.
• Secrets in MCP errors: clean. AUTH_TOKEN is read only at lib/runtime.ts:15, execute.ts:125 and search.ts:84 and passed into createRuntime; the token is injected host-side at request-core.ts:363 and never interpolated into a message. No request body, header or JWT reaches a tool result.
• Thrown values: clean. All 34 MCP throw sites throw real Error instances, so no [object Object] reaches the model from that path. The three ad-hoc narrowings (page-place-content.ts:432, page-verify.ts:393,406) and runtime.ts:27 all guard with instanceof. (cause is threaded nowhere, which is the separate finding in §1.)
• Angular effect dependencies: clean. All 8 effect() and afterRenderEffect sites in the new code were checked for the read-after-guard pattern where a guard placed before a signal read silently drops that dependency.
• ALLOWED_PREFIXES allowlist: not a bug. spec-transform.ts:179 uses raw startsWith, which is not segment-aware, but computed against the committed spec (572 paths) it over-admits 0 paths. Latent robustness only, so I dropped it.
• resolveRef basename $ref resolution: not triggerable. nameOf resolves any $ref against components.schemas by basename, which would misresolve a #/components/parameters/X, but the spec uses only #/components/schemas/ (1527 refs) and no schema name collides with a non-schema component.
• matchesPattern (spec-transform.ts:84-95): correct. The **→.* then (?<!\.)\*→[^/]+ ordering is deliberate and the negative lookbehind correctly protects the .*.
• Worker logs cross-execution leakage: none. executor.ts:66-68 creates a sandbox per execute and disposes it in finally, so the module-level logs array cannot survive between runs.
• Pre-existing, not introduced here: in worker-harness.ts only resolveRef is new. The pick last-segment key collision, table's first-item-only keys, and the unguarded adapterMethods destructure in the init handler all predate this PR, so they should not be attributed to it.
• PageScannerA11yResponse reshape: no stale consumers left.

---

8. Overlap with @oidacra's review

Where we independently reached the same conclusion, treat it as two reviewers agreeing rather than one finding restated: a11y-run.store.ts:665 (publish and discard local only), a11y-run.store.ts:647 (stopAgent swallows failures), dot-page-scanner.service.ts:121 (the 4200 hack), app.routes.ts:189/193 (the missing guard), and a11y-groups.ts:59 (target.join).

Two of @oidacra's findings are not duplicated above and should be worked from their comments: a11y-run.store.ts:619 (the error case not bumping previewRevision the way done and aborted do) and a11y-run.store.ts:105 (the initialState and teardown interaction).

---

Posted by Claude Code on behalf of @zJaaal. Consolidated from 45 inline threads plus 11 shorter notes, removed to keep the PR readable; the analysis is Claude Code's, the account is the one holding the token.

[fmontes]

Fixing:

• @claude: comment — Claude finished @fmontes's task in 5m 54s** —— [View job](https://githu…
• @fmontes: comment — We need to find a way so the build don't breaks because we can't genera…
• @copilot-pull-request-reviewer: review summary — Copilot wasn't able to review this pull request because it exceeds the…
• @fmontes: comment — Fixing:
• @nicobytes: review summary — A couple of Angular 22 hygiene notes from review (no blockers assumed —…
• @zJaaal: comment — Consolidated review: frontend and MCP server

Will be in the next commit.

[fmontes]

📌 Guide for Java reviewers

This PR is big (163 files), but only 34 are Java/backend — everything else is frontend, SDK, and MCP server. This comment is the map for the backend half so you don't have to read the whole diff.

Why any of this exists: we drove dotCMS's own REST API with AI agents for several build sessions. Every change below traces to a specific, reproducible failure — an undocumented default, a silent zero-result, or a 500 where a 400 belonged. Nothing here is speculative polish.

---

What changed

Group	Files	Risk
@Schema / @Operation docs — documenting real traps (unqualified Lucene fields silently match nothing; host-scoping; PUT vs POST fire envelopes; theme is a folder id not a path)	~15	🟢 None — annotations only
Short clazz names — clazz: "CONTENT" / "TEXT" instead of ImmutableSimpleContentType	ContentType, Field, LegacyFieldTypes + 2 tests	🔴 Highest — see #1
A11y agent SSE proxy (new)	rest/api/v1/a11yagent/*	🟠 See #2
VTL /dynamic structured errors + warnings	VTLResource + 4 new views + 1 test	🟠 See #3
500 → 400 on write endpoints	UnrecognizedPropertyExceptionMapper, TemplateResource	🟠 See #4
openapi.yaml	1	🟢 Generated — please skip. CI verifies it matches the build
Language.properties (+51 keys)	1	🟢 None

---

🔍 Where I need your eyes

1. ContentType.typeFromId / Field.typeFromId — biggest blast radius

This is the Jackson chokepoint every clazz value flows through: REST, bundle import, and push publish. That's why it was the right place to fix, and why it's the riskiest thing here.

FQCN input short-circuits before the new code, so normal internal traffic is untouched — please confirm that reading. The concern is the new path:

• ContentType uses Try.of(() -> BaseContentType.getBaseContentType(id)), and that method throws IllegalArgumentException + logs at INFO on every miss. The still-supported simple-name form ("WidgetContentType") always misses → one thrown exception + one INFO log line per object. A push-publish bundle or CT import with N types produces N of each.
• Field.typeFromId does the same job with Optional + no exception. The two siblings use different patterns; the Field one looks correct. Worth aligning?

2. A11yAgentResource.mintShortLivedToken — most security-sensitive code in the PR

It calls persistApiToken(...) — a DB row + a full-permission user JWT — and hands it to an external service. Three questions:

• Scope: the token carries the user's full dotCMS rights for 5 min (DOT_PAGE_SCANNER_TOKEN_TTL_MS). Acceptable, or does it need narrowing?
• Churn: minted on every call including GET /active-run, which the UI polls. Never revoked after use. Is per-poll token-table growth OK?
• IP binding: requestingIp is set to the browser's getRemoteAddr(), but the token is used by the agent service from a different egress IP. If that field is enforced at validation, does this actually work in prod behind a proxy?

Also: buildAgentPayload hand-rolls JSON via a jsonString() helper that escapes \ " \n \r \t but not the rest of the U+0000–U+001F range (\b, \f, …), so an odd char in a page title or URI would yield invalid JSON. Should this just use Jackson?

Last: there's a package-private test constructor A11yAgentResource(WebResource, HttpClient) but no test — while sibling PageScannerResource has PageScannerResourceTest. I'd like a second opinion on the minimum coverage bar here.

(Credit where due: the SSE relay uses Thread.ofVirtual() + finally { output.close() }, so it's blocking socket I/O off the container threads — that part matches our virtual-threads guidance.)

3. VTL /dynamic — contract change + shared-engine question

• /dynamic now returns 400 with a structured body where it previously returned a 500 or a partial 200. Intentional, but it is a breaking change for any client keyed on the old behavior.
• Warnings come from an EventCartridge attached to the per-evaluation Context. Please confirm it cannot leak into the shared global Velocity engine, and that the handler never substitutes a value (it shouldn't — every callback returns the default).
• A dynamic boolean was threaded through ~14 processRequest overloads to gate all of this. The convention-based .vtl-file endpoints must be byte-for-byte unchanged — that's the main thing to verify in the mechanical part of the diff.

4. UnrecognizedPropertyExceptionMapper — global @Provider

One edit changes the 400 message on every write endpoint. Two asks:

• Any Postman/integration test asserting the old raw Jackson text?
• It now lists all valid field names for the target form. Fine, or unwanted disclosure of DTO shape?

5. PageResource signature change (quick check)

loadJson, render, and renderHTMLOnly each gained a host_id param. The value was already honored from the request; the param makes it discoverable in the spec. These are public methods — any OSGi plugin binary-compatibility concern?

---

✅ Safe to skim

openapi.yaml (generated), all @Schema/@Operation text, Language.properties, and DotRestApplication (one new tag). The three new unit tests are DB-free.

Happy to split any of the above into its own PR if that makes review saner — say the word.

[claude]

Working on it

The trigger comment is fmontes's status note ("Fixing: ... Will be in the next commit"), which lists the outstanding review threads rather than a single explicit ask. I'm reading it as: implement the concrete, mechanical items from that list that don't require a product/architecture decision, and flag the rest.

Todo list

• Read trigger comment + gather context
• Investigate the Angular 22 hygiene items (@nicobytes): redundant standalone: true / OnPush, global vs globalThis in ai-ui/src/test-setup.ts
• Investigate the "build breaks because we can't generate the specs" issue (@fmontes, 2026-08-06)
• Implement fixes for what's safely mechanical
• Run relevant lint/build checks
• Commit + push
• Post summary of what was done vs. left for follow-up (zJaaal's consolidated review has many items needing fmontes's own judgment calls, not mechanical fixes)

[fmontes]

Quick note on the blocking items from @zJaaal's consolidated review and @claude's review — fixed in 535e2f6f2e.

dot-page-scanner.service.ts:121 — the 4200 → 8080 hack is gone, no substitute. Beyond the string-replace problem, the port was wrong anyway: proxy-dev.conf.mjs resolves the backend dynamically via DOT_BE_PORT, so 8080 was already incorrect in any dotwt worktree.

a11y-run.store.ts:578 — the fix stream now subscribes with a complete handler and falls back to scanned when the body ends without a terminal frame. Worth flagging one thing the review didn't catch: the error path completes too, via the EMPTY that catchError returns, so a naive complete handler overwrites the error it just set. There's a settled flag guarding that.

a11y-run.store.ts:647 — stopAgent() no longer swallows failures. Replied on that thread.

a11y-run.component.ts:311 — back button navigates absolutely now. Confirmed your read: .. drops one URL segment, so from /agents/a11y/about-us/index it landed on ** again and the screen never changed. The route constant lives in its own module because a11y.routes.ts imports the run component.

Also removed DotHttpErrorManagerService from the run store: scan, fix and stop failures now share one runError channel rendered at the top of the portlet, rather than a modal over a run in progress.

Test note: several existing mid-run assertions used of(...), which completes immediately and now reads as a dropped connection — they were asserting against a closed stream while their comments said "no terminal event → still fixing". Added an openStream() helper that emits and stays open.

Still open, not in this commit:

• a11y-run.store.ts:665/677 — publish/discard are still local-only. Needs a product call (wire the endpoint vs. disable and label promotion as manual), not a mechanical fix.
• app.routes.ts:189 — merging without the guard for now, answered on its thread.
• The rest of the consolidated review is triaged and queued; the next ones up are dot-page-sources.service.ts:165 (a 502 renders as a whole-file deletion in the diff), a11y-diff.component.ts:114 (a stale response can flip hasChangedFiles() false after the agent wrote files, blocking publish), and assets-transfer.ts:227.

[fmontes]

Second batch — ba8f66abbd. Ten findings from @zJaaal's consolidated review and @oidacra's threads; the six with inline threads are answered there, the rest below.

a11y-page-list.store.ts:44 — missing languageId clause. Confirmed. Query is now scoped to the instance default language, resolved once on init and gating the first load the same way currentSiteId already did. Note this is a real functional restriction: you can no longer reach a non-default translation from this screen. A language column plus a picker is the honest fix, and this list is being redesigned after merge, so it is deliberately deferred rather than missed.

a11y-page-list.store.ts:55 — Lucene escape and whitespace. Confirmed, and the failure is exactly as described: about us produced path:*about plus a bare us* against the default field. Spaces now collapse to ?, Lucene's single-character wildcard, keeping the phrase in one term. Test asserts the old broken form is gone rather than only that the new one is present.

dot-page-sources.service.ts:174 — getFileVersion precedence inversion. Confirmed and fixed by delegating to the util. Kept fileAsset as a local last resort since the util does not consider it, so there is now one precedence rather than two.

a11y-diff.component.ts:114 + a11y-run.store.ts:392 — refetch per progress frame. Fixed as one change at the consumer, rebuilt as toObservable → distinctUntilChanged → debounceTime(400) → switchMap. Worth separating the two halves: the debounce fixes the request volume, but the switchMap is what actually fixes the bug you flagged — nothing previously superseded an in-flight load, so a slow early response could land after a fast later one and flip hasChangedFiles() back to false after the agent had written files, blocking publish.

Left previewRevision itself hot, since the preview iframe wants frequent reloads during a run and only the diff panel needed to be insulated.

---

Test notes, since two of these changed how the suites work:

• The diff panel's ten tests were synchronous and now drive the debounce through fake timers via a flushReload() helper.
• Default-language selection is tested through an extracted pure function rather than a second store: Spectator's mockProvider shares one mock instance across createService() calls, so a second store in the same test never re-fetched. The extraction also makes "pick the flagged default, not merely the first returned" explicit in the code.
• The report narrowing is covered at the service, not the store — the store spec mocks DotA11yAgentService, so it cannot exercise it.

188 tests in dot-agents (was 169), 767 in data-access, 338 in edit-ema-ui. Lint clean.

Still open: a11y-run.store.ts:665/677 (publish/discard local-only) — unchanged, still needs a product call rather than a mechanical fix. Next up from the triage are the MCP-side items, starting with assets-transfer.ts:227.

[fmontes]

Third batch — 09ee788fb7. Frontend UI findings; each thread is answered inline. Two things worth raising here rather than in a thread.

⚠️ text-color-secondary does not exist — and it is used well beyond this PR

Thread #61 suggested matching dot-agents-landing.component.html, which uses text-color / text-color-secondary. Half of that is right: text-color is real. text-color-secondary is not.

It is a PrimeFlex class, and PrimeFlex is no longer in package.json. The tailwindcss-primeui plugin we do use exposes only:

.text-color   .text-color-emphasis   .text-muted-color   .text-muted-color-emphasis

So text-color-secondary resolves to nothing and the text falls back to inherited color — the same silent no-op as the hover:shadow-4 in thread #57, in the same file. The suggested exemplar was itself broken.

Everything in this PR now uses text-muted-color, including the two landing-page usages.

Outside this PR it is still live, in at least dot-users, dot-query-tool, dot-analytics, edit-ema's favorite selector, and others — all rendering with unintended color today. That is a repo-wide sweep, not something to fold in here. Happy to file an issue if nobody has.

target[0] vs target.at(-1)

@oidacra suggested node.target?.[0]; @zJaaal suggested the last entry. Went with last, because target is ordered outermost-first: for ['iframe#promo','button.cta'], [0] is the iframe — precisely the wrong element the finding is about. The A11yGroupItem.selector doc comment saying "First CSS selector" was the thing that made [0] look right, so it is corrected too.

---

Also in this batch: the activity log now auto-scrolls only when the user is already at the bottom and no longer treats the heartbeat text as a scroll trigger; the !p-0 overrides are on the Tailwind 4 suffix form with inline justifications; the velocity playground badges are p-tag; test-setup.ts uses globalThis; and the a11y.routes.ts comment now describes the actual bounce behaviour.

New a11y-groups.spec.ts covers the selector chain. Test note: the activity-log scroll tests need ApplicationRef.tick() (detectChanges() does not run afterRenderEffect) plus a getComputedStyle stub — jsdom does no layout, so it reports overflowY as undefined and the component's scroller walk finds nothing. Without both, the "leaves the scroll alone" assertions pass vacuously.

193 tests in dot-agents, 21 ai-ui, 119 velocity playground, 338 edit-ema-ui, 767 data-access. Lint clean.

Still open: a11y-run.store.ts:665/677 — publish/discard remain local-only, still the one item needing a product decision rather than a fix. Remaining triaged work is MCP-server side, starting with assets-transfer.ts:227.

[jcastro-dotcms]

I think there are Swagger annotations we can put here. You can refer to com.dotcms.rest.api.v1.system.permission.SaveUserPermissionsForm for more details.

[jcastro-dotcms]

I think we've been using immutables for these kinds of Form classes. You can refer to com.dotcms.rest.api.v1.drive.AbstractDriveRequestForm for more info.

[jcastro-dotcms]

Include the Template's title and ID in here so that Support can figure out what Template is failing more easily.

[jcastro-dotcms]

NICE!

[fmontes]

Final batch — cf139e1313. The 🟠 list from @zJaaal's consolidated review is now complete (28/28).

Path interpolation — page-place-content.ts:321, page-verify.ts:114

Fixed, but not quite as suggested. The proposal was to reuse splitUrlPath; its folder-vs-leaf behaviour is page_create-specific, though — verifying /about-us must render /about-us, not /about-us/index. So the normalization is now shared in lib/page-path.ts and splitUrlPath keeps its own output shape on top of it.

Both halves of the finding confirmed: page_verify({path:'/a/../b'}) rendered /b while the manifest reported /a/../b, and a # truncated silently. The latent half stands too — requestCore policy-checks the raw string and normalizes afterwards, so /../../../../api/v1/users/current would have passed an /api/v1/page/ prefix allowlist.

Writing the tests surfaced a third bug neither of us had listed: a leading // is scheme-relative to the URL API, so //books/index parsed books as a host and yielded /index. Answered on @oidacra's splitUrlPath thread; it affected both functions.

format-result.ts:41 — verified, not assumed

Worth recording the evidence, since "structured clone allows what JSON doesn't" is easy to assert and easy to get wrong. Run against the real executor:

CIRCULAR success: true   CYCLE PRESERVED: true
BIGINT   success: true   BIGINT typeof: bigint

The worker reports success, the cycle survives the boundary intact (children[0].parent === root on the host side), and JSON.stringify then throws out of the function whose job is to report that result. Logs were attached after the stringify, so they died with it.

The trigger is ordinary rather than exotic: building a tree from a flat folder or page list with parent back-pointers. Logs are built first now, and serialization goes through a WeakSet replacer inside a try/catch.

worker-harness.ts:96 — three bounds, not one

Confirmed the spec is not acyclic: self-refs on FolderView/MultiPart/Permissionable, the BodyPart → MultiPart → BodyPart cycle, fan-out ~10 on PageView. depth is model-chosen and each hop copies the whole target subtree with no memo, so resolveRef('PageView', 12) exhausted the heap and the worker was killed — an opaque sandbox death with the logs lost. Added a depth clamp, a visited set along each branch's ancestry, and a node budget. The clamp is self-evident in the output, since an unexpanded { $ref } is the same progressive-disclosure signal the model already follows.

---

🔴 a11y-run.store.ts:665/677 — publish/discard: merging as-is, deliberately

Flagging clearly so this does not read as an oversight. @fmontes's call: the user flow and design around apply/discard need rework first, so the buttons ship in their current local-only form and the behaviour is addressed with that redesign rather than patched here. The finding is correct and is not being disputed — it is sequenced behind the design work.

Scoreboard

🔴 Blockers 5/6 (the sixth above, by decision) · 🟠 Fix 28/28 · 🟡 Discuss 0/11, all pending @fmontes's calls on conventions, type duplication, test-coverage scope, and the Java block.

Across the six commits: 148 tests in mcp-server (from 77), 68 in sdk-ai, 193 in dot-agents. Lint and typecheck clean throughout.