Add pl-cli: CLI for Platforma server state manipulation#1516
Conversation
New @platforma-sdk/pl-cli package with commands: - project list/info/duplicate/rename/delete - admin copy-project (cross-user, controller auth) - admin user-list Supports --format text|json output for all commands.
🦋 Changeset detectedLatest commit: 6dd1552 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a new command-line interface (CLI) tool, Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new and valuable CLI tool, pl-cli, for interacting with the Platforma server. The code is well-structured, leveraging oclif for the command framework and creating clear base classes for different command types (user vs. admin). The separation of concerns with project_ops.ts and connection helpers is also well done.
My review focuses on a few areas for improvement:
- Performance: Some operations that iterate over projects can be optimized by parallelizing requests.
- Code Duplication: There's an opportunity to refactor some repeated logic into a shared helper function.
- Potential Bug: A helper function for output formatting has a minor bug that could cause issues if used in a certain way.
Overall, this is a solid addition to the toolset. The changes are well-thought-out and the CLI provides useful functionality as described.
- Export project model constants, ProjectsField, and duplicateProject from pl-middle-layer public API - Import them in pl-cli instead of duplicating (eliminates drift risk) - Remove duplicateProjectInTx copy from CLI - Extract shared CLI flags into cmd-opts.ts (matching monorepo pattern) - Move navigateToUserRoot from admin_connection to project_ops - Merge admin connection into connection.ts - Remove admin user-list (reads wrong resource, no PL API to enumerate users) - Fix dynamic import of isNullResourceId in duplicate command - Normalize --auto-rename flag between duplicate and copy-project - Remove dead output() function, replace with outputJson() - Remove unused outputFormat accessor from PlCommand - Expand lib.ts exports for programmatic use - Add double-connect guard in base commands - Switch to ESM-only build (type: module) for pl-middle-layer compat
vitest exits 1 when no test files exist. Add --passWithNoTests flag.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1516 +/- ##
=======================================
Coverage 54.27% 54.27%
=======================================
Files 242 242
Lines 13727 13727
Branches 2828 2828
=======================================
Hits 7451 7451
Misses 5341 5341
Partials 935 935 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…target-user
Merge AdminCommand into PlCommand with dual-mode connect():
- connect() returns { pl, projectListRid } for single-user ops
- connectClient() returns raw PlClient for multi-user admin ops
- AdminTargetFlags (optional) on all project commands
- AdminAuthFlags (required) on purely admin commands like copy-project
- Delete separate admin_base_command.ts
…jects - Add getExistingLabelsInTx(tx, projectListRid) to deduplicate label reading logic shared between duplicate and admin copy-project - Use Promise.all for parallel KV fetches in listProjects
| "dependencies": { | ||
| "@milaboratories/pl-client": "workspace:*", | ||
| "@milaboratories/pl-middle-layer": "workspace:*", | ||
| "@oclif/core": "catalog:" |
There was a problem hiding this comment.
can we use commander? me and Alexander agreed that we need to migrate block-tools to it, but had no time, and now this new package uses oclif again
There was a problem hiding this comment.
Let's separately migrate it.
| ); | ||
|
|
||
| const target = | ||
| targetUser === flags["source-user"] ? source : await navigateToUserRoot(pl, targetUser); |
There was a problem hiding this comment.
can we not use await in ternary operator? that looks dangerous
| tx.createField(field(target.projectListRid, newId), "Dynamic", newPrj); | ||
| await tx.commit(); | ||
|
|
||
| return { rid: await toGlobalResourceId(newPrj), label: newLabel }; |
There was a problem hiding this comment.
again, please place statements with await on a separate line, not as a part of bigger expression. they should catch the eye of the reader instantly
| targetUser, | ||
| }); | ||
| } else { | ||
| console.log( |
There was a problem hiding this comment.
if we are using outputJson for json, maybe we should define outputText instead of calling console.log directly?
| const info = await getProjectInfo(pl, projectListRid, id); | ||
|
|
||
| if (!flags.force) { | ||
| const readline = await import("node:readline"); |
There was a problem hiding this comment.
why inline import, not on top of the file?
| const answer = await new Promise<string>((resolve) => { | ||
| rl.question(`Delete project "${info.label}" (${info.blockCount} blocks)? [y/N] `, resolve); | ||
| }); | ||
| rl.close(); |
There was a problem hiding this comment.
should it be try-finally to always close?
|
|
||
| const newRid = await pl.withWriteTx("duplicateProject", async (tx) => { | ||
| const sourceMetaStr = await tx.getKValueString(sourceRid, ProjectMetaKey); | ||
| const sourceMeta = JSON.parse(sourceMetaStr); |
There was a problem hiding this comment.
should we add zod validation here maybe?
| "admin-user"?: string; | ||
| "admin-password"?: string; | ||
| }): Promise<PlClient> { | ||
| if (this._pl) throw new Error("connectClient() called twice"); |
There was a problem hiding this comment.
is it correct? maybe we should re-initialize connection instead?
There was a problem hiding this comment.
This is a bug guard - each command runs once and should only connect once. Re-initializing would leak the previous connection. If a command somehow calls this twice, that's a bug we want to catch immediately rather than silently reconnect.
| return { pl, projectListRid }; | ||
| } | ||
|
|
||
| protected async finally(_: Error | undefined): Promise<void> { |
There was a problem hiding this comment.
maybe we can utilize AsyncDisposable somehow instead of manually calling close?
There was a problem hiding this comment.
Adding AsyncDisposable to PlClient is a good idea but belongs in a separate pl-client PR. This finally() is the idiomatic oclif cleanup mechanism.
| tx.getKValueStringIfExists(f.value, ProjectLastModifiedTimestamp), | ||
| ]); | ||
|
|
||
| const meta: ProjectMeta = metaStr ? JSON.parse(metaStr) : { label: "(unknown)" }; |
There was a problem hiding this comment.
again, maybe we need zod? I looks dangerous to just trust the type of JSON.parse
There was a problem hiding this comment.
We should add it on the level of model in a separate PR.
…y cleanup - Add outputText() to output.ts for consistency with outputJson() - Replace all console.log in commands with outputText() - Move node:readline import to top of delete.ts - Wrap readline in try-finally to ensure close on error
2 participants
Summary
@platforma-sdk/pl-clipackage intools/pl-cli/block-toolsandpl-bootstrap)Commands
Project management (standard user auth):
pl-cli project list— list all projects (table or JSON)pl-cli project info <id|label>— show project details (schema, blocks, timestamps)pl-cli project duplicate <id|label>— duplicate with auto-rename on collisionpl-cli project rename <id|label> --name "X"— rename a projectpl-cli project delete <id|label>— delete with confirmation (or--force)Admin operations (controller credentials via
PL_ADMIN_USER/PL_ADMIN_PASSWORD):pl-cli admin copy-project— copy project between userspl-cli admin user-list— list user roots on serverDesign decisions
--format text(human-readable) and--format json(automation)Test plan
project listagainst live serverproject duplicatewith auto-rename deduplicationadmin copy-project(same user to same user)project infoandproject delete