Conversation
os.UserHomeDir reads $HOME on Unix but %USERPROFILE% on Windows, so a Windows process could not relocate its store the way it can everywhere else. That asymmetry was not academic: it pointed the test suite at the real ~/.spettro, where it read and would have overwritten live credentials, and drove real API calls. internal/homedir resolves the user's home directory with consistent semantics everywhere: $HOME is honoured on every platform, with a Windows-only requirement that the value be a genuine absolute path (drive- or UNC-rooted). The POSIX-style HOME that Git Bash and MSYS2 export to native children, such as /c/Users/alice, is rejected so the rest of the codebase does not have to defend against it. Co-Authored-By: Spettro <spettro@eyed.to>
The ~/.spettro stores hold real secrets — provider API keys in keys.enc, the master key that decrypts them, MCP auth tokens, Telegram bot tokens — and the codebase protects them with 0600/0700 modes. Those modes are no-ops on Windows: os.Chmod there only toggles the read-only attribute, and a new file simply inherits its parent directory's ACL. A project checkout on a shared drive or outside the user profile therefore gets no protection at all from a mode argument. internal/fsperm expresses the intent directly. RestrictToOwner chmods on Unix and installs a non-inherited DACL naming only the current user on Windows; IsOwnerOnly is the matching predicate so tests can assert the guarantee in the platform's own terms rather than comparing Unix mode bits Windows does not implement. SecureMkdirAll restricts the directory itself, which is what makes this tractable: on Windows the entry is inheritable, so every secret written inside afterwards starts out owner-only without each write site having to remember. Co-Authored-By: Spettro <spettro@eyed.to>
A read-modify-write store on Unix renames its temp file over the live one atomically; readers polling the file see either the old or the new contents, never a torn write. Windows does not behave that way: Go opens files without FILE_SHARE_DELETE, so any reader that has the destination open turns the rename into a sharing violation and concurrent saves fail outright. internal/safeio hides the difference. Replace retries the rename briefly on the transient errors — access denied while a reader holds the file, ERROR_SHARING_VIOLATION while an antivirus or indexer has it open, both of which clear in milliseconds — and the predicate is uniform across platforms, with a Unix-side sentinel that matches nothing. ReadFile mirrors the fix on the reader side, where a poller that lands inside a save window currently fails entirely; the retry is bounded at a hundred milliseconds, far longer than a small JSON save needs and short enough that a genuinely stuck file still reports an error promptly. Co-Authored-By: Spettro <spettro@eyed.to>
Co-Authored-By: Spettro <spettro@eyed.to>
Co-Authored-By: Spettro <spettro@eyed.to>
Co-Authored-By: Spettro <spettro@eyed.to>
Co-Authored-By: Spettro <spettro@eyed.to>
Co-Authored-By: Spettro <spettro@eyed.to>
Co-Authored-By: Spettro <spettro@eyed.to>
Co-Authored-By: Spettro <spettro@eyed.to>
Co-Authored-By: Spettro <spettro@eyed.to>
Two small corrections to bring the tests in line with what each host actually guarantees. shell.classify(): baseName() recognises both '/' and '\' when stripping the directory part of an interpreter spec. Without that, a Windows spec read on a Linux build (and vice versa, in cross-build setups) kept its backslash intact, so 'pwsh.exe' inside 'C:\Program Files\...' was classified as a POSIX shell. Classification now depends on the spec, not on the separator of the platform reading it. fsperm tests: the 'files created in a secured directory inherit owner- only' assertion only held on Windows, where the directory DACL is inheritable. On Unix, a new file's mode comes from its creation mode and the umask, so what protects a secret there is that no other account can traverse a 0700 directory. The test now asserts the guarantee the host actually makes (file on Windows, directory on Unix) and stops being umask-sensitive. Co-Authored-By: Spettro <spettro@eyed.to>
git reports the toplevel with symlinks already resolved (e.g. /var/... on macOS becomes /private/var/...), so a raw filepath.Rel between the resolved toplevel and an unresolved cwd always appeared to escape the repo and the delegated subagent silently landed at the worktree root. EvalSymlinks both sides before the comparison, and replace the strings.HasPrefix(rel, "..") check (which also rejected real directories named "..foo") with a helper that tests the path components properly. The test fixture is resolved the same way for the same reason: t.TempDir on macOS sits under a symlink, so the prior setup compared two spellings of the same directory. Co-Authored-By: Spettro <spettro@eyed.to>
Drop the python -m http.server dependency in TestBackgroundHTTPServer. The helper is now the test binary itself, which switches into a long-lived HTTP server when SPETTRO_TEST_HTTP_ADDR is set. This avoids a flaky start-up on macOS CI (socket.getfqdn between bind and listen) and stops the case from silently skipping on hosts without python. shelltest gains Exec(path, args...) for quoting an external binary, including the & call operator that PowerShell needs to run a quoted path as a command. Co-Authored-By: Spettro <spettro@eyed.to>
LSP servers (clangd, gopls, …) keep OS handles on the workspace after the agent stops using them. On Windows this made two things fail: - Manager.Restart followed by file replace/remove — the old server still held the file open. - TempDir cleanup at the end of TestGopls* / TestZeroConfigClangd — the TempDir could not be removed while the server was alive. Add Manager.Shutdown that closes every client and drops the manager from the registry, and have Client.Close wait up to 5s for the process to actually exit after Kill. Wire Shutdown into the e2e tests as t.Cleanup so the TempDir is releasable. Co-Authored-By: Spettro <spettro@eyed.to>
- gofmt struct field alignment in internal/acp/ext_providers.go - fix stray quote in PowerShell comment in shelltest.go - drop blank trailing line in internal/update/apply.go - tidy import grouping in internal/provider/images_test.go Co-Authored-By: Spettro <spettro@eyed.to>
A live language server holds handles on workspace files, which on Windows blocks the delete-or-replace done by /update and checkpoint restore. Add lsp.ShutdownAll and call it on both the headless and interactive session exits, alongside the existing jobs.KillAll / pty.KillAll / spool cleanup. Move Close() out of the manager lock so a wedged server does not stall every other LSP call; close the (already-unregistered) clients in parallel. On Windows a sandboxed child cannot write the real per-user temp directory, because that directory sits at Medium integrity and the child runs Low. Redirect its TEMP and TMP to a spettro-owned scratch dir, share one low-integrity token across the session (a token handed to SysProcAttr.Token is never closed by os/exec, so minting one per command would leak), and expose that scratch dir through sandbox.WritableTempDirs so the in-process file tools refuse exactly what the shell layer would refuse. Release Windows job-object handles once a job has no live processes; the only entries that outlive their job are the children the KILL_ON_JOB_CLOSE backstop exists to reap, and that backstop needs the handle kept until spettro exits. Release it on all the other jobs (the common case) so we do not accumulate one handle per shell call for the life of the session. Replace literal '/dev/null' and '/tmp' with os.DevNull / writableTempDirs() in the git invocations that have to run unchanged on Linux and Windows; document install.ps1 flags, which the 'irm | iex' form has nowhere to put. Co-Authored-By: Spettro <spettro@eyed.to>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.