[itzzdev09]

Refs #1259

Before / after

Full suite on Windows 11, Python 3.12, main @ 95e085e:

	failed	passed	skipped
before	31	1710	12
after	12	1726	16

Every one of the remaining 12 is owned by another issue, so none of them is a Unix assumption:

Remaining	Count	Owner
test_cloud_source_upload / test_cloud_cli_runtime archive repack	7–8 (intermittent)	#1258
test_config_loader::test_persist_current_sets_0600_mode	1	#648 / #652
test_pricing::test_resolves_common_bare_model_names	1	#1288
test_workspace_files drive letters	2	#1285

Changes

/usr/bin/env git → shutil.which("git") — one line in tests/test_threat_model_tool.py accounted for 19 of the 31 failures. shutil.which("git") was already the idiom in tests/test_cloud_source_upload.py:46, so this aligns with the suite rather than introducing a pattern. The module skips if git is absent.

Git config isolation — a new autouse fixture points GIT_CONFIG_GLOBAL / GIT_CONFIG_SYSTEM at a non-existent path and supplies a commit identity. Tests that build a throwaway repo were reading the developer's own config underneath, so a global core.excludesFile silently drops files from source selection, and commit.gpgSign or core.hooksPath can fail the commit outright. This is machine-independence, not just Windows.

Control-character filenames — two tests prove Strix never echoes an escape sequence back to the terminal, and needed a hostile filename on disk to do it. Windows rejects those characters outright, so the fixture could not even be built and the test failed in setup. A write_control_character_file fixture now skips with the real OSError when the filesystem refuses the name. The POSIX contract keeps being exercised; Windows stops reporting a product failure it does not have.

Mount policy — test_infer_target_type_applies_the_mount_policy hardcoded /etc, which does not exist on Windows, so it failed at target validation before reaching the policy. check_mountable_dir already handles both families (_FORBIDDEN_MOUNT_TREES and _FORBIDDEN_WINDOWS_TREE_NAMES) — only the fixture was Unix-only. It now picks the platform's protected directory, following the existing idiom in test_check_mountable_dir_rejects_system_subdirs a few lines above. This one now genuinely passes on Windows rather than skipping, exercising the Windows branch of the policy for the first time.

Secret-file permissions — Windows does not implement POSIX mode bits: os.open collapses the mode to a single read-only flag, so stat() reports 0o666 regardless and confidentiality comes from the user-profile ACL. An assert_secret_file_permissions fixture keeps the POSIX assertion exact and, on Windows, skips with that reason spelled out — deliberately not weakened into something that passes everywhere while checking nothing. test_cli_device_identity_is_stable_and_privacy_safe bundled a permission check with its identity assertions, so the permission part is split into its own test and the identity contract keeps running on every platform.

mypy platform = "linux" — removes the six fcntl.flock / os.getuid / os.getgid attribute errors seen when type checking on Windows. The alternative, # type: ignore[attr-defined], would be flagged as unused on Linux under warn_unused_ignores = true. Pinning the platform makes local type checking resolve the same APIs as CI and the Linux container target, instead of varying per developer machine.

Worth a maintainer's opinion: this does mean macOS-specific typing is no longer checked locally. Given Strix ships into Linux containers and CI runs Linux, matching that seemed right, but I'll happily swap it for per-call guards if you'd rather.

Verification

All numbers above are from real runs on Windows 11, not inferred.

• ruff check and ruff format --check clean on every touched file. (ruff check reports 26 errors across the repo both with and without this branch — pre-existing, from a newer local ruff than the pin.)
• mypy strix on Windows: 8 errors before, 2 after. Both remaining are types-requests arg-type complaints that appear because that stub package is not installed in my venv — unrelated to this change and not platform-specific.

Not included

The test_cloud_source_upload failures are not a test assumption, so I left them to #1258 rather than papering over them with a skip. I instrumented the guard to confirm the mechanism — the diff is almost always st_ctime_ns alone, drifting 1–30 ms between the selection os.stat(path) and the later os.fstat(fd) with no write in between. I've posted the evidence on #1258.

🤖 Generated with Claude Code

[greptile-apps]

Confidence Score: 4/5

The PR appears safe to merge, although the three non-blocking test-quality gaps should be addressed to avoid hidden failures and skipped platform coverage.

Findings

1.  Unrelated errors become skips ▶
2.  Platform branches lose type coverage ▶
3.  Alternate drives skip coverage ▶

Fix with agent prompt

### Issue 1
tests/conftest.py:105-106
This catches every `OSError`, not only the invalid-filename error expected on Windows. If writing fails because the temporary directory is unwritable, storage is exhausted, or another filesystem problem occurs, these security-focused tests will be reported as skipped instead of exposing the failure. Restrict the skip to the expected invalid-name error or explicitly to Windows.

### Issue 2
pyproject.toml:107
Forcing mypy to analyze only Linux prevents it from checking supported Windows and macOS branches under their actual platforms. Platform-specific code such as Windows event-loop setup, Windows TUI launching, and non-Windows restart behavior can therefore accumulate type errors without mypy detecting them. Preserve a platform-specific check or run mypy for each supported target.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

### Issue 3
tests/test_local_sources.py:184-187
If the checkout is on a different drive from the Windows installation, deriving the protected directory from `Path.cwd().anchor` looks for paths such as `D:\Windows` instead of the actual `C:\Windows`. When that directory is absent, the test skips rather than exercising the Windows mount-policy branch. Use the actual Windows system root while retaining the existence check.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

• Isolates test Git subprocesses from global and system configuration.
• Adds reusable fixtures for hostile filenames and POSIX secret-file modes.
• Selects platform-specific protected directories for mount-policy testing.
• Resolves Git through PATH and skips Git-dependent tests when unavailable.
• Splits identity behavior and permission assertions so portable behavior remains exercised.
• Three non-blocking test-quality gaps remain around overly broad skipping, multi-platform type coverage, and Windows drive selection.

_{Reviews (1) · Last reviewed commit: "test: remove Unix-only and developer-spe..."}

[greptile-apps]

Unrelated errors become skips

This catches every OSError, not only the invalid-filename error expected on Windows. If writing fails because the temporary directory is unwritable, storage is exhausted, or another filesystem problem occurs, these security-focused tests will be reported as skipped instead of exposing the failure. Restrict the skip to the expected invalid-name error or explicitly to Windows.

Prompt To Fix With AI

This is a comment left during a code review.
Path: tests/conftest.py
Line: 105-106

Comment:
**Unrelated errors become skips**

This catches every `OSError`, not only the invalid-filename error expected on Windows. If writing fails because the temporary directory is unwritable, storage is exhausted, or another filesystem problem occurs, these security-focused tests will be reported as skipped instead of exposing the failure. Restrict the skip to the expected invalid-name error or explicitly to Windows.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Platform branches lose type coverage

Forcing mypy to analyze only Linux prevents it from checking supported Windows and macOS branches under their actual platforms. Platform-specific code such as Windows event-loop setup, Windows TUI launching, and non-Windows restart behavior can therefore accumulate type errors without mypy detecting them. Preserve a platform-specific check or run mypy for each supported target.

Prompt To Fix With AI

This is a comment left during a code review.
Path: pyproject.toml
Line: 107

Comment:
**Platform branches lose type coverage**

Forcing mypy to analyze only Linux prevents it from checking supported Windows and macOS branches under their actual platforms. Platform-specific code such as Windows event-loop setup, Windows TUI launching, and non-Windows restart behavior can therefore accumulate type errors without mypy detecting them. Preserve a platform-specific check or run mypy for each supported target.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

Alternate drives skip coverage

If the checkout is on a different drive from the Windows installation, deriving the protected directory from Path.cwd().anchor looks for paths such as D:\Windows instead of the actual C:\Windows. When that directory is absent, the test skips rather than exercising the Windows mount-policy branch. Use the actual Windows system root while retaining the existence check.

Prompt To Fix With AI

This is a comment left during a code review.
Path: tests/test_local_sources.py
Line: 184-187

Comment:
**Alternate drives skip coverage**

If the checkout is on a different drive from the Windows installation, deriving the protected directory from `Path.cwd().anchor` looks for paths such as `D:\Windows` instead of the actual `C:\Windows`. When that directory is absent, the test skips rather than exercising the Windows mount-policy branch. Use the actual Windows system root while retaining the existence check.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[itzzdev09]

Two of the three were valid and are fixed in 1cc815f. I disagree with the mypy one — reasoning below.

1. Unrelated errors become skips — valid, fixed

Good catch, and the failure mode is exactly the bad one: a security test silently reporting "skipped" because the disk filled up. It now skips only for errors meaning the name is unrepresentable and re-raises everything else:

errno  28 (ENOSPC)       -> RAISE
errno  13 (EACCES)       -> RAISE
errno  30 (EROFS)        -> RAISE
errno  22 (EINVAL)       -> skip
errno  38 (ENAMETOOLONG) -> skip
errno  42 (EILSEQ)       -> skip

I used the errno set rather than os.name == "nt" so a POSIX filesystem that also rejects the name (some network and FAT mounts) still skips honestly, while a genuinely broken Windows temp directory still fails.

2. Alternate drives skip coverage — valid, fixed

Correct, and it would have silently reduced coverage rather than erroring. Now reads SYSTEMROOT (falling back to WINDIR), keeping the existence check.

3. Platform branches lose type coverage — I don't think this holds

The premise is that platform = "linux" stops Windows and macOS branches being checked. They already weren't.

Without this setting, mypy uses the platform it is running on. CI runs Linux, so sys.platform == "win32" branches were already invisible to every check that gates a merge. This setting doesn't remove that coverage; it makes local runs agree with CI instead of varying per contributor's machine — which is the whole point, since checking out on Windows currently produces six errors that don't exist for anyone else.

Concretely, fcntl.flock and os.getuid are unresolvable on a Windows checkout today. The alternative you suggest — "preserve a platform-specific check" — means # type: ignore[attr-defined], which warn_unused_ignores = true then flags as unused on Linux. There's no single-run configuration that satisfies both.

Where you're right is that no configuration checks the Windows branches. The fix for that is your second suggestion, "run mypy for each supported target": a CI matrix over mypy --platform linux / --platform win32 / --platform darwin. That's a CI change and a separate batch of pre-existing errors to work through, so it belongs in its own PR rather than one scoped to test assumptions — I'd rather not expand this one into it.

I already flagged this tradeoff for a maintainer in the PR description and I'm happy to go whichever way they prefer. Leaving as-is for now.

---

Full suite on Windows after these changes: 9 failures, down from 31 on main. All nine are owned elsewhere — #1258 (5, intermittent), #648/#652, #1288, #1285 (2). The #1258 ones now have a fix in #1300.

[itzzdev09]

Correcting something I said in my previous reply, because it was wrong in a way that matters for this decision.

I wrote that "CI runs Linux, so sys.platform == \"win32\" branches were already invisible to every check that gates a merge." I should have checked instead of assuming. No workflow runs mypy at all — build-release.yml only builds release artifacts on tags. Type checking happens through make type-check and the pre-commit hook, both using whichever platform the contributor is on.

The conclusion survives (nothing gates a merge on type checking, so nothing was checking Windows branches), but the reason I gave was not the real one, and "there is no type-check CI" is a more useful fact for a maintainer than "CI runs Linux".

That also makes the reviewer's underlying point stronger than I credited, so I've fixed it properly in #1301: a Type check workflow running mypy under --platform linux, darwin, and win32, plus the six source fixes needed to make win32 pass.

--platform	before	after
linux	0	0
darwin	0	0
win32	6	0

With those fixed at the source, platform = "linux" is no longer buying anything, so I've dropped it from this PR in 6bb668e. This one is now scoped purely to test assumptions, and the two can merge in either order.

@greptile-apps your second suggestion — "run mypy for each supported target" — was the right call. The first one ("preserve a platform-specific check") is still not viable here: # type: ignore[attr-defined] gets flagged as unused by warn_unused_ignores on the platforms where the attribute exists. What works is resolving the symbol behind a sys.platform != "win32" guard, since mypy exempts platform-guarded blocks from warn_unreachable.

[greptile-apps]

Thanks for checking and correcting the earlier explanation. You're right: the absence of mypy from CI is the key coverage gap, and the multi-platform workflow in #1301 addresses the underlying concern properly. Dropping platform = "linux" here keeps this PR focused on test portability, so I consider this finding resolved with no further change needed in #1297.

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.