Repository navigation
fix(snippets): preserve all-hosts scope in snippet import/export - #3636
netcatty-bot wants to merge 2 commits into
Conversation
netcatty-bot
added
automation:bot-pr
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: 86c3419210
ℹ️ 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".
| tags?: string[]; | ||
| package?: string; | ||
| targetGroups?: string[]; | ||
| targetsAllHosts?: boolean; |
There was a problem hiding this comment.
When a toolkit produced here is imported by a pre-change Netcatty release, the payload still advertises version 2, so that release accepts it but silently discards targetsAllHosts; an all-host onConnect script is then stored without targets or groups and is excluded from getGlobalConnectScripts, disabling its automation. Use a new export version while continuing to accept versions 1 and 2, or provide another compatibility representation rather than reusing version 2.
AGENTS.md reference: AGENTS.md:L153-L155
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
netcatty-bot
added
automation:codex-clean
|
Codex reported no major issues. This PR is marked ready for human review/merge. |
2 participants
Summary
targetsAllHosts?: booleantoSnippetExportItemso the portable "apply to all hosts" scope survives snippet toolkit export/import.toExportItemnow writestargetsAllHosts: trueand, when the flag is set, omits group scope so exported JSON is never ambiguous.sanitizeImportItemaccepts the flag only when it is exactlytrueand dropstargetGroupsin that case, so an all-hosts import always wins the unambiguous way (matching save-time semantics).toImportedSnippetrestorestargetsAllHostson the imported snippet, keepstargets: [], and only appliestargetGroupswhen the flag is absent.targets) remain intentionally omitted from exports, so imports stay portable across machines.Why
Importing a portable snippet toolkit JSON preserved
targetGroupsbut silently droppedtargetsAllHosts: true, so after importing a shared toolkit with 20+ snippets each one had to be reopened and re-scoped to all hosts by hand. The boolean does not depend on local host IDs or group paths, so it is safe to round-trip.Changes
domain/snippetTransfer.ts: export type +toExportItem,sanitizeImportItem,toImportedSnippet.domain/snippetTransfer.test.ts: new tests covering export → parse → merge round-trip oftargetsAllHostsand conflict resolution againsttargetGroups(all-hosts wins; explicitfalsestays unscoped).No version bump: the field is additive and optional, and legacy version-1 payloads go through the same sanitizer.
Testing
node --test --import tsx domain/snippetTransfer.test.ts— 12/12 pass, including the two new tests.node --test --import tsx domain/snippetTargets.test.ts domain/snippetAgentOps.test.ts— still pass.npx tsc --noEmit -p tsconfig.json— no errors in the touched files.npx eslint domain/snippetTransfer.ts domain/snippetTransfer.test.ts— clean.Fixes #3635
Automation
@codex review(own/bot PRs only)