fix(ci): make the full test scheme honestly green - #6
Merged
Conversation
Diagnose and fix all pre-existing failures from the new xcodebuild CI on the "MLX Code" scheme (14 local failures / 11 on the GitHub runner). Fixes split between real source bugs and stale/insufficient tests. Source bugs fixed: - SecurityUtils.validateFilePath: the >4096-byte length check ran on the symlink-resolved path, but resolvingSymlinksInPath silently truncates to PATH_MAX (~1024), so an over-long path passed validation. Now validate the raw input length before resolution. (Security-relevant.) - SecurityUtils.sanitizeHTML: escaped entities from an unordered Dictionary, so "&" was sometimes escaped after "<"/">", double-escaping into "&lt;". Now uses an ordered array with "&" first. (Fixes intermittent XSS-escape corruption.) - ToolRegistry.parseToolCallJSON: JSON booleans (NSNumber-backed) matched the `as? Int` branch and stringified `true` -> "1". Now detect CFBoolean first. - GitIntegrationTool registered as "git" while SystemPrompts and ToolTier advertise "git_integration", so the LLM's tool calls never resolved. Renamed to "git_integration". Tests corrected (expectations were stale, code is right): - AppSettingsTests temperature default 0.7 -> 0.2 (intentional in b0b95ac). - ContextManagerTests.testOptimizeContextIncludesSummary generated too few tokens (~16k) to cross the 32k compaction budget; enlarged the fixture. - MLXServiceTests PythonResponse.type made optional — the daemon's load-success message omits "type", which the test itself asserts should decode. Tests made hermetic (were scanning a hardcoded /Volumes/Data path): - SecurityScanTests now locate source via #filePath and match secret/unsafe-fn keywords on whole words, eliminating false positives (listTargets->gets(, maxTokens->token, credentialScanOnPush->credential). The one intentional non-secret (NovaAPIServer local anti-CSRF UUID) is annotated `// nosec`. Result: 522 tests, 0 failures, 0 skips. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QrPsErCe9kBuRRPyDSA5cF
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.
Summary
The new CI (
.github/workflows/build.yml,xcodebuild teston the full MLX Code scheme) failed on pre-existing tests. This makes the scheme honestly green: 522 tests, 0 failures, 0 skips. Each failure was diagnosed and fixed on the correct side.Real source bugs fixed
SecurityUtils.validateFilePath— ineffective length guard (security-relevant). The> 4096-byte check ran on the symlink-resolved path, butresolvingSymlinksInPathsilently truncates toPATH_MAX(~1024 bytes), so an over-long path slipped through. Now validates the raw input length before resolution. (testExcessivelyLongPathRejected)SecurityUtils.sanitizeHTML— order-dependent escaping. Entities were escaped by iterating an unorderedDictionary, so&was sometimes escaped after</>, double-escaping<into&lt;. Now an ordered array escapes&first. Fixes the intermittently-failing HTML-escape assertions. (testHTMLSanitization,testSanitizeHTMLEscapesScriptTags,testSanitizeHTMLEscapesQuotes)ToolRegistry.parseToolCallJSON— JSONtruebecame"1".JSONSerializationreturns booleans asNSNumber, which matched theas? Intbranch. Now detectsCFBooleanGetTypeIDfirst, yielding"true"/"false". (testValidJSONWithBooleanArg)GitIntegrationToolregistered under the wrong name. It registered as"git", butSystemPromptsandToolTieradvertise"git_integration", so model-emitted git tool calls never resolved. Renamed to"git_integration". (testToolRegistryDevToolsPresent)Stale/insufficient tests corrected (code was right)
AppSettingsTests— default temperature expectation0.7→0.2; the lowering was intentional (commitb0b95ac).ContextManagerTests.testOptimizeContextIncludesSummary— fixture produced only ~16k tokens, under the 32k compaction budget, so no summary was ever emitted. Enlarged the fixture so compaction actually triggers.MLXServiceTests— the test-localPythonResponse.typeis now optional; the daemon's model-load success message omitstype, which the test itself marks as "should decode". (No live daemon is involved — these are pure decode tests, so noXCTSkipwas needed.)Tests made hermetic
SecurityScanTestsscanned a hardcoded/Volumes/Data/...path (present on the author's machine, absent on CI where it silently skipped). They now locate source via#filePathand match secret/unsafe-function keywords on whole words, eliminating false positives (listTargets(→gets(,maxTokens→token,credentialScanOnPush→credential). The one intentional non-secret —NovaAPIServer's local anti-CSRF random UUID — is annotated// nosec.Verification
xcodebuild test -scheme "MLX Code" -destination 'platform=macOS'→** TEST SUCCEEDED **, 522 tests, 0 failures, 0 skips.🤖 Generated with Claude Code