fix(themes): enable theme pack routes on Windows

[leonlaiyc]

Problem / Motivation

#311 originally kept theme-pack routes disabled on Windows because the shared descriptor-path resolver could not establish the opened file's real path there.

That prerequisite has since landed independently in #530: _fd_real_path now resolves Windows handles with GetFinalPathNameByHandleW, and containment continues to fail closed when the opened path cannot be established. The theme handlers, however, still retain their old Windows 501 gate.

Why it matters

Theme packs are simply unavailable to every Windows user — the routes 404, so the
Themes surface cannot install, list, or remove a pack at all. The block was a
deliberate temporary measure while a descriptor-path resolver gap was open, and it
outlived the gap.

What changed

• remove the stale Windows 501 gate from the theme-pack route family
• translate a typed GitHub-clone sandbox refusal into HTTP 503 with a machine-readable code instead of an uncaught 500
• exercise install/detail/removal/asset/overlay/topbar behavior on Windows
• require real Windows descriptor-path resolution in regression coverage
• use errno-based capability skips for symlink fixtures while continuing to raise unrelated filesystem failures
• tighten the error-code ratchet after removing the obsolete uncoded 501 response
• update theme security and Windows-support documentation

This PR does not introduce a new Windows descriptor resolver; it enables the theme routes against the shared resolver already landed in #530.

Theme-pack reads continue to use descriptor-pinned containment. Windows uses the shared _fd_real_path implementation, and any inability to resolve or compare the opened descriptor fails closed.

Tests

• focused theme/hooks/symlink/error-contract pytest surface — 512 passed, 18 skipped
• pytest -q test/test_error_code_contract.py — 6 passed
• isort check on the changed Python files
• Flake8 on the changed Python files
• mypy on the changed theme handler
• docs-lint repository scan
• brand gate self-test and diff scan
• git diff --check

Fixes #311

[github-actions]

GPT 5.6 Review (fork) — 🔴 changes requested (blocking)

Reviewed c6eb0a7e4133d0a8c5d000a05d028db29e6869da via the fork AI-review pipeline; updated in place on each push.

BLOCKING -- src/kiro_crew/dashboard/handlers/themes.py:508 -- Windows GitHub installs now raise an uncaught sandbox error
if _THEMES_WIN_UNSUPPORTED: return _win_unsupported_response() (removed)
Default Windows + GitHub source -> api_themes_install -> _clone_github -> SandboxUnavailableError -> HTTP 500.
Fix: Restore the install guard until sandbox refusal is mapped to a normal response.

BLOCKING -- test/conftest.py:56 -- Unsupported filesystems now abort test collection
raise
Filesystem returns EPERM/EOPNOTSUPP for symlink creation -> conftest import -> _can_create_symlink -> collection aborts.
Fix: Return False for capability-related OSError codes such as EPERM and EOPNOTSUPP.

[BLOCK-MERGE] c6eb0a7
[GPT-REVIEWED] c6eb0a7

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Advisory design-level review of c6eb0a7e4133d0a8c5d000a05d028db29e6869da via the fork AI-review pipeline — updated in place on each push; does not block merge.

Design-Verdict: PASS

Removing a gate whose prerequisite landed is the right shape; every Windows read still funnels through the fail-closed descriptor-containment chokepoint.

Suggestions

• The pathname-level link guards this PR now activates on Windows (p.is_symlink() in _list_themes_sync/_resolve_local_source, os.path.islink in _copy_installed_theme) miss NTFS junctions — the repo's own platform_compat table warns exactly this. Byte reads stay safe because GetFinalPathNameByHandleW containment rejects the escaped path, but the "linked directories are never listed" invariant (asserted only via @requires_symlinks) silently doesn't hold for junctions; swap in platform_compat.is_link_or_junction so the first-line guards match the platform they now run on.

[DESIGN-REVIEWED] c6eb0a7

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed c6eb0a7e4133d0a8c5d000a05d028db29e6869da via the fork AI-review pipeline; updated in place on each push.

Review details

Both candidates in .review-candidates.md are premised on unverified "if" conditions, and both carry the discovery pass's own "low" confidence. I'll falsify each against the actual code.

Candidate 1 (hooks.py:1833, st_nlink unreliable on Windows): The premise is "If st_nlink is not populated to the true link count on the Windows runtime." That is exactly the disqualifying "if a caller were to" shape, and it requires assuming runtime behavior the discovery pass admits it could not verify. In fact CPython's Windows os.fstat populates st_nlink from nNumberOfLinks (via GetFileInformationByHandle on the descriptor), so the premise is false. Additionally, the trust boundary here is the single OS user; a same-UID attacker who can plant a hardlink can already read the sensitive file directly. No concrete (a)/(b)/(c) re-derivable. Drop.

Candidate 2 (hooks.py:1841, case-sensitive commonpath): The stated outcome is explicitly "a false-negative, not an escape" — a legitimate asset returns 404. That is neither a security hole, crash, data loss, nor corruption, so it could not be BLOCKING even if real. And it is gated on "if that differs from root_real's casing" — a "might" condition. On Python ≥3.8 Windows, os.path.realpath itself resolves via GetFinalPathNameByHandleW, so both sides agree on casing; the now-un-skipped test_reads_a_regular_file_inside_the_pack runs on Windows and would surface any skew. The line is also unchanged by this diff (only the docstring above it changed). Drop.

Neither survivor answers (a)/(b)/(c) from code I opened without assuming unverified behavior. Nothing reaches the 80 threshold.

No findings.

[OPUS-REVIEWED] c6eb0a7

[leonlaiyc]

CI forensic A/B for head a243083f9cec7d0f9767e76eb02abc52882e96c:

1. Exact failing node: est/test_denied_commands_security.py::TestIsDeniedReDoSResistance::test_mid_dotstar_chain_spam_stays_linear in Backend Tests (3.10, 2).
2. First meaningful assertion: measured doubling ratio 3.054023822108979 < 3.0 (marginal threshold miss). The shard otherwise completed with 12,599 passed.
3. Base A/B: the requested base c6d3394 passed the same Backend Tests (3.10, 2) job in run 31702955259. The current PR merge-base �e5007022fe70bff2c332606807841c96c2786d2 also passed it in run 31765682600.
4. Ownership: neither est/test_denied_commands_security.py nor src/kiro_crew/security.py is in this PR's 9-file diff. The only changed security-adjacent file is hooks.py, and its diff is documentation for fd-path resolution only; it cannot reach this matcher benchmark. Windows backend shards 1-4 are green on this head.

Classification: unrelated performance-test flake / runner variance (case D), not a theme/fd regression. No #3293 code change is warranted.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[bolichen97]

Hi @leonlaiyc, a maintainer nudge on this one: it is carrying the readiness: action required label and has had no activity for about 4 days, so it is not moving toward merge.

Current state:

• Base: 727 commits behind main
• Merge state: CONFLICTING, so it cannot merge until the conflicts are resolved
• Red signals:
  • Backend Tests (Windows) (3)
  • Publish readiness signal
  • PR Readiness

Could you resolve the conflicts against the latest main and push a fix?

git fetch upstream           # or: git fetch origin, if this branch lives here
git rebase upstream/main
# resolve any conflicts, run the local gates below, then update the branch with
# a force-with-lease so the rebase lands without clobbering anyone else's work

The local gates to run before updating the branch:

black src/kiro_crew test && isort src/kiro_crew test
flake8 src/kiro_crew test && mypy src/kiro_crew
python -m pytest
cd website && npm run build && npm run test

A good part of this branch's redness is likely stale rather than a real defect: it predates a lot of what is now on main, and several of these gates (the Coverage Gate, the Windows shards, the AI review lanes) have changed since the last run here. A rebase alone often clears them. If something still fails afterwards and you believe it is a false positive, say so in a comment and we will take a look. If the change is no longer needed, feel free to close the PR.

[chenmingwei23]

Kiro Crew [operator: chenmingwei23#de330d0c]: This PR has been inactive for 7+ days. I reviewed the blockers but they require your input:

• This is a Windows-only behavioral change (enabling theme-pack routes on Windows), and the current red is the Backend Tests (Windows) shard plus a merge conflict. The correct behavior cannot be verified from a Linux host, so the pipeline will not drive it automatically.
• GPT 5.6 is BLOCKING with two Windows-specific findings that need your platform judgment: (1) removing the _THEMES_WIN_UNSUPPORTED install guard now lets a Windows + GitHub-source install raise an uncaught SandboxUnavailableError (HTTP 500) -- you need to decide whether to restore the guard or map the sandbox refusal to a normal response; (2) test/conftest.py re-raises on filesystems that reject symlink creation (EPERM/EOPNOTSUPP), aborting collection -- confirm the intended fallback.

Please rebase onto main, decide the Windows install/sandbox contract, and push. The pipeline will re-assess on its next cycle.

[iamwhatever]

This PR is the only thing standing between #311 and done, so here is a precise conflict inventory to make the rebase cheap. Probed read-only with git merge-tree against origin/main @ 2ddfb409d; nothing was pushed anywhere.

State: head eeb816f2b, merge base 659cab000 — the branch carries 1 commit and is 1234 commits behind main.

Three files conflict; everything else auto-merges:

file	result
src/kiro_crew/dashboard/handlers/themes.py	CONFLICT — this is the fix file
error-code-baseline.json	CONFLICT
docs/guides/windows-install.md	CONFLICT
src/kiro_crew/hooks.py	auto-merges
test/test_dashboard_themes_coverage.py	auto-merges
test/test_hooks_coverage.py	auto-merges
test/test_theme_install.py	auto-merges
docs/system-specs/common/testing-conventions.md	auto-merges

One thing that should make the themes.py resolution easier than it looks: the Windows _fd_real_path implementation this PR was originally paired with has already landed on main independently. hooks.py:1816-1849 now carries the full ctypes.WinDLL("kernel32") + msvcrt.get_osfhandle → GetFinalPathNameByHandleW branch, with \\?\UNC\ / \\?\ prefix normalization and fail-closed return None. So the remaining substance of this PR really is just the gate removal plus its tests, and the hooks.py half auto-merges.

Related: main's themes.py:75-80 still carries a comment asserting the chokepoint "has no Windows implementation (_fd_real_path returns None there -> fail-closed on every read)". That is now factually wrong, and since your diff rewrites that exact region it is the natural place to correct it.

error-code-baseline.json is a generated ratchet — regenerate it on the rebased tree rather than hand-merging the hunks.

Happy to take the rebase off your hands if you'd rather not: say the word and I'll either push it to this branch (maintainerCanModify is on) or open a successor PR crediting you and close this one. Not touching your fork without you asking.

[iamwhatever]

Hi @leonlaiyc — closing this in favour of #5943, which is your commit, rebased onto current main, not a rewrite. You are retained as the git Author on it; I am only the committer. Sorry to move it out from under you rather than waiting — the maintainer asked for #311 unblocked today, and I would rather not force-push to your fork.

Why a successor instead of a rebase in place: this branch had drifted 1234 commits behind main with conflicts in three files, and the honest reason it drifted is that nobody picked it up for twelve days, not anything about the change.

Your design survived intact, and it is the reason the PR reads well. Three calls in particular I kept verbatim because they were better than the obvious alternatives:

• 503 rather than 500 for the sandbox-unavailable clone, on the grounds that a host capability gap is not a server fault and no retry or different body can carry the remedy.
• No unsandboxed retry, because the URL is user-influenced and git clone executes remote content — which is exactly why the spawn is chokepointed at all. That was the tempting shortcut and you were right to refuse it.
• The fail-soft symlink probe over an errno allowlist, because the probe runs at conftest import time, so a propagating error is a whole-session collection failure rather than one loud test — and an allowlist necessarily misses the environments nobody anticipated. test_symlink_capability_probe.py is your file, unchanged.

The only substantive edits were conflict resolutions, both caused by one upstream rename — main replaced resource_limit_preexec with run_limited, so the import block and the _clone_github comment both had to take main's spelling while keeping your SandboxUnavailableError translation. I also kept main's newer windows-install.md rows and added only your new theme-pack row, and regenerated error-code-baseline.json on the rebased tree.

One thing worth your eye if you have a moment: _fd_real_path gained its full Windows GetFinalPathNameByHandleW branch on main independently while this sat, so the gate you were removing had already outlived its stated justification — which makes the change stronger than when you wrote it, and I said so in the PR body.

Thanks for the work, and for the commit message — I lifted most of the reasoning in the PR description straight from it. Happy to hand #5943 back to you, or to close it if you would rather reopen and rebase this one yourself; just say which.