fix(semantic-layer): support snapshot export on Windows

[padak]

Design: Make semantic-layer snapshot export portable to Windows

Issue: #529
Status: Proposed
Target: kbagent semantic-layer export and every shared snapshot writer

Problem

write_snapshot_to_file() builds its os.open() flags with
os.O_NOFOLLOW. That flag is available on POSIX platforms but is not defined
by Python's os module on Windows. Evaluating the flags therefore raises
AttributeError before the output file is opened.

The helper is shared by semantic-layer export and the optional build output
path, so the portability bug affects more than one command even though export
is the reported reproducer.

Goals

• Make snapshot writes work on Windows without weakening the existing POSIX
  symlink protection.
• Continue writing UTF-8 JSON bytes with deterministic line endings.
• Preserve the current 0o644 permissions contract on POSIX.
• Cover both the Windows-compatible path and the POSIX hardening path with
  regression tests.

Non-goals

• Add Windows ACL management; the numeric mode argument is intentionally
  ignored by Windows.
• Guarantee protection against every Windows reparse-point race. Python does
  not expose a direct O_NOFOLLOW equivalent through os.open.
• Change the snapshot schema, export result, default filename, or CLI output.

Design

Build the flags incrementally from portable flags:

flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC
flags |= getattr(os, "O_NOFOLLOW", 0)
flags |= getattr(os, "O_BINARY", 0)

• On POSIX, O_NOFOLLOW remains active and a pre-existing final-component
  symlink is rejected.
• On Windows, the unavailable flag contributes zero, so os.open() succeeds.
• O_BINARY is a no-op where unavailable and prevents Windows text-mode
  newline translation because the helper writes an already encoded byte
  payload with os.write().

Keep the descriptor lifecycle in the existing try/finally; do not replace it
with a text-mode Path.write_text() call, which would discard the POSIX
hardening and change the low-level write contract.

Update the docstring to describe conditional O_NOFOLLOW protection rather
than implying that every platform provides it.

Implementation map

• src/keboola_agent_cli/services/_semantic_layer_internals.py
  • construct portable open flags with guarded platform-specific additions;
  • document POSIX and Windows behavior.
• tests/test_semantic_layer_service.py
  • simulate Windows by temporarily removing os.O_NOFOLLOW and verify export
    creates valid UTF-8 JSON;
  • where O_NOFOLLOW exists, verify a symlink output path is rejected and its
    target remains unchanged;
  • retain the existing content and POSIX permission checks.
• .github/workflows/ci.yml
  • run the focused semantic-layer export regression on the existing
    windows-latest job so the real Windows os flags are exercised.
• src/keboola_agent_cli/changelog.py
  • record the Windows export fix in the next release entry.

Acceptance criteria

1. Importing/evaluating write_snapshot_to_file() on Windows never accesses
   os.O_NOFOLLOW directly.
2. kbagent semantic-layer export --output <path> creates parseable UTF-8 JSON
   on Windows.
3. The shared build-output writer follows the same portable path.
4. POSIX exports still create mode 0o644 files.
5. On platforms with O_NOFOLLOW, a pre-existing symlink at the output path is
   rejected and the symlink target is not modified.
6. The regression is tested both by a platform-independent missing-flag unit
   test and on the existing Windows CI runner.
7. Focused tests plus lint, format, and type checks pass.

Validation

Run locally:

uv run pytest tests/test_semantic_layer_service.py -k "export" -v
uv run ruff check src/ tests/
uv run ruff format . --check
uv run ty check

On windows-latest, run the focused export regression against Python 3.12 and
the built project. Confirm the output contains non-ASCII data without newline
or encoding corruption.

Risks

• Windows lacks the exact final-component symlink behavior provided by
  O_NOFOLLOW. This PR preserves the strongest protection Python exposes
  through the current low-level API on each platform and documents the gap.
• CI workflow time increases slightly from one focused pytest invocation; the
  existing Windows dependency/build setup is reused.

[devin-ai-integration]

Devin Review found 1 potential issue.

[devin-ai-integration]

🔍 Changelog entry promised in design doc is missing from the PR

The design spec's implementation map (docs/superpowers/specs/2026-07-23-issue-529-windows-semantic-layer-export-design.md:73-74) states src/keboola_agent_cli/changelog.py should record the Windows export fix, but no changelog change is present in this PR. Per CONTRIBUTING.md, make changelog-check is a release-time / local-only gate (not a per-PR CI gate), so this will not fail CI, but the PR does not fully match its own stated implementation map. Reviewer may want to confirm whether the changelog entry is intended for a follow-up version bump.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[padak]

Addressed in 2c0e080. The PRD now states that this PR intentionally has no changelog edit because the repository has no unreleased section; the fix is recorded when the next release version is prepared.

[padak]

Review of #531 — fix(semantic-layer): support snapshot export on Windows

Generated by kbagent-pr-reviewer subagent. Verdict and findings below
are advisory; the human author retains every veto. CI-coverable issues
(lint, format, tests) are confirmed via make check, not duplicated here.

Summary

This PR fixes a genuine, reproducible Windows crash: write_snapshot_to_file() (shared by semantic-layer export and the optional semantic-layer build --output path) hardcoded os.O_NOFOLLOW in its os.open() flags expression, which raises AttributeError on Windows before the file is ever opened, since that constant does not exist in Windows' os module. The fix builds the flags incrementally with getattr(os, "O_NOFOLLOW", 0) (preserves POSIX symlink hardening, no-ops on Windows) and additionally adds getattr(os, "O_BINARY", 0) to prevent Windows CRT text-mode CRLF translation from corrupting the byte-exact JSON payload — a detail that is easy to miss and correctly reasoned through in the PR's design doc. Two new regression tests are added (a cross-platform Windows-simulation test via monkeypatch.delattr(os, "O_NOFOLLOW"), using real non-ASCII content, and a POSIX-only symlink-rejection test that did not exist before this PR), plus a new Windows CI job step that runs the focused export tests on a real windows-latest runner. The change is tightly scoped: no CLI command, flag, JSON shape, or exit code changed. Verdict: APPROVE — no blocking or non-blocking issues found; one cosmetic nit on PR-description staleness.

Verdict

• Verdict: APPROVE
• Blocking findings: 0
• Non-blocking findings: 0
• Nits: 1

Blocking findings

(none)

Non-blocking findings

(none)

Nits

• [NIT-1] PR description ("Implementation map") vs. committed design doc — the GitHub PR description still lists src/keboola_agent_cli/changelog.py under "Implementation map" ("record the Windows export fix in the next release entry"), but the actual committed file docs/superpowers/specs/2026-07-23-issue-529-windows-semantic-layer-export-design.md:114-116 was revised during implementation to explicitly say no changelog entry is made in this PR — and indeed changelog.py is not in the diff, which is the correct call (0.76.1 is already tagged/released; there's no open "unreleased" changelog key to append to, matching the precedent set by the immediately-preceding sibling PR #530). Consider gh pr edit 531 to sync the description with the final decision so a reviewer skimming only the GitHub body doesn't wonder whether changelog.py was supposed to change.

Verification log

• gh auth status → authenticated as padak, repo/workflow scopes ✓
• Read CONTRIBUTING.md (Checklist: Adding a New CLI Command, Plugin synchronization map, Releasing a new version) ✓ — confirmed this PR adds/removes/renames zero CLI commands, so the per-command checklist and most of the sync-map table are N/A.
• Read CLAUDE.md convention #17 and ## All CLI Commands ✓ — no command signature affected.
• Read plugins/kbagent/agents/keboola-expert.md §1 and §3 ✓ — no rule/gotcha implicated; grepped context.py, keboola-expert.md, gotchas.md, commands-reference.md, CLAUDE.md for O_NOFOLLOW/0o644/atomic-write → empty, nothing agent-facing references the changed internals.
• gh pr view 531 --json title,body,files,additions,deletions,baseRefName,headRefName,labels,state → OPEN, fix(semantic-layer): prefix matches a bug fix, 4 files, +170/-13 ✓
• gh pr checkout 531 → failed (already used by worktree at /private/tmp/kbagent-issue-529); used git worktree list + gh pr view --json headRefOid to confirm that pre-existing worktree is clean and sits exactly at the PR head SHA 2c0e080d1a8d52fd63907c046a1b89ba85bfe47d ✓ — read PR code from there; own tree (claude/pr-531-review-f4d37a) left untouched (git status --short clean, confirmed before and after).
• gh pr diff 531 → 264-line diff across .github/workflows/ci.yml, new docs/superpowers/specs/2026-07-23-issue-529-windows-semantic-layer-export-design.md, src/keboola_agent_cli/services/_semantic_layer_internals.py, tests/test_semantic_layer_service.py ✓
• Layer-violation greps (typer/click in services, httpx/requests in commands, formatter/typer in clients) → all empty ✓; git diff main...HEAD -- cli.py commands/** for new/removed @*_app.command → empty (no command surface change) ✓
• grep -rn write_snapshot_to_file src/ tests/ → confirmed exactly 2 call sites (semantic_layer_service.py:619 in export_model, :1595 in build), both delegate identically with no divergent wrapper logic, so one root-cause fix + one shared regression test genuinely covers both callers ✓
• Reproduced the reported bug directly: python -c with os.O_NOFOLLOW deleted, evaluating the old (pre-fix, git show main:...) expression os.O_WRONLY | os.O_CREAT | os.O_TRUNC | os.O_NOFOLLOW → AttributeError: module 'os' has no attribute 'O_NOFOLLOW' (exact match to the issue); evaluating the new getattr-based expression under the same simulated environment → succeeds, flags = 1537 ✓
• uv run pytest tests/test_semantic_layer_service.py -k "export" -v (mirrors the new CI Windows step exactly) → 5 passed (test_export_envelope_shape, test_export_writes_file_with_correct_permissions, test_export_writes_utf8_json_without_o_nofollow, test_export_rejects_existing_symlink_output_path, test_export_default_path) ✓
• uv sync --extra server (environment fix for a fresh worktree venv, not a code change) then make check → ruff check clean, ruff format --check clean, ty check exit 0 (3 pre-existing warning[unresolved-import] diagnostics unrelated to this diff — explicitly non-blocking per CONTRIBUTING.md), SKILL.md up-to-date, version in sync, check_command_sync.py → "all 251 CLI commands registered/documented", generate_changelog.py --check → "All 43 stable releases have changelog entries" (confirms no changelog gap), check_error_codes.py clean, full suite → 4669 passed, 8 skipped, 132 deselected ✓
• python -c "import yaml; yaml.safe_load(open('.github/workflows/ci.yml'))" → parses cleanly, new Test semantic-layer export on Windows (issue #529) step correctly ordered inside build-windows job, no --extra server needed since test_semantic_layer_service.py/conftest.py import nothing from fastapi/server (verified via import grep) so uv run pytest on the bare Windows runner env resolves fine ✓
• Verified os.O_BINARY does not exist on this POSIX runner (hasattr(os, "O_BINARY") == False) so the new flag is a genuine no-op here, matching the PR's own claim ✓
• diff of the raw GitHub PR body against the committed design-doc file → 2 differences: trailing whitespace (cosmetic) and the changelog.py paragraph (→ NIT-1) ✓
• Checked precedent: sibling PR #530 (fix(update): make self-update safe on Windows, merged immediately prior on main) also added a docs/superpowers/specs/...design.md, also touched zero plugin-sync surfaces and zero changelog.py, also did not bump the version — same author pattern, already accepted ✓

Open questions for the author

(none)