release: prepare v1.3.0#77
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR adds a feature-permission policy with built-in categories and host-access declarations, a typed command/exec JSON-RPC surface, sandboxed command execution models, app-wide feature-operation event streaming, Git observability with parsers and selected-worktree snapshots, a marketplace-upgrade maintenance workflow, library wiring for Git refreshes, tests, doc updates, and README/ROADMAP/version-bump script edits targeting v1.3.0. Feature Permission Policy & Git Observability System
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/SwiftASB/Public/CodexAppServer.swift (1)
200-215:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFinish
featureOperationEvents()on unexpected transport shutdown too.The new stream is cleaned up in
stop(), but the existing event-loop error/end paths later in this file still omitfinishAllFeatureOperationEventStreams(). After an unexpected transport failure, subscribers tofeatureOperationEvents()can wait forever for the next element instead of seeing the stream end.Suggested change
} catch { await self.finishAllThreadEventStreams( throwing: CodexAppServerError.wrap(error, operation: "server events") ) await self.finishAllDiagnosticEventStreams( throwing: CodexAppServerError.wrap(error, operation: "server events") ) await self.finishAllLibraryEventStreams() + await self.finishAllFeatureOperationEventStreams() await self.finishAllFSChangeStreams() await self.finishAllTurnEventStreams( throwing: CodexAppServerError.wrap(error, operation: "server events") ) } @@ guard hasStarted, !isStopping else { finishAllThreadEventStreams(throwing: nil) finishAllDiagnosticEventStreams(throwing: nil) finishAllLibraryEventStreams() + finishAllFeatureOperationEventStreams() finishAllFSChangeStreams() finishAllThreadObservableActivityStreams() finishAllThreadCommandDeltaStreams() finishAllThreadFileDeltaStreams() finishAllTurnEventStreams(throwing: nil) @@ finishAllThreadEventStreams( throwing: CodexAppServerError.transportFailure( operation: "server events", reason: "Codex app-server stopped delivering thread notifications before pending thread streams finished." ) ) finishAllDiagnosticEventStreams( throwing: CodexAppServerError.transportFailure( operation: "server events", reason: "Codex app-server stopped delivering diagnostics before pending diagnostic streams finished." ) ) finishAllLibraryEventStreams() + finishAllFeatureOperationEventStreams() finishAllFSChangeStreams() finishAllThreadObservableActivityStreams() finishAllThreadCommandDeltaStreams() finishAllThreadFileDeltaStreams() finishAllTurnEventStreams(Also applies to: 254-263, 3355-3362
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/SwiftASB/Public/CodexAppServer.swift` around lines 200 - 215, The featureOperationEvents stream isn't closed in the transport shutdown/error/end paths — add a call to finishAllFeatureOperationEventStreams() alongside the other cleanup calls (e.g., finishAllFSChangeStreams(), finishAllThreadObservableActivityStreams(), finishAllTurnEventStreams(...)) so subscribers receive the stream completion after an unexpected transport.stop()/error; ensure this call is placed before resetting state (hasStarted/hasCompletedInitializeHandshake/thread* collections) in every error/termination branch where other finishAll* functions are invoked (including the other places analogous to the shown block).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 34: Fix the typographical error in the README motivation paragraph by
changing the phrase "on thee desktop" to "on the desktop"; locate the sentence
that contains "I built SwiftASB... on thee desktop" and update it to read "on
the desktop" so the product-facing copy is correct and approachable.
In `@ROADMAP.md`:
- Around line 419-421: The roadmap currently treats `v1.3.0` as already released
(the checklist line "- [x] Update stale release references after the `v1.3.0`
release." and the following "Decision: README now names `v1.3.0` as the current
released baseline...") which will be stale before the tag is published; change
that checklist item and the decision wording to indicate a pending/target
release (e.g., uncheck the box or mark as "pending release" and replace "current
released baseline" with "target release" or "release candidate"), and ensure the
README reference is updated only after the tag is actually published so the
roadmap and README remain consistent with the later "once the tag is published"
language.
In `@Sources/SwiftASB/Public/CodexAppServer.swift`:
- Around line 350-364: The helper requireFeatureEnabled currently only rejects
.disabled and therefore lets a .readOnly policy pass for mutation paths; update
the gate so mutation operations explicitly require the .enabled mode instead of
just not .disabled. Modify requireFeatureEnabled (or add a new helper such as
requireFeatureWritable) to call featurePolicy.mode(for: categoryID) and throw
the same CodexAppServerError.invalidState when the mode != .enabled (preserving
the existing categoryName and error message), and update mutation call sites to
use the new/changed helper while leaving read paths able to accept .readOnly via
the original helper.
In `@Sources/SwiftASB/Public/CodexWorkspace.swift`:
- Around line 318-320: GitStatusSummary.isDirty currently only checks
changedFileCount; update the isDirty computed property (and any dependent checks
like GitStatusSnapshot.isDirty) to return true when either changedFileCount > 0
OR untrackedFileCount > 0 so repositories with only untracked files are treated
as dirty. Locate the isDirty property on GitStatusSummary and modify its boolean
expression to include untrackedFileCount, and ensure any callers relying on
GitStatusSnapshot.isDirty continue to behave correctly.
In `@Sources/SwiftASB/Public/SwiftASBFeaturePolicy.swift`:
- Around line 121-127: The feature policy for the write-capable category
extensionMaintenance in SwiftASBFeaturePolicy is currently set to defaultMode:
.enabled; change defaultMode to .disabled for the extensionMaintenance feature
so it follows the read-only-by-default model, and update any unit tests that
assert the default for extensionMaintenance to expect .disabled instead of
.enabled (ensure SwiftASBFeaturePolicy or the extensionMaintenance constant is
the one modified and adjust related test assertions accordingly).
In `@Sources/SwiftASB/SwiftASB.docc/CodexAppServer.md`:
- Around line 107-108: The Topics list currently includes the request/result
types but omits the API method; add the method symbol
`CodexAppServer/CodexExtensions/upgradeMarketplace(_:)` to the Topics section
alongside `CodexExtensions/MarketplaceUpgradeRequest` and
`CodexExtensions/MarketplaceUpgradeResult` so the `upgradeMarketplace(_:)`
method is discoverable.
In `@Sources/SwiftASB/SwiftASB.docc/FeaturePermissionPolicy.md`:
- Around line 12-14: The docs currently contradict about default mutation
availability: the general statement "mutation categories are disabled until the
consuming app enables them" conflicts with the later statement that
extensionMaintenance is enabled by default; update FeaturePermissionPolicy.md so
the policy is consistent by either (a) making clear that "mutation categories
are disabled by default, except for explicitly allowed mutations such as
extensionMaintenance which is enabled by default" or (b) changing the
extensionMaintenance sentence to indicate it is also disabled unless
enabled—edit the sentences referencing "mutation categories" and the symbol
"extensionMaintenance" so they state the same default behavior and add a brief
note about exceptions if any.
In `@Tests/SwiftASBTests/Protocol/CodexAppServerProtocolTests.swift`:
- Around line 93-94: The test currently force-unwraps the UTF-8 conversion when
creating payload (payload = ... .data(using: .utf8)!), which can crash; replace
the force-unwrap with the Testing framework's throwing assertion (use the
`#require`()/require macro) to assert the Data is non-nil and surface failures via
test errors. Locate the payload declaration in CodexAppServerProtocolTests (the
payload variable in the failing test) and change it to use try require(...) (or
the `#require` variant used in this project) around the .data(using: .utf8) call
so the test fails cleanly instead of crashing.
---
Outside diff comments:
In `@Sources/SwiftASB/Public/CodexAppServer.swift`:
- Around line 200-215: The featureOperationEvents stream isn't closed in the
transport shutdown/error/end paths — add a call to
finishAllFeatureOperationEventStreams() alongside the other cleanup calls (e.g.,
finishAllFSChangeStreams(), finishAllThreadObservableActivityStreams(),
finishAllTurnEventStreams(...)) so subscribers receive the stream completion
after an unexpected transport.stop()/error; ensure this call is placed before
resetting state (hasStarted/hasCompletedInitializeHandshake/thread* collections)
in every error/termination branch where other finishAll* functions are invoked
(including the other places analogous to the shown block).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7851cb8e-b074-460d-b130-f69648700861
⛔ Files ignored due to path filters (1)
docs/media/swiftasb-codex-apps-promo.mp3is excluded by!**/*.mp3
📒 Files selected for processing (31)
README.mdROADMAP.mdSources/SwiftASB/Protocol/CodexAppServerProtocol+Types.swiftSources/SwiftASB/Protocol/CodexAppServerProtocol.swiftSources/SwiftASB/Public/CodexAppServer+Bootstrap.swiftSources/SwiftASB/Public/CodexAppServer+CodexExtensions.swiftSources/SwiftASB/Public/CodexAppServer+CommandExecution.swiftSources/SwiftASB/Public/CodexAppServer+GitObservability.swiftSources/SwiftASB/Public/CodexAppServer+Library.swiftSources/SwiftASB/Public/CodexAppServer.swiftSources/SwiftASB/Public/CodexWorkspace.swiftSources/SwiftASB/Public/SwiftASBFeatureOperationEvent.swiftSources/SwiftASB/Public/SwiftASBFeaturePolicy.swiftSources/SwiftASB/SwiftASB.docc/AppWideCapabilities.mdSources/SwiftASB/SwiftASB.docc/CodexAppServer.mdSources/SwiftASB/SwiftASB.docc/CodexWorkspace.mdSources/SwiftASB/SwiftASB.docc/FeaturePermissionPolicy.mdSources/SwiftASB/SwiftASB.docc/SwiftASB.mdSources/SwiftASB/SwiftASB.docc/SwiftUIObservableCompanions.mdTests/SwiftASBTests/Protocol/CodexAppServerProtocolTests.swiftTests/SwiftASBTests/Public/CodexAppServerFileSystemTests.swiftTests/SwiftASBTests/Public/CodexAppServerLibraryTests.swiftTests/SwiftASBTests/Public/CodexAppServerTestSupport.swiftTests/SwiftASBTests/Public/CodexAppServerTests.swiftTests/SwiftASBTests/Public/CodexWorkspaceTests.swiftTests/SwiftASBTests/Public/SwiftASBFeatureOperationEventTests.swiftTests/SwiftASBTests/Public/SwiftASBFeaturePolicyTests.swiftdocs/maintainers/feature-permission-policy-plan.mddocs/maintainers/v1-public-api-audit.mddocs/maintainers/v1-public-api-symbol-inventory.mdscripts/repo-maintenance/version-bump.sh
Comment on lines
+419
to
421
| - [x] Update stale release references after the `v1.3.0` release. | ||
| Decision: README now names `v1.3.0` as the current released baseline and no | ||
| longer describes the package as early development. |
There was a problem hiding this comment.
Avoid describing v1.3.0 as already released.
This PR is still release prep, and later sections in the same roadmap still say “once the tag is published.” Calling v1.3.0 the current released baseline here will read as stale before the tag is actually public.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ROADMAP.md` around lines 419 - 421, The roadmap currently treats `v1.3.0` as
already released (the checklist line "- [x] Update stale release references
after the `v1.3.0` release." and the following "Decision: README now names
`v1.3.0` as the current released baseline...") which will be stale before the
tag is published; change that checklist item and the decision wording to
indicate a pending/target release (e.g., uncheck the box or mark as "pending
release" and replace "current released baseline" with "target release" or
"release candidate"), and ensure the README reference is updated only after the
tag is actually published so the roadmap and README remain consistent with the
later "once the tag is published" language.
Comment on lines
+121
to
+127
| id: .extensionMaintenance, | ||
| displayName: "Extension Maintenance", | ||
| description: "Upgrade already-installed extensions, plugins, skills, or marketplace entries.", | ||
| permissionReason: "SwiftASB can keep existing trusted extension installs current while reporting any maintenance write it performs.", | ||
| defaultMode: .enabled, | ||
| sensitivity: .maintenance, | ||
| eventPolicy: .notifyOnMutation |
There was a problem hiding this comment.
Disable extensionMaintenance by default.
Line 125 currently ships a write-capable category as .enabled, which contradicts the documented “read-only defaults, explicit mutation enablement” model and lets marketplace upgrades run unless the host opts out. This should default to .disabled, with the matching test expectations updated alongside it.
Suggested change
.init(
id: .extensionMaintenance,
displayName: "Extension Maintenance",
description: "Upgrade already-installed extensions, plugins, skills, or marketplace entries.",
permissionReason: "SwiftASB can keep existing trusted extension installs current while reporting any maintenance write it performs.",
- defaultMode: .enabled,
+ defaultMode: .disabled,
sensitivity: .maintenance,
eventPolicy: .notifyOnMutation
),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| id: .extensionMaintenance, | |
| displayName: "Extension Maintenance", | |
| description: "Upgrade already-installed extensions, plugins, skills, or marketplace entries.", | |
| permissionReason: "SwiftASB can keep existing trusted extension installs current while reporting any maintenance write it performs.", | |
| defaultMode: .enabled, | |
| sensitivity: .maintenance, | |
| eventPolicy: .notifyOnMutation | |
| id: .extensionMaintenance, | |
| displayName: "Extension Maintenance", | |
| description: "Upgrade already-installed extensions, plugins, skills, or marketplace entries.", | |
| permissionReason: "SwiftASB can keep existing trusted extension installs current while reporting any maintenance write it performs.", | |
| defaultMode: .disabled, | |
| sensitivity: .maintenance, | |
| eventPolicy: .notifyOnMutation |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Sources/SwiftASB/Public/SwiftASBFeaturePolicy.swift` around lines 121 - 127,
The feature policy for the write-capable category extensionMaintenance in
SwiftASBFeaturePolicy is currently set to defaultMode: .enabled; change
defaultMode to .disabled for the extensionMaintenance feature so it follows the
read-only-by-default model, and update any unit tests that assert the default
for extensionMaintenance to expect .disabled instead of .enabled (ensure
SwiftASBFeaturePolicy or the extensionMaintenance constant is the one modified
and adjust related test assertions accordingly).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6347f911fa
ℹ️ 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".
| _ categoryID: SwiftASBFeatureCategory.ID, | ||
| for operation: String | ||
| ) throws { | ||
| guard featurePolicy.mode(for: categoryID) != .disabled else { |
There was a problem hiding this comment.
requireFeatureEnabled only rejects .disabled, so mutation APIs still run when a category is set to .readOnly. In this commit, upgradeMarketplace relies on this guard, which means hosts that set extensionMaintenance to read-only will still execute command/exec and mutate marketplace state, violating the documented read-only policy semantics.
Useful? React with 👍 / 👎.
| sortedBy: SortedBy = .updatedNewestFirst, | ||
| groupedBy: GroupedBy = .cwd, | ||
| query: CodexAppServer.ThreadListQD = .init(), | ||
| featurePolicy: SwiftASBFeaturePolicy = .defaults, |
There was a problem hiding this comment.
The library config defaults featurePolicy to .defaults instead of inheriting the CodexAppServer policy, so makeLibrary() can silently re-enable features that were disabled at app-server construction. A host that disables gitObservability in CodexAppServer.Configuration but uses default Library.Configuration will still trigger Git command/exec probes on selection, which breaks app-wide policy expectations.
Useful? React with 👍 / 👎.
| behindCount: Int? | ||
| ) { | ||
| let trimmed = String(line.dropFirst(3)) | ||
| let branchAndTracking = trimmed.split(separator: " ", maxSplits: 1).first.map(String.init) ?? trimmed |
There was a problem hiding this comment.
Branch parsing currently takes only the first whitespace-delimited token from the ## status line, so porcelain output like ## No commits yet on master is parsed as branch No. This yields incorrect Git metadata for newly initialized repositories and makes GitStatusSnapshot.repository.branch unreliable in that common setup state.
Useful? React with 👍 / 👎.
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.
Release
docs/feature-permission-planmainupdates behind pull request review and CIv1.3.0was created locally before this PR so the reviewed release candidate is preserved exactlyReview Loop
Before merge,
scripts/repo-maintenance/release.shwatches CI and stops on review comments unless the maintainer has already addressed or resolved them and reruns with--review-comments-addressed.Summary by CodeRabbit
New Features
Documentation