fix(storage): convention parity#120
Conversation
|
@codex review |
🦋 Changeset detectedLatest commit: ed84565 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![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: 9ca267cb30
ℹ️ 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".
| if (!force) { | ||
| const confirmed = await confirm( | ||
| `Unlink from ${existing.name ?? existing.id}?`, | ||
| ); |
There was a problem hiding this comment.
When --output json is used and .bunny/storage.json exists, this branch still opens a confirmation prompt unless --force is also supplied. In CI or other JSON consumers this can block or produce the plain-text Unlink cancelled. success path instead of a JSON payload, violating the command's JSON-output contract; require --force/return a JSON error before calling confirm in JSON mode.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR aligns the storage commands around a consistent set of conventions: a new
Confidence Score: 5/5Safe to merge; changes are additive convention fixes with no API contract breakage. All changed code paths are interactive CLI flows. The destructive zone remove command now has stronger confirmation guards (type-to-confirm + stale manifest cleanup), the new unlink command correctly errors in non-interactive mode, and the offerLink offer only fires after the picker which already requires a TTY. The one small gap — unlinked absent from JSON output — is a cosmetic completeness issue with no impact on correctness. packages/cli/src/commands/storage/zone/remove.ts — JSON output missing the unlinked field; packages/cli/src/commands/storage/file/remove.ts — offerLink: true on a destructive command (noted in previous review round) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([resolveStorageZoneInteractive]) --> B{ref provided?}
B -- yes --> C[resolveStorageZone by name/ID]
C --> D([return zone — no link offer])
B -- no --> E{ignoreManifest?}
E -- no --> F{manifest.id present?}
F -- yes --> G[fetchStorageZone by manifest ID]
G --> D
F -- no --> H{force OR not interactive?}
E -- yes --> H
H -- yes --> ERR([UserError: zone required])
H -- no --> I[fetchStorageZones → picker prompt]
I --> J{user cancelled?}
J -- yes --> ERR2([UserError: zone required])
J -- no --> K[fetchStorageZone by picked ID]
K --> L{offerLink?}
L -- no --> M([return zone])
L -- yes --> N[confirm: Link this directory?]
N -- yes --> O[writeStorageManifest]
O --> M
N -- no --> M
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A([resolveStorageZoneInteractive]) --> B{ref provided?}
B -- yes --> C[resolveStorageZone by name/ID]
C --> D([return zone — no link offer])
B -- no --> E{ignoreManifest?}
E -- no --> F{manifest.id present?}
F -- yes --> G[fetchStorageZone by manifest ID]
G --> D
F -- no --> H{force OR not interactive?}
E -- yes --> H
H -- yes --> ERR([UserError: zone required])
H -- no --> I[fetchStorageZones → picker prompt]
I --> J{user cancelled?}
J -- yes --> ERR2([UserError: zone required])
J -- no --> K[fetchStorageZone by picked ID]
K --> L{offerLink?}
L -- no --> M([return zone])
L -- yes --> N[confirm: Link this directory?]
N -- yes --> O[writeStorageManifest]
O --> M
N -- no --> M
Reviews (3): Last reviewed commit: "fix(storage): close Greptile P2 gaps in ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…tions - Use the shared isInteractive() gate (stdin + stdout TTY, json-aware) instead of stdout-only checks in the zone resolver, link, and zone add - zones update: require field flags whenever non-interactive (json, non-TTY, or --force) instead of only under --output json, and abort the whole interactive edit on a mid-flow cancel rather than applying partial answers - zones update --force now matches zones add: skip prompts and confirmations, use flag values only - storage zone domains commands now resolve the zone like every other storage command: explicit ref, then linked zone, then picker
- zones remove: double confirmation (yes/no, then type the zone name) unless --force, matching db and scripts delete; also removes a stale .bunny/storage.json that pointed at the deleted zone - New storage unlink command mirroring dns zones unlink - resolveStorageZoneInteractive takes DNS-style opts (output, force, offerLink, ignoreManifest); the picker now offers to link the directory on non-destructive commands, and link reuses the resolver instead of duplicating it - The zone-required error now hints at bunny storage link - Replication confirmation defaults to no: adding a replica is permanent and adds cost
- Hints and the domains factory commandPath use the canonical plural (storage zones add / storage zones domains) - Declining the replication confirm in zones add prints Cancelled. and exits cleanly instead of throwing, matching the other declines - --custom-404-path "" clears the custom 404 (null), matching the prompt's blank-for-none behavior
9ca267c to
d2bd230
Compare
- remove: compare the type-to-confirm value against zone.Name ?? "" so a Ctrl+C (undefined) can never match an undefined zone name and skip the guard - unlink: throw when run non-interactively without --force instead of silently cancelling and leaving the manifest, matching the other commands
3 participants
No description provided.