fix: unit tests failing on the macOS test gate (autocomplete, password command, MCP)#1587
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0248fc7d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let remainingStdout = stdoutPipe.fileHandleForReading.readDataToEndOfFile() | ||
| if !remainingStdout.isEmpty { stdoutCollector.append(remainingStdout) } | ||
| let remainingStderr = stderrPipe.fileHandleForReading.readDataToEndOfFile() |
There was a problem hiding this comment.
Avoid blocking on inherited pipes after command exit
When a password command starts a background child that inherits stdout or stderr (for example printf secret; sleep 999 &), waitUntilExit() returns for the shell and the timeout task is cancelled, but these readDataToEndOfFile() calls wait for EOF from every remaining pipe writer. In that scenario the resolver hangs indefinitely instead of returning the captured password or timing out, so command-based passwords can block connection attempts.
Useful? React with 👍 / 👎.
The macOS test gate had four failing assertions across three suites. None of them are in code the open Snowflake PR (#1580) touches, so they were already red on
mainand turned the gate red on every PR. Each fix is in source, not the tests.Fixes
fix(editor): quoted schema in qualified table referencesThe table-reference regex matched an optional quote, then
[\w.]+, then an optional quote. ForFROM "sales".ordersthe opening quote was consumed first, the character class stopped at the closing quote, and.orderswas dropped, so the analyzer recorded a table namedsales. The capture now matches optionally-quoted dotted segments and strips quotes per segment for both the table and the schema.Fixes
SQLContextAnalyzerTests.testSchemaSegmentQuotingStripped.fix(connections): full output of a fast password commandresolveCommandread stdout through areadabilityHandlerand stopped atwaitUntilExit. For a small, fast command the only chunk could arrive after the handler was removed, so the password came back empty (.emptyPassword). It now drains the remaining stdout and stderr after the process exits, the same pattern used elsewhere in the app.Fixes
PasswordSourceResolverTests.commandPreservesSpaces.fix(mcp): cancellation and progress before the connection lookupToolConnectionMetadata.resolveran before the cancellation check and the first progress emit, so a request that was already cancelled returnedinvalidParamsand no progress notification fired. The early cancellation check and the initial progress emit now run before the connection is resolved.Fixes
ExecuteQueryToolTests.cancellationPropagatesandprogressEmittedWhenTokenPresent.Verification
xcodebuild teston the three suites:** TEST SUCCEEDED **, 322 cases, 0 failures, 0 recorded issues.swiftlint lint --stricton the changed files: clean.The autocomplete fix folds into the unreleased #1581 entry, so it has no separate changelog line; the other two are added under Fixed.