Re-architecture: one modular package, explicit composition root, typed seams - #2192
Open
matthijseikelenboom wants to merge 336 commits into
Open
Re-architecture: one modular package, explicit composition root, typed seams#2192matthijseikelenboom wants to merge 336 commits into
matthijseikelenboom wants to merge 336 commits into
Conversation
…ditorManager dependency
Racing a fresh SearchState's background indexing against the next test's directory teardown/recreate on the same path caused intermittent host process crashes. Poll-until-timeout also replaces fixed sleeps for the sync assertions themselves.
…rkspaceNavigator.reveal
…View and sever Editor dependency
…nder loop in StatusBarFileInfoView
The question 'why is this a target for one file' has an answer that was nowhere written down: 19 files across CESourceControl and CELSP depend on ShellClientProtocol and none on the implementation, which only the app target composes. States what the boundary actually buys — a manifest edit is needed to reach the implementation, which is visible in review — rather than claiming the import audit forbids it. It does not: import honesty checks that imports are declared, so a feature that declared the dependency would pass. Records the coherent alternative too, so the question is settled rather than merely answered once.
Two files in Styles/ were not styles. View+actionBar is a plain extension View providing a modifier, and MenuWithButtonStyle declares a View — a menu drawn to resemble a bordered button — not a MenuStyle conformer. Both move to Views/. MenuWithButtonStyle is renamed ButtonStyledMenu: the old name reads as a style type, which is what put it in the wrong folder. One consumer. CodeEditUI is otherwise left alone. Grouping by kind is wrong inside a feature, but this target is a component library with no feature semantics by charter, so Styles/, Views/ and EnvironmentKeys/ are the subject — the same terms SwiftUI itself is documented in. Imposing subjects here would mean several two-file folders; SplitView/ stays the one genuine subsystem.
Its Styles/Views/EnvironmentKeys grouping looks like the kind-grouping this rule forbids. It is not: the rule targets layering inside a feature, and this target is a component library with no feature semantics by charter, so kind is the subject a consumer browses by. Also reframes the remaining count. Of the eight folders, only two are actual work — CESearch's, pending its rebuild. The rest are the excluded Accounts/, the two stated exceptions, and CodeEditSettings waiting on its naming question.
Names the disposition of all twelve so the question 'did anyone look at X' has an answer. CodeEditDocument and CEWorkspaceFileManager needed nothing — both are flat and under the size where the convention asks for groups — which is why they never appeared in the kind-folder counts.
Both stay, and the conclusions depend on each other. CEEditor and CELSP reference each other zero times in either direction; what keeps them apart is LanguageServicesProvider, declared in CodeEditDocument, implemented in CELSP and consumed by CEEditor through an environment key — with a no-op implementation so the editor works with no language service at all. CodeEditDocument is therefore the contract between two independent features, not a leftover holding a document type, and CodeFileDocument's AppKit/SwiftUI imports bar it from Core. CELSP is not editor-internal either: its consumers are the settings UI, the utility area and app lifecycle, and 28 of its 78 files install language servers rather than edit text. Also removes an unused 'import CodeEditDocument' from CELSP's LanguageServerDocument.swift — its only mention of CodeFileDocument is a doc comment. Import honesty checks that imports are declared, not that they are used, so nothing flagged it.
Untouched since 2025-01-05. Of the 34 symbols its landing page linked, 9 no longer exist and 13 moved into package targets — DocC documents one module, so an app-target catalog cannot resolve CEWorkspaceFile, FileIcon or ShellClient any more. Ten of the 34 were still app-target symbols. Repair could not have succeeded. The catalog's model — one app target, one documented module — stopped matching a codebase of twelve library targets and a thin shell, and the only thing it could still document is the shell, which is the part least needing an external-audience explainer. Its section names were the retired Features/ folders, and AppPreferences/ was nine files of tutorial for the god object ARCHITECTURE.md names as a cause of the 2022 collapse. Nothing referenced it: no inbound links, absent from Package.swift and CI, 19 months stale while compiled in the app target's Sources phase. This matches what the CodeEditApp org actually does. All five libraries it publishes — CodeEditSourceEditor, CodeEditTextView, CodeEditKit, CodeEditSymbols, CodeEditLanguages — ship a catalog at Sources/<Target>/Documentation.docc, because a published library has readers who never open its source. None of CodeEditModules' twelve targets has an external consumer, so none needs one; a catalog is optional anyway, since DocC generates symbol docs from doc comments without it. If a target is ever extracted for publication, that is when it gains a catalog. Four project.pbxproj entries removed with it: the catalog was an explicit file reference in the Sources build phase, not a synchronized group, so deleting the folder alone would have broken the build. Also fixes ThemeSettingsView's header, which still named the deleted ThemePreferencesView.
67 headers named a file they were not, all fossils of renames the header comment never followed. Most record the old vocabulary directly: ThemePreferences, KeybindingsPreferences and AccountsPreferences for the retired Settings names; CommandPaletteView for QuickActionsView; TabManager for EditorManager; OutlineViewController for ProjectNavigatorViewController; SourceControlModel for SourceControlManager. Two were plain typos (AccoundSelectionView, OutlintViewController). Header-only: 67 files, 67 insertions, 67 deletions, and every changed line begins '// '. Found while researching the DocC catalog — ThemeSettingsView's header still named the deleted ThemePreferencesView, which suggested the pattern was wider. Note for anyone re-running the check: scope it to CodeEdit/, CodeEditModules/Sources/, CodeEditModules/Tests/, CodeEditTests/ and CodeEditUITests/. A bare find over CodeEditModules/ walks into .build/checkouts and reports dependency sources — that inflated the first count from 67 to 139.
…spaceFileManager
The norm was justified in the abstract, so it read as decorative. It now carries
what it actually prevents: merging CEWorkspaceFileManager into Core. That target
holds 50 FileManager calls and a full FSEvents implementation with a C callback
and its own dispatch queue. Without the norm the merge looks reasonable, since
that target depends on nothing but Core and folding it in removes a target. With
the norm it is obviously wrong, because it would put a live filesystem event
stream in the sink all twelve targets rest on.
The testability half is now empirical rather than asserted: CodeEditCoreTests is
five files with zero FileManager, temporaryDirectory or Data(contentsOf:) use.
Records CEWorkspaceFileManager alongside ShellClient. Core declares
WorkspaceFileProviding, CEWorkspaceFileManager.swift:263 conforms to it, four
CEEditor files depend on the protocol, and no package imports the
implementation. Contract in Core, adapter in its own target, app composes.
Also fixes a typo from the em-dash pass ('organized an,' to 'organized and,').
86 down to 14. The remainder are the two cases worth keeping: 10 columns in the
topology diagram, where they align target names with their descriptions, and 4
separators between a bold label and its explanation in list items.
Done in two passes, because a mechanical substitution produces bad prose. The
first replaced the dashes; the second fixed what that broke, roughly 35 places.
Comma splices became colons or full stops ('the acyclicity guarantee, because it
makes Core a sink', 'not prevention: import honesty checks'). Paired dashes that
had been holding a parenthetical became actual parentheses, which mattered most
where a list would otherwise read wrong: 'across CESourceControl and CELSP
(GitClient, SourceControlManager, ...)' had briefly read as though GitClient were
a package.
73 lines changed, structure untouched: 19 headings, 4 code fences, 44 table rows
before and after.
It was hard-wrapped at 100 columns, which nothing in the repo asks for. README.md has a median line length of 142 and CONTRIBUTING.md 85, so this file was the odd one out, and every Markdown renderer soft-wraps regardless. The cost was real: hard wrapping reflows a whole paragraph when one sentence changes, so the em dash pass touched 73 lines to make mostly single-word edits. One sentence per line means a sentence edit changes one line. Verified content-preserving rather than assumed. Prose hashes identically with whitespace collapsed, code blocks are byte-identical, word count is 6484 before and after, and the sequence of block-start indents hashes identically so every list continuation keeps its nesting. The first attempt did not: it flattened 21 indented continuation paragraphs to column zero, which would have re-rendered them as siblings of their list items instead of part of them.
…ion log The repo had two files called ARCHITECTURE.md. The root one, an untracked Architecture Vision file excluded via .git/info/exclude, was last touched on 26 July and described the tiered Packages/ layout that the consolidation replaced. The tracked docs/ARCHITECTURE.md was the maintained one. Reading the stale file while I edited the other is what surfaced this. The guide now sits at the root, where a newcomer looks alongside README and CONTRIBUTING, and is tracked normally with the exclude entry removed. Three sections were merged from the vision file first, each verified against the code: the design principles (point 8 rewritten, since it described the retired Packages/ tiers), state ownership (reworded to stop overloading the word service, because git status, tasks and LSP sessions live in feature targets rather than service targets), and the rejected-options list. Two claims in that list were wrong and are corrected. The @observable entry justified itself with a macOS 13 minimum, but the declared target is 14 in both the pbxproj and the manifest, so the blocker is team agreement rather than code. The Combine entry claimed the EventBus could swap to AsyncStream without touching call sites; subscribe(_:) returns AnyPublisher and all five subscribers use sink and AnyCancellable, so it would rewrite every one. The guide dropped from 557 to 435 lines by moving decision records to docs/architecture-decisions.md: the four target-separation rulings, what the I/O norm prevents, the panel seam detail, the Core charter evidence, and the singletons inventory. What stayed is the rule plus its reasoning. Panel contributions shrank from 41 lines to the reusable pattern, since it is the first of several seams and logging each in full would bloat the guide. Folder conventions lost its per-target history, which is the only content deleted rather than moved: 170 words. Also removes the em dash separator from CONTRIBUTING.md prose, per the writing style rule.
matthijseikelenboom
requested review from
0xWDG,
FastestMolasses,
austincondiff,
thecoolwinter and
tom-ludwig
August 22, 2026 08:17
matthijseikelenboom
added
documentation
CI failed to compile CodeEditSettings with four errors in SettingsValue.swift: EnvironmentObject's init and its wrapped value are @mainactor in the SDK, so a nonisolated property wrapper touching them is rejected under Swift 6 strict concurrency. It compiled locally because this machine runs Xcode 26.6 while the runner uses Xcode 16.4, and newer SwiftUI carries @preconcurrency annotations that soften the isolation. The app-side @appsettings wrapper has the same shape and is unaffected, because the app target is still Swift 5. The isolation is correct on the merits, not just a way to satisfy the compiler. SettingsValue is documented as valid only inside a View, and PersistentSettingsStore.setValue already asserts the main thread. SwiftUI's own @StateObject and @ObservedObject are main-actor wrappers used the same way. Nothing in the branch needs a newer SDK, so the runner is adequate: the 37 #available(macOS 26) guards are version checks that compile against any SDK, GlassEffectView is our own NSViewRepresentable, and the only Apple glass API reference in the tree is commented out. Upgrading the runner would have hidden this rather than fixed it. Cannot be verified locally, since Xcode 26.6 does not produce the error. CI is the check.
It crosses GitClientProtocol, which is Sendable, and holds only [GitChangedFile], which is Sendable too. Every other type returned by that protocol already declares the conformance, so Status was the lone outlier. Xcode 26 accepts the call site without this because its region-based isolation analysis can prove the value is disconnected there. Xcode 16.4, which the CI runner uses, cannot, and asks for the conformance instead.
These values are produced by an AsyncSequence and consumed on the main actor, so they cross an isolation boundary. The struct holds a Double and a payload-free internal enum, so it was already Sendable in fact and only lacked the declaration. Same toolchain split as GitClient.Status: Xcode 26 proves the value is disconnected at the call site, while the CI runner's Xcode 16.4 asks for the conformance.
The method had no callers anywhere in the repo, was internal to CESearch so nothing outside could reach it, witnessed no protocol requirement, and was not @objc, so no dynamic dispatch could find it either. It also built an NSAlert and called runModal() from a synchronous nonisolated method on a nonisolated class, which presents a modal alert from an arbitrary thread. Xcode 26 accepts this; the CI runner's Xcode 16.4 rejects it, correctly. The three other NSAlert sites in the packages all sit on @mainactor types and are unaffected. Removing this one leaves the file with no AppKit dependency, so that import goes too: its only other NS use, NSString.CompareOptions, is Foundation.
Both coordinators write a main-actor binding on their NSViewRepresentable parent from inside a Combine sink on NSMenu.didSendActionNotification. AppKit posts that notification on the main thread, so the write was always main-thread in practice, but nothing said so. This mirrors the fix already used by FindNavigatorResultList's coordinator in the same package: annotate the coordinator, then state the invariant at the mutation with MainActor.assumeIsolated, which traps if it is ever violated. CESearch is the one CI reported, since it is Swift 6. CEEditor has the identical construct and is silent only because it declares swiftLanguageMode(.v5), so it is fixed here too rather than left to fail when that exception is lifted.
The class is @mainactor, but both methods override nonisolated declarations on XCTestCase, and an override cannot add isolation the superclass lacks. So they stayed nonisolated while the property they assign and the initialiser they call are main-actor. XCTest runs setUp and tearDown on the main thread for synchronous test cases, so MainActor.assumeIsolated states that invariant instead of weakening the isolation of the view model. Swept the other seventeen test files with these overrides: no other package test is affected. The remaining @mainactor ones live in CodeEditTests, which is Swift 5.
Supersedes the previous attempt on this file. MainActor.assumeIsolated was the wrong tool: its closure captures self, the XCTestCase, which is not Sendable, so sending it into a main-actor closure from a nonisolated override is itself a data race the compiler rejects. A lazy property needs no escape hatch. Its getter is main-actor because the class is, and XCTest creates a fresh test-case instance per test method, so each test still gets its own view model. tearDown only nilled the property, which per-test instances already handle.
NSDocument is main-actor isolated but declares read(from:ofType:) and presentedItemDidChange() nonisolated, since AppKit may call them off the main thread. Both touch main-actor document state, which was silent in the Swift 5 app target and is six errors now the file lives in a Swift 6 package target. The failing lines are byte-identical on main. Three changes, none of them a behaviour change: 1. canConcurrentlyReadDocuments(ofType:) is overridden to false. That is already AppKit's default; stating it pins the invariant the read path relies on, which was previously unwritten. 2. read(from:ofType:) states its main-actor isolation, and registerContentChangeUndo takes a String rather than an NSString so nothing non-Sendable is captured. 3. presentedItemDidChange() consults its main-actor state through the same Thread.isMainThread branch notifyLSPDidOpen() already uses. An unconditional DispatchQueue.main.sync deadlocks, because the tests call this on the main thread while NSFileCoordinator does not. This is a bridge, not a resolution: assumeIsolated states what the compiler cannot check, and the runtime branch stands in for a static guarantee. Recorded as deferred in docs/architecture-decisions.md and in a doc comment, with the redesign sketched. The external-changes section moved to its own file to stay under the 400-line lint limit.
The timer block optional-chained self on every access, including inside a MainActor.assumeIsolated closure. Region-based isolation cannot treat self as disconnected there, because the escaping timer block still shares it, so the isolated call reads as sending self. Binding once with a guard gives the analysis a local value and changes no semantics: the capture stays weak, so there is still no retain cycle, and the strong binding lasts one firing, which repeated self? accesses already amounted to. This surfaced only after the type errors in this file were cleared: the sending diagnostic is a SIL pass that runs after type checking succeeds, so it never reached this code before.
read(from:ofType:) used a bare MainActor.assumeIsolated, justified by canConcurrentlyReadDocuments(ofType:) being pinned to false. That justification was wrong: the pin constrains AppKit's own reads and says nothing about an in-process caller constructing a document off the main actor. Commit 1985839 records that exact shape trapping here before and taking twenty unit tests down with it. All four nonisolated-override sites now branch on Thread.isMainThread and assume isolation only on the main-thread side. read and presentedItemDidChange block with .sync because both must complete before returning; the LSP notifications and undo registration hop with .async as they already did. docs/architecture-decisions.md is corrected too. It previously described the pin as what made the read path sound, which overstated it.
Five build configs carry ENABLE_APP_SANDBOX = NO and the entitlements file has no app-sandbox key, with nothing in the repo saying why. A reviewer had no way to tell a deliberate decision from a leftover. The sandbox blocks Process from spawning subprocesses, which is what ShellClient, CETerminal, and every git path depend on, so this is the project's long-standing configuration rather than a workaround. PR CodeEditApp#2147 enabled it by accident in December as part of an unrelated fix and broke git, LSP, the terminal, and package installs; commit a2fff0c reverted that. Recording the history so the next person does not repeat it. Also notes the two consequences: App Store distribution is out of scope, and the security-scoped bookmark handling is a deliberate no-op kept for the case where this is revisited.
Contributor
Author
Toolchain note for reviewers: Xcode 16.4 versus 26Worth knowing before you build this branch, because it will affect your own results. The CI runner has Xcode 16.4 (Swift 6.1); current local development is Xcode 26 (Swift 6.3). The package targets are Swift 6 strict-concurrency, so they are the first code in this project where that gap matters, and it produced nine consecutive CI failures on this branch. The diagnostics split two ways: Same analysis, different severity. Xcode 26 reports the isolation errors at the identical lines, as warnings. Judging a local build by its exit code hides them completely. To see what CI will reject: xcodebuild -workspace CodeEdit.xcworkspace -scheme CodeEdit \
-testPlan CodeEditTestPlan -destination 'platform=macOS,arch=arm64' \
build-for-testing 2>&1 | grep 'warning:' | grep -Ei 'isolat|sendable|concurren|actor|data race'Warnings carrying the suffix "this is an error in the Swift 6 language mode" live in the Swift 5 app target and stay warnings. Ones without it, in package code, are the CI blockers. Different analysis, invisible locally. Where 16.4 demands an explicit Why CI is not green right now. All nine compile failures are fixed and the twelve library targets plus the test targets build on the runner. The first run to get that far then sat on the "Testing App" step for 45 minutes with no output, against a 4-to-5-minute historical baseline for a passing run, and was cancelled. That is one observation and it is undiagnosed. The same invocation ( |
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.
Suggested title: Re-architecture: one modular package, explicit composition root, typed seams
Summary
This branch is a large instalment of an ongoing re-architecture, not its completion. The app target
is decomposed into a single multi-target Swift package, dependency injection is consolidated into one
explicit composition root, every cross-feature signal is typed, and settings, themes and panel tabs
each gained a seam that a feature package can use without reaching back into the app.
What it deliberately does not settle is listed under Still open: the search
feature needs a rebuild rather than a refactor, two targets have unresolved questions about whether
they should exist in their current form, and the target names themselves are still under discussion.
321 commits, 955 files, +19,423 / −12,270. It is large, and squashing loses the step-by-step history,
so the reasoning behind the non-obvious decisions is recorded in
ARCHITECTURE.mdand in doccomments on the types themselves rather than only in commit messages.
Everything below describes the difference between
mainand this branch. The route taken to get hereincluded stages that no longer exist. They are not described, because nothing in the diff shows
them.
Why, and why not just "split it into packages"
CodeEdit tried modularisation in 2022 and reverted it after cyclic dependencies. That history shaped
this attempt, because the post-mortem does not say what people remember it saying: the cycles were
not caused by having packages. They were caused by two specific things: there was no dependency
sink, so any two features that needed to share a type had nowhere downward to put it, and two edges
pointed the wrong way.
So the fix is not a packaging shape but a rule set:
CodeEditCorehas zero local dependencies. That makes it a sink, so cross-feature sharingalways resolves downward. This is the direct fix for the 2022 failure.
CodeEditCoreimports no UI framework. This keeps the placement question answerable. Withoutit, Core becomes the new
AppPreferences, a bag everything drifts into.CodeEditUIdepends on no local target. Presentation atoms stay atoms.There are currently zero feature→feature imports, and Core is reachable from everything without a
cycle being expressible.
Package topology
One package, twelve library targets:
mainhas noPackage.swiftat all: it is a single app target, with features as folders underCodeEdit/Features/. One package with many targets was chosen over many packages so that thedependency edges are declared in one readable file, and so adding a target is a three-line change.
Package targets are Swift 6 strict-concurrency (except
CEEditor, pinned to Swift 5 language mode).The app target is still Swift 5; full migration is deliberately out of scope.
What changed
Dependency injection. On
main, services are reached through singletons.Settings.sharedalone is named in 22 files. This branch introduces
AppDependencies, a single app-scope compositionroot: objects take what they need by initialiser, SwiftUI views get environment keys, and only
composition roots hold the whole dependency object.
Settings.sharedis deleted. Several otherpre-existing singletons remain and are listed rather than quietly kept (see Still open below).
Cross-feature communication is typed.
mainbroadcasts some cross-feature signals throughuntyped
NotificationCenternames (TaskNotificationHandlerdeclares its own). Here there arezero custom
Notification.Names. Featurescommunicate through the
Workspaceaggregate, a typedEventBusfor facts, or command interfaces inCore for requests with one handler (
WorkspaceNavigator,WorkspaceFileOpener,FileRelocator).Remaining
NotificationCenteruse observes platform notifications only.Settings. On
mainevery consumer reads theSettings.sharedsingleton and addresses fieldsthrough one app-wide aggregate. Here, a feature package owns its own settings sections and reads them
through an injected seam, one section at a time, so it never names the aggregate and never reaches a
singleton. The store is an observed object injected into the view tree; a subtree that never receives
it fails loudly instead of reading plausible defaults and silently discarding writes.
Panel tabs.
NavigatorTab,InspectorTabandUtilityAreaTabare deleted. A panel's tabs are a[any WorkspacePanelContribution]assembled at the composition root, so first-party tabs, app shellchrome and extension tabs are the same kind of value. Previously extensions had a privileged dynamic
case that features could not use.
Themes. The active theme moved into Core and is delivered as an observed object.
Folder conventions. Grouping is by purpose, never by kind: no
Models/,Views/,ViewModels/,Services/orUseCases/folders. Six package targets have been regrouped on thatbasis (
CEEditor,CESourceControl,CENotifications,CodeEditCore,CETerminal,CELSP),each a pure rename with no content change.
CodeEditCorekeeps itsDomain/andInfrastructure/split as a stated exception: there the layer is the purpose.
Documentation.doccis deleted (34 files). It was untouched since January 2025, and of the 34symbols its landing page linked, 9 no longer existed and 13 had moved into package targets, DocC
documents one module, so an app-target catalog could no longer resolve them. Its section names were
the retired
Features/folders, andAppPreferences/was nine files of tutorial for the god objectthis re-architecture exists to have removed. Nothing referenced it: no inbound links, absent from
Package.swiftand CI, while compiled in the app target's Sources phase. This also matches what theCodeEditApporg does: all five libraries it publishes ship a catalog, because a publishedlibrary has readers who never open its source; none of these twelve internal targets does. A catalog
is optional in any case: DocC generates symbol documentation from doc comments without one.
App target layout is now scope-first (
App/,WorkspaceWindow/,AuxiliaryWindows/,Utils/),mapping onto the scenes the app actually declares, and grouped by purpose rather than by kind, so there
are no
Models//Views//Services/folders.The constraint that kept paying off
The most useful result was not a module boundary; it was that the boundaries kept forcing better
APIs. Four times, something "obviously" needed a forbidden import and the constraint produced a
better design instead:
FileIconis keyed onURL, not on a domain type, so it needs only SwiftUI andUniformTypeIdentifiers, and lives in
CodeEditUIwith no charter change. Deduplicating it alsoretired an 80-case
FileTypeenum and fixed a silent bug where every unenumerated file extensionreported itself as
"text".WorkspacePanelContributionvendsAnyView, so it needs SwiftUI only and cannot live in Core.Color+HEXis keyed on aString, so the colour conversion needs nothing from Core. Splittingit revealed that its file was always two unrelated things: the app used only the SwiftUI accessor,
CEEditoronly the AppKit one.ActiveThemeis anObservableObject, andObservableObjectis Combine, not SwiftUI, so anobservable theme is legal in a zero-UI target.
Each of these was a placement argument that looked like it needed a rule change and did not.
Enforcement
Three automated checks, run on every build:
no_ui_in_core(SwiftLint, error severity):CodeEditCoremay not import SwiftUI/AppKit/Cocoa.ui_package_purity:CodeEditUImay not import a local target.audit_package_imports.py: import honesty across all 12 library targets.The audit is load-bearing rather than belt-and-braces:
swift buildcannot validate 7 of the 12targets standalone (an external dependency lacks a
resources:declaration, soBundle.moduleisunavailable), which makes Xcode the build gate and the audit the only structural check.
Bugs on
mainthat this fixesEach of these is present on
maintoday and was found by moving the code that contained it:updateNSViewnever setview.font, so afont change applied only to terminals opened afterwards.
FileTypeenum's raw value for.txtis
"text", and every unenumerated extension also reported itself as"text", so "show/hide theseextensions" silently compared the wrong strings. Retiring the enum in favour of matching
url.pathExtensionfixed it.type whose name obscured what it actually held.
SearchState.replaceRangebuilt anNSAlertand called
runModal()from a synchronous nonisolated method. It had no callers anywhere in therepo, so it is deleted rather than fixed.
Eight further pre-existing defects were concurrency errors rather than behavioural ones.
Moving a file into
CodeEditModulesis not only a move: the app target is Swift 5 with minimalconcurrency checking, the package is Swift 6, so code that compiled silently for years arrives with
hard errors.
The failing lines are byte-identical on
main.The toolchain detail is in a comment on this PR.
Several other defects were introduced and fixed within this branch as the seams were built. They
are not listed here: they are not on
main, so they are not fixes a reviewer can verify.One behaviour is new rather than a fix: an unreadable
settings.jsonis now copied aside assettings.json.corrupt-<timestamp>before anything writes over it, and a section this build cannotdecode is re-emitted verbatim instead of being replaced by defaults.
Still open
Deliberately out of scope here, and the reason each waits:
Open design questions. These may change the package layout again.
CodeEditSettingsneeds a rename, or a split. Its dependency structure is sound, but itsname is not: it reads as the home for all settings, while a feature's own settings belong in
that feature and settings pages are app-side. Three homes, and one of them advertises itself as
the answer.
CodeEditDocumentmay dissolve intoCEEditor. OnlyCEEditorandCELSPdepend on it, andCELSPalready declares its ownLanguageServerDocumentprotocol, but it still namesCodeFileDocumentin 15 places, so this is a design slice, not a move.CodeEdit*marks substrate andCE*marks features, butShellClientandCEWorkspaceFileManagerfit neither, and whether platform adapters deservetheir own signal is undecided.
CESearchneeds a rebuild, not a refactor. Its index does no matching, never updates, and hasno reachable cancellation. Restructuring it now would move files a rewrite will move again.
Its indexing also has no completion signal, so tests poll
indexStatuswith a timeout, which isthe mechanical cause of the known flakiness there.
CESourceControl/Accounts/is 58 of that target's 133 files with three call sites, andBitBucket is unreferenced outside its own subtree. What is dead needs settling before anything is
reorganised.
Known work, no open questions.
SettingsAccessingis unblocked by this branch but waits for it.SettingsData/@AppSettingsstill exist (40 declarations, 24 files). The seam is in placeand preferred for new code; the cutover is mechanical and its own change.
ThemeModel,FeedbackModel,ExtensionManager, …) shouldmove onto
AppDependencies.TerminalCacheis separate: it is a scope error, holdingworkspace-window-scoped views in a process-global cache with no eviction on workspace close.
Accounts/. Six targets have been regrouped by purpose;CESearch,CodeEditSettingsandCodeEditUIhave not.migration machinery before there is a migration to run would be guessing.
round-trip, which is the guarantee that matters; the accessor waits on the extension manifest
schema.
CodeFileDocument's isolation is bridged, not solved.NSDocumentis main-actor isolated butdeclares
read(from:ofType:)andpresentedItemDidChange()nonisolated. Four sites now branch onThread.isMainThreadand assume isolation only on the main-thread side; two must block withDispatchQueue.main.sync, which rests on an invariant nothing enforces. The proper fix separatesmain-actor UI state from the I/O lifecycle: decode to a
Sendablevalue, install it in onemain-actor step. Recorded in
docs/architecture-decisions.md.CEWorkspaceFilekeeps itsCEprefix despite no name collision: 294 references, mechanicalbut across eight targets.
Verification
--strict, and the package import audit are green.CodeEditTestPlanpasses locally: exit 0,** TEST SUCCEEDED **, 175 XCTest cases plus 110swift-testing cases across 28 suites.
main.mainrunsCodeEditTestsandCodeEditUITests; this branch addsCodeEditCoreTests,CodeEditSettingsTests,CESearchTests,CodeEditUIUnitTests,CELSPTestsandCESourceControlTests. CI runtime rises accordingly..github/scripts/test_app.shgains-workspace CodeEdit.xcworkspace.mainhas no workspace atall; this branch introduces it, and without the flag bare
xcodebuildpicks the.xcodeprojandfails with "Missing package product" once per package.
environment injection or colour fidelity. The last full pass covered every window, panel, sheet and
popover after settings delivery changed to a trapping mechanism.
CodeEditUITests/ProjectNavigatorFileManagementUITests/testCreateNewFilesfails on macOS 26,including on
main, verified by running it againstmainin a separate worktree: byte-identicalfailure. CI does not see it because its runner predates macOS 26 and takes the other branch of
WorkspacePanelView's availability check.This branch also merges
main, including the macOS Tahoe panel redesign (#2126). That redesigntouched the same three panels restructured here; its visuals and its per-tab bottom toolbars are
ported onto the contribution seam, where the toolbar became a protocol requirement each tab vends
rather than a
switchevery new tab must be added to.How to review this
Reading it as one diff will not work. Suggested order:
ARCHITECTURE.md: the rules, the 2022 post-mortem, and "where does my code go?". Thendocs/architecture-decisions.mdfor the rulings and the accepted bridges.CodeEditModules/Package.swift: the twelve targets and their edges.CodeEdit/App/AppDependencies.swift: the composition root.WorkspacePanelContributionplusPanelContributions.swiftand onepackage-vended conformer.
The doc comments carry the reasoning that a squashed history will not. Where something looks
gratuitously indirect, the comment usually explains which failure it prevents.