Windows: guard focus(), unstick the opening class, add child windows

[AllTerrainDeveloper]

Three related pieces of window-integrity work. The first two are fixes for reported bugs, both of which produced the same symptom — a window that is present, painted and unusable. The third is a new API that came out of the same conversation.

WindowManager.focus() — argument guard

focus() took a Window with no runtime check, and pushed its argument onto _stack before calling setZIndex() across every member. So focus( 'some-id' ) — the form javascript-reference.md and the comment in src/built-in-commands.ts already show — left a non-Window wedged in the stack, and every later focus() threw on it. Click-to-focus, dock activation and open-reuse were all dead until a reload, with nothing in the console pointing at the original bad call.

It now accepts Window | string, resolves ids, and rejects anything else before touching the stack. An unknown id stays a silent no-op, matching raise(): a window closing between the moment its id was captured and the moment focus is requested is a routine race, not a programming error. A non-Window, non-string argument warns.

Two corrections to the report this came from:

• raise() is not affected. It already takes a string, resolves via getById(), and returns early on idx === -1. The described splice( -1, 1 ) on a miss cannot happen.
• conflict-toast.ts was broken, but not by this. It declared its own declare global asserting window.openStation.windowManager.focus( id ) => Window — a global the shell never defines (the namespace is wp.os) with a signature that matches nothing. Optional chaining meant it never threw and never poisoned the stack; "View folder" just silently did nothing. share-menu-items.ts carried the identical fabricated global, calling a close() that doesn't exist on the manager either, so leaving a shared folder quietly left its window open. Both now go through wp.os, and the folder close goes through the Window, where close() actually lives.

.os-window--opening always comes off

Removal hung on a single once-only, unfiltered animationend listener. Two ways that fails:

1. animationend bubbles. A spinner, shimmer or holo drift finishing anywhere inside the window body consumed the listener and cut the open animation short.
2. A hidden document never advances the animation, so the event never arrives at all. The class is not cosmetic — its animation's from frame is opacity: 0; scale(0.92) — so a window opened in a backgrounded tab (session restore the user tabs away from, an os-open-requested from another surface, automation) stays painted invisible.

Now filtered on animationName + target, paired with animationcancel, backed by a timeout deadline, and skipped outright while document.hidden — nothing to animate for a document nobody is watching, so the stuck state can't arise at the source.

One correction here too: .os-window--opening carries no pointer-events: none in our CSS, and neither do unfocused windows. The reported "unclickable" framing isn't ours — that's either the third-party plugin's own CSS or the focus-stack poisoning above. The stuck-class hazard is real regardless; it just makes the window invisible rather than inert.

New: child windows

openChild( parentWindowId, config ) opens a real window — own chrome, drag, resize, minimize, taskbar entry — with one rule layered on top: its owner can never sit above it. Clicking the owner shakes the child and leaves focus there.

For the shapes a modal dialog gets reached for when what you actually want is a window: a full editor for one row of a list, a wizard beside the page it configures, a diff over the revision it belongs to.

await wp.os.windowManager.openChild( 'edit-post-42', {
    id: 'my-plugin-seo-audit-42',
    url: '#seo-audit-42',
    title: 'SEO audit',
    icon: 'dashicons-chart-line',
    native: true,
    render: ( body ) => { /* … */ },
} );

The owner stays fully usable — scrollable, draggable, resizable, minimizable. Only its z-order is constrained; that's the reason to use a window rather than a dialog in the first place. The redirect lives inside focus(), so click-to-focus, dock, taskbar, alt-tab and open-reuse are covered by construction rather than per call site.

Surface: openChild, ownerOf, childrenOf, blockingChildOf, the os-window-child-blocked event, the os.window.child-blocked action, and WindowConfig.parentWindowId as the primitive.

Behaviors, each with a test:

Chains	A child can own a child; focus goes to the deepest link.
Close	Closing an owner closes its children. A child with unsaved changes still gets to veto and becomes an ordinary window — better than discarding the user's work.
Minimize	Cascades down, and restore brings back only what the cascade put away. A child the user minimized themselves stays minimized.
Minimized children	Stop blocking. The user put it away on purpose, so the owner is theirs again.
Session	Children are excluded from snapshots, so a reload can't restore a child blocking an owner that failed to come back.

Two subtleties carry comments where they live: the ownership stack pass exempts minimized windows, or hoisting an invisible child to the top would make setFocused( i === length - 1 ) hand focus to a window nobody can see; and blockingChildOf carries a cycle guard, since a plugin declaring A-owns-B-owns-A would otherwise spin on the first click.

Testing

28 new tests in tests/vitest/window-focus-guard-and-opening-class.test.ts and tests/vitest/window-child-ownership.test.ts.

• npm run test:js — 4458 pass
• npm run lint — clean
• npm run typecheck — clean
• npm run build — run

No PHP touched.

Worth a manual pass before merge: that the opening animation still looks right on an ordinary open, and that a child window's shake-on-blocked-focus reads as helpful rather than annoying. Those two are feel, not logic, and no browser was driven for this.

🤖 Generated with Claude Code

[AllTerrainDeveloper]

@claude can you review?

[claude]

Claude finished @AllTerrainDeveloper's task in 5m 24s —— View job

---

Review — window focus guard, opening-class fix, child windows

• Gather context (PR description, changed files list)
• Read diff for src/window-manager/index.ts, src/window/index.ts, src/types.ts, src/hooks.ts
• Read diff for src/desktop-files/conflict-toast.ts, src/desktop-files/share-menu-items.ts, src/drag/focus-window-on-drag-hover.ts
• Read new tests
• Check docs updates for accuracy
• Post review feedback

Note on tooling: npm ci/npm run * required network/approval that wasn't grantable in this session, so I couldn't independently re-run test:js/typecheck/build. Everything below is from reading the diff (git diff origin/trunk...HEAD) and tracing the code paths by hand.

focus() guard, raise(), .os-window--opening fix

These are solid. The id-or-Window guard resolves before touching _stack, the console.warn path is correctly gated to non-string/non-Window inputs only, and enforceOwnershipOrder()'s stable topological placement (src/window-manager/index.ts ~1440–1478) is correct under manual tracing, including when the stack starts in "wrong" order or a cycle exists (the seen/chain guards in blockingChildOf and place() both prevent infinite loops). The conflict-toast.ts/share-menu-items.ts fixes for the fabricated window.openStation global match the PR description and now correctly go through wp.os.

The opening-class fix (src/window/index.ts _armOpeningClassRemoval) correctly filters animationend/animationcancel on target + animationName, races a timeout, and skips the class entirely under document.hidden. No issues found there.

Bug: openChild() doesn't actually center when width/height are omitted

src/window-manager/index.ts:1296-1312:

const owner = parent.getSnapshot();
const width = config.width ?? Math.round( owner.width * 0.8 );
const height = config.height ?? Math.round( owner.height * 0.8 );
placement = {
    x: Math.max( 0, Math.round( owner.x + ( owner.width - width ) / 2 ) ),
    y: Math.max( 0, Math.round( owner.y + ( owner.height - height ) / 2 ) ),
};
...
return this.open( { ...config, ...placement, desktopId: parent.config.desktopId, parentWindowId } );

width/height here are only used to compute x/y — they are never passed into the open() call. Unless the caller explicitly sets config.width/config.height, open() resolves its own size from the desktop-rect-based default (defaultWidth = min(desktopRect.width * 0.8, 1200), same for height) or from saved per-baseId geometry (src/window-manager/index.ts:694-726) — neither of which has anything to do with owner.width * 0.8. So the x/y computed for "centered assuming an 80%-of-owner-sized child" get applied to a child that actually opens at a completely different size, and the result is not centered over the owner at all.

This exactly matches the PR's own documented usage:

await wp.os.windowManager.openChild( 'edit-post-42', {
    id: 'my-plugin-seo-audit-42', url: '#seo-audit-42', title: 'SEO audit',
    icon: 'dashicons-chart-line', native: true, render: ( body ) => { /* … */ },
} );

— no width/height passed, so this call gets the mismatched placement.

The only placement test (tests/vitest/window-child-ownership.test.ts "openChild centers over the live owner rect, not its config") always passes explicit width/height in the child config, so this path isn't covered and the mismatch wasn't caught.

Fix: include the computed width/height in placement (so they're actually applied to the open() config when the caller didn't supply them), not just used for the x/y math. Worth a test for the no-explicit-size call shape too, since that's the shape shown in the docs/examples.

Fix this →

Minor: redundant focus churn on multi-child minimize/close cascades

onMinimize (src/window-manager/index.ts:897-907) does cascadeMinimize(w) then recomputes "topmost visible window" and calls focus() — for every window in the cascade, not just the outermost one. When an owner with 2+ children is minimized, each child's own minimize() re-enters this same handler and fires its own intermediate focus() call (and thus os-window-focused/os-window-blurred events) before the cascade settles on the final window. The end state is correct, but subscribers building activity feeds/analytics off WINDOW_FOCUSED/WINDOW_BLURRED will see extra transient events for a single user action. Same shape applies to the children-close loop in remove(). Not blocking, but worth a look given the "framework MUST NOT auto-render / event churn" ethos in AGENTS.md.

Docs

docs/javascript-reference.md, docs/event-driven-framework.md, docs/api-index.md, docs/examples/child-windows.md are all consistent with the implemented behavior and cover the new API surface (openChild, ownerOf, childrenOf, blockingChildOf, the new event/hook, WindowConfig.parentWindowId). Good adherence to the "update docs in the same PR" rule.

Summary

Solid PR overall — the focus-guard and opening-class fixes are correct and well-tested, and the child-window ownership model (chains, minimize/restore cascades, close cascades, cycle guards, session exclusion) checks out under manual tracing. The one real bug is the openChild() sizing/centering mismatch above, which affects the common (no explicit width/height) call shape shown in the PR's own docs — worth fixing before merge since it undermines the "centers over the owner" guarantee that's the main selling point of the feature.