Conversation
Create SafeUrlValidatorService for http/https validation. Create DomainOriginVerificationService for strict iframe networking validation.
Wire bypassSecurityTrustResourceUrl inside rendered-frame.ts through the rigorous URL validator. Swap the check in preview-bridge.ts for the strict origin verification. Fix LLM concurrency in ChatCoordinator submitPrompt. Fix Monaco model.dispose() in a2ui-composer-monaco-editor. Inject takeUntilDestroyed in api-key-selector.ts and renderer-selector.ts.
Delete debug/mock-rules/ entirely. Rip out console.error patch hacks from ChatCoordinator. Delete obsolete storage key ACTIVE_DRAFT.
…hot Capture and IDB Base
…ks into decoupled targets accurately abandoning monolithic arrays globally
…mentations and un-deferred webrtc layout code gracefully
…ngestion - Implement TrackEventDirective applying declarative tracking boundaries natively. - Construct FileIngestionService strictly leveraging @angular/cdk/clipboard utilities. - Construct ShareService cleanly encapsulating clipboard interactions.
- Sweep all active standalone UI components throughout the app, converting default ChangeDetection behavior to ChangeDetectionStrategy.OnPush. - Strip FileReader native accesses from ChatPanel into the FileIngestionService. - Swap template tracking to the directive.
- Rip out the legacy shareDesign() bloated component implementation comprehensively. - Delete deprecated inner test asserts and old inline code.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the A2UI Composer shell by modularizing several core features, including extracting payload parsing and syntax healing into a2ui-payload-parser, error formatting into ChatErrorFormatterService, file ingestion into FileIngestionService, and sharing capabilities into ShareService. It also introduces a base AbstractSelector for settings selectors, delegates startup state to StartupConfigStateService, and adds services for safe URL validation and screenshot capture. Feedback on these changes highlights several critical regressions and improvements: same-window messages should be allowed in preview-bridge.ts to avoid runtime regressions; the single JSON object fallback in a2ui-payload-parser.ts must be removed to comply with repository rules, while a nullish input guard should be added and the missing rules/mock* property filtering restored; the earlyMessageBuffer cleanup in host-communication.ts should be preserved to prevent memory leaks; and the gallery component should use @angular/cdk/clipboard for robust clipboard operations.
| if ( | ||
| !DomainOriginVerificationService.verifyStrictOrigin(event.origin, event.source, window.parent) | ||
| ) | ||
| return; |
There was a problem hiding this comment.
The new verifyStrictOrigin check only validates messages originating from window.parent. However, the original implementation explicitly allowed self-messages where event.source === window. Blocking these self-messages can cause runtime regressions if the preview bridge or its tests rely on same-window postMessage communication. Please preserve the original behavior by allowing same-window messages directly.
| if ( | |
| !DomainOriginVerificationService.verifyStrictOrigin(event.origin, event.source, window.parent) | |
| ) | |
| return; | |
| if ( | |
| event.source !== window && | |
| !DomainOriginVerificationService.verifyStrictOrigin(event.origin, event.source, window.parent) | |
| ) | |
| return; |
| export function parseAndHealJsonLines(content: string): ParseResult { | ||
| let wasHealed = false; |
There was a problem hiding this comment.
To comply with the repository's General Rule on hardening string processing utility methods, please add a guard clause to safely handle nullish or undefined inputs by returning a safe default.
| export function parseAndHealJsonLines(content: string): ParseResult { | |
| let wasHealed = false; | |
| export function parseAndHealJsonLines(content: string | null | undefined): ParseResult { | |
| if (!content) { | |
| return {blocks: [], wasHealed: false}; | |
| } | |
| let wasHealed = false; |
References
- Harden string processing utility methods with guard clauses to safely handle nullish or undefined inputs by returning safe defaults, and ensure corresponding unit tests are added to verify resilience against nullish values.
| if (!target) { | ||
| this.iframeElement = null; | ||
| this.iframeWindow = null; | ||
| this.earlyMessageBuffer.length = 0; | ||
| return; | ||
| } |
There was a problem hiding this comment.
When unregistering the iframe (i.e., when target is nullish), the original implementation cleared the earlyMessageBuffer by setting its length to 0. Removing this cleanup can lead to memory leaks or stale messages being replayed when a new iframe is registered later. Please restore this cleanup step.
| if (!target) { | |
| this.iframeElement = null; | |
| this.iframeWindow = null; | |
| this.earlyMessageBuffer.length = 0; | |
| return; | |
| } | |
| if (!target) { | |
| this.iframeElement = null; | |
| this.iframeWindow = null; | |
| this.earlyMessageBuffer.length = 0; | |
| return; | |
| } |
| navigator.clipboard.writeText(payload).catch(err => { | ||
| console.error('Failed to copy A2UI component usage to clipboard: ', err); | ||
| }); |
There was a problem hiding this comment.
To ensure robust clipboard operations across all environments (including non-secure contexts or older browsers where navigator.clipboard is unavailable), please use @angular/cdk/clipboard's Clipboard service instead of navigator.clipboard.writeText directly. This aligns with the ShareService implementation introduced in this PR.
Note: You will need to import Clipboard from @angular/cdk/clipboard and inject it at the class level:
private readonly clipboard = inject(Clipboard);
this.clipboard.copy(payload);
⚡ A2UI Composer PR PreviewYour automated preview is successfully live (commit
|
1 participant
Description
Fifth and final in a series of refactorings to clean up the A2UI Composer code.
UI Shell Encapsulation, CDK Share Service & Telemetry Directives
Pre-launch Checklist