fix(agent-browser): defer preview origin exposure
This commit is contained in:
@@ -0,0 +1,95 @@
|
|||||||
|
# Task: Remediate X-01 stable preview injection origin
|
||||||
|
|
||||||
|
## Identity
|
||||||
|
|
||||||
|
- Task ID: 20260827-x01-injection-origin-6d7a91c4
|
||||||
|
- Mode: Feature
|
||||||
|
- Branch: codex/20260827-x01-injection-origin-6d7a91c4-x01-injection-origin-6d7a91c4
|
||||||
|
- Worktree: D:\Datas\OthersProjects\makelore-x01-injection-origin-6d7a91c4
|
||||||
|
- Base commit: 0a4f526c2c6b301a6b7fcaef982bea2f703b41c0
|
||||||
|
- Owner: codex
|
||||||
|
- Status: Ready for Integration
|
||||||
|
|
||||||
|
## Scope
|
||||||
|
|
||||||
|
- X-01 live-acceptance remediation on exact client coordinator product base
|
||||||
|
`0a4f526c2c6b301a6b7fcaef982bea2f703b41c0`.
|
||||||
|
- Fix the Agent Browser pre-document preview-data injection in
|
||||||
|
`electron/agent-browser/module.ts` so a transient early `location.origin`
|
||||||
|
does not prevent the capability from becoming available once the target
|
||||||
|
Origin is stable.
|
||||||
|
- Add only the focused regression in `tests/unit/agent-browser-core.test.ts`
|
||||||
|
and this task-scoped record.
|
||||||
|
|
||||||
|
## Intent And Constraints
|
||||||
|
|
||||||
|
- Follow spec §10.1/§10.4 and §14 groups 2/13, ticket ML-09, and X-01 live
|
||||||
|
acceptance. Preserve explicit opt-in, exact-Origin checking, the existing
|
||||||
|
non-enumerable/non-configurable descriptor contract, child/sessionRef
|
||||||
|
handling, and foreign-origin absence of the credential value.
|
||||||
|
- Live evidence shows CDP script registration and preview-session opening now
|
||||||
|
succeed, but packaged Electron sees no `globalThis.__MAKELORE_DATA__` on the
|
||||||
|
target page. The one-shot Origin check can run while Chromium reports an
|
||||||
|
early `null`/unstable Origin. Use the smallest safe closure-based fix: a
|
||||||
|
non-configurable getter resolves the closed-over value only for the exact
|
||||||
|
current Origin; it must not expose the credential for another Origin.
|
||||||
|
- Do not touch root main, the coordinator worktree, server code, PRs, routes,
|
||||||
|
other tests, or unrelated E2E behavior. Do not revert concurrent changes.
|
||||||
|
|
||||||
|
## Planning Gate
|
||||||
|
|
||||||
|
- Result: Passed on 2026-08-27.
|
||||||
|
- Ran `check_project_docs.py`, released the prior completed task through
|
||||||
|
`task_context.py release`, and started this feature task against the exact
|
||||||
|
coordinator base. Status JSON matches this task ID, absolute worktree and
|
||||||
|
branch, feature mode, and base commit.
|
||||||
|
- Read the required project-memory startup set, coordinator task scope,
|
||||||
|
relevant Agent Browser architecture/data-flow, spec §10.1/§10.4/§13/§14,
|
||||||
|
ticket graph ML-06/ML-09/X-01, and canonical preview contract. The
|
||||||
|
coordinator owns the parent path; this isolated task owns only the stated
|
||||||
|
module/test/record. Other active scopes are unrelated or placeholders.
|
||||||
|
|
||||||
|
## Outcome
|
||||||
|
|
||||||
|
- Replaced the one-shot Origin check in `previewDataInjectionScript` with an
|
||||||
|
IIFE closure containing the serialized value and expected Origin. The
|
||||||
|
non-enumerable, non-configurable getter returns the closed-over value only
|
||||||
|
while the current frame has the exact expected Origin; otherwise it returns
|
||||||
|
`undefined`. Child/sessionRef installation and lifecycle cleanup are
|
||||||
|
unchanged.
|
||||||
|
- Added regression coverage for an early `null` Origin becoming the target
|
||||||
|
Origin, foreign-origin denial, descriptor shape, and the getter source not
|
||||||
|
containing the credential value.
|
||||||
|
|
||||||
|
## Plan
|
||||||
|
|
||||||
|
1. Add a red regression that evaluates the injection script with an early
|
||||||
|
`null` Origin, then changes to the target Origin, and verifies the exact
|
||||||
|
value, descriptor, and foreign-origin behavior.
|
||||||
|
2. Replace the one-shot root injection guard with the smallest closure getter;
|
||||||
|
keep injection serialization, child sessions, and lifecycle cleanup intact.
|
||||||
|
3. Run focused Agent Browser/preview tests, typecheck, scoped lint, diff/doc
|
||||||
|
gates, then make one clean commit.
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
- Red-first regression before the implementation failed: after the simulated
|
||||||
|
`null` → target Origin transition, `__MAKELORE_DATA__` remained `undefined`.
|
||||||
|
- `corepack pnpm vitest run tests/unit/agent-browser-core.test.ts` — 1 file,
|
||||||
|
77 tests passed.
|
||||||
|
- `corepack pnpm run typecheck` — passed.
|
||||||
|
- `corepack pnpm exec eslint electron/agent-browser/module.ts
|
||||||
|
tests/unit/agent-browser-core.test.ts` — passed.
|
||||||
|
- `git diff --check` — passed.
|
||||||
|
- No live/Electron E2E was run here; the coordinator must rerun X-01 live
|
||||||
|
acceptance after integration.
|
||||||
|
|
||||||
|
## Follow-ups
|
||||||
|
|
||||||
|
- Integrate the single commit into the client coordinator and rerun the
|
||||||
|
packaged Electron X-01 preview write/read with real PostgreSQL and a signed-in
|
||||||
|
preview. Focused repository tests do not establish live acceptance.
|
||||||
|
|
||||||
|
## Promotion Candidates
|
||||||
|
|
||||||
|
- None recorded.
|
||||||
@@ -1944,14 +1944,15 @@ function previewDataInjectionScript(
|
|||||||
if (!serializedOrigin || !serializedValue) {
|
if (!serializedOrigin || !serializedValue) {
|
||||||
throw new Error('Preview data injection value is not serializable');
|
throw new Error('Preview data injection value is not serializable');
|
||||||
}
|
}
|
||||||
return `if (globalThis.location?.origin === ${serializedOrigin}) {
|
return `(() => {
|
||||||
|
const expectedOrigin = ${serializedOrigin};
|
||||||
|
const value = ${serializedValue};
|
||||||
Object.defineProperty(globalThis, "__MAKELORE_DATA__", {
|
Object.defineProperty(globalThis, "__MAKELORE_DATA__", {
|
||||||
value: ${serializedValue},
|
get: () => globalThis.location?.origin === expectedOrigin ? value : undefined,
|
||||||
enumerable: false,
|
enumerable: false,
|
||||||
writable: false,
|
|
||||||
configurable: false,
|
configurable: false,
|
||||||
});
|
});
|
||||||
}`;
|
})();`;
|
||||||
}
|
}
|
||||||
|
|
||||||
function crossOriginNavigation(previousUrl: string, nextUrl: string): boolean {
|
function crossOriginNavigation(previousUrl: string, nextUrl: string): boolean {
|
||||||
|
|||||||
@@ -600,7 +600,9 @@ describe('AgentBrowserModule', () => {
|
|||||||
contractVersion: 1,
|
contractVersion: 1,
|
||||||
});
|
});
|
||||||
const descriptor = Object.getOwnPropertyDescriptor(sameOriginGlobal, '__MAKELORE_DATA__');
|
const descriptor = Object.getOwnPropertyDescriptor(sameOriginGlobal, '__MAKELORE_DATA__');
|
||||||
expect(descriptor).toMatchObject({ enumerable: false, writable: false, configurable: false });
|
expect(descriptor).toMatchObject({ enumerable: false, configurable: false });
|
||||||
|
expect(descriptor?.get).toEqual(expect.any(Function));
|
||||||
|
expect(descriptor).not.toHaveProperty('value');
|
||||||
|
|
||||||
const externalGlobal: Record<string, unknown> = {
|
const externalGlobal: Record<string, unknown> = {
|
||||||
location: { origin: 'https://external.example' },
|
location: { origin: 'https://external.example' },
|
||||||
@@ -609,6 +611,48 @@ describe('AgentBrowserModule', () => {
|
|||||||
expect(externalGlobal.__MAKELORE_DATA__).toBeUndefined();
|
expect(externalGlobal.__MAKELORE_DATA__).toBeUndefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('resolves preview data after an early unstable Origin settles', async () => {
|
||||||
|
const adapter = new FakeAdapter();
|
||||||
|
const preview = new FakePreviewDataSession();
|
||||||
|
adapter.onCreate = (view) => {
|
||||||
|
view.webContents.debugger.responders.set('Page.addScriptToEvaluateOnNewDocument', async () => ({
|
||||||
|
identifier: 'script-root',
|
||||||
|
}));
|
||||||
|
};
|
||||||
|
const module = new AgentBrowserModule(adapter, { previewDataSession: preview });
|
||||||
|
|
||||||
|
await module.open({
|
||||||
|
projectId: 'clock',
|
||||||
|
projectPath,
|
||||||
|
url: 'http://127.0.0.1:4173/',
|
||||||
|
injectProjectData: true,
|
||||||
|
});
|
||||||
|
const source = adapter.views[0].webContents.debugger.commands.find(
|
||||||
|
(command) => command.method === 'Page.addScriptToEvaluateOnNewDocument',
|
||||||
|
)?.params?.source;
|
||||||
|
expect(typeof source).toBe('string');
|
||||||
|
|
||||||
|
const changingGlobal: { location: { origin: string }; __MAKELORE_DATA__?: unknown } = {
|
||||||
|
location: { origin: 'null' },
|
||||||
|
};
|
||||||
|
runInNewContext(source as string, changingGlobal);
|
||||||
|
expect(changingGlobal.__MAKELORE_DATA__).toBeUndefined();
|
||||||
|
|
||||||
|
changingGlobal.location.origin = 'http://127.0.0.1:4173';
|
||||||
|
expect(changingGlobal.__MAKELORE_DATA__).toEqual({
|
||||||
|
endpoint: 'http://127.0.0.1:13210/api/runtime/data/v1',
|
||||||
|
token: 'preview-token',
|
||||||
|
contractVersion: 1,
|
||||||
|
});
|
||||||
|
const descriptor = Object.getOwnPropertyDescriptor(changingGlobal, '__MAKELORE_DATA__');
|
||||||
|
expect(descriptor).toMatchObject({ enumerable: false, configurable: false });
|
||||||
|
expect(descriptor?.get).toEqual(expect.any(Function));
|
||||||
|
expect(String(descriptor?.get)).not.toContain('preview-token');
|
||||||
|
|
||||||
|
changingGlobal.location.origin = 'https://external.example';
|
||||||
|
expect(changingGlobal.__MAKELORE_DATA__).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
it('uses the target Origin when the prime navigation reports about:blank', async () => {
|
it('uses the target Origin when the prime navigation reports about:blank', async () => {
|
||||||
const adapter = new FakeAdapter();
|
const adapter = new FakeAdapter();
|
||||||
const preview = new FakePreviewDataSession();
|
const preview = new FakePreviewDataSession();
|
||||||
|
|||||||
Reference in New Issue
Block a user