Skip to content

Commit b07537b

Browse files
seefeldbclaude
andauthored
refactor(cli): make charm execution explicit via required runIt parameter (#1967)
* refactor(cli): make charm execution explicit via required runIt parameter Previously, CharmManager.get() defaulted to running charms (runIt=true), causing unnecessary charm execution for read/write operations. This change makes the default false and requires all callers to explicitly pass the runIt parameter. Changes: - Change default runIt from true to false in CharmManager.get() and CharmsController.get() - Update CLI lib functions to explicitly pass runIt: - Pass false for read/write operations (inspect, get, set, apply, getsrc, setsrc) - Pass true for operations requiring execution (call, render) - Update shell AppView to pass runIt: true when displaying active charm Benefits: - Eliminates unnecessary charm runs for ct charm ls, inspect, get, set, etc. - Makes code intent explicit at every call site - Improves performance by avoiding unnecessary work - Maintains correct behavior for commands that need execution (call, render, step) Technical notes: - Preserved all three overload signatures to avoid TypeScript type explosion - Only changed the default value in the implementation - All tests pass without type checker OOM * add step to integration test * fix script to not exit early without printing an error message when charm is not found --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9fd7bea commit b07537b

File tree

6 files changed

+28
-14
lines changed

6 files changed

+28
-14
lines changed

packages/charm/src/manager.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ export class CharmManager {
382382
): Promise<Cell<T>>;
383383
async get<T = unknown>(
384384
id: string | Cell<unknown>,
385-
runIt: boolean = true,
385+
runIt: boolean = false,
386386
asSchema?: JSONSchema,
387387
): Promise<Cell<T>> {
388388
// Load the charm from storage.

packages/charm/src/ops/charms-controller.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,21 @@ export class CharmsController<T = unknown> {
4949

5050
async get<S extends JSONSchema = JSONSchema>(
5151
charmId: string,
52+
runIt: boolean,
5253
schema: S,
5354
): Promise<CharmController<Schema<S>>>;
5455
async get<T = unknown>(
5556
charmId: string,
57+
runIt?: boolean,
5658
schema?: JSONSchema,
5759
): Promise<CharmController<T>>;
58-
async get(charmId: string, schema?: JSONSchema): Promise<CharmController> {
60+
async get(
61+
charmId: string,
62+
runIt: boolean = false,
63+
schema?: JSONSchema,
64+
): Promise<CharmController> {
5965
this.disposeCheck();
60-
const cell = await this.#manager.get(charmId, true, schema);
66+
const cell = await this.#manager.get(charmId, runIt, schema);
6167
if (!cell) {
6268
throw new Error(`Charm "${charmId}" not found.`);
6369
}

packages/cli/integration/integration.sh

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,10 +158,14 @@ echo "Testing --input flag operations..."
158158
test_json_value "Input flag set" "userData" '{"user":{"name":"test"}}' "--input"
159159
test_value "Nested input path" "userData/user/name" '"inputValue"' '"inputValue"' "--input"
160160

161+
echo "Testing charm step..."
162+
163+
# Recompute (one iteration) with updated inputs
164+
ct charm step $SPACE_ARGS --charm $CHARM_ID
165+
161166
# Check space has new charm with correct inputs and title
162167
TITLE="Simple counter 2: 10"
163-
ct charm ls $SPACE_ARGS | grep -q "$CHARM_ID $TITLE <unnamed>"
164-
if [ $? -ne 0 ]; then
168+
if ! ct charm ls $SPACE_ARGS | grep -q "$CHARM_ID $TITLE <unnamed>"; then
165169
error "Charm did not appear in list of space charms."
166170
fi
167171

packages/cli/lib/charm-render.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export async function renderCharm(
2929
// 2. Get charm controller to access the Cell
3030
const manager = await loadManager(config);
3131
const charms = new CharmsController(manager);
32-
const charm = await charms.get(config.charm);
32+
const charm = await charms.get(config.charm, true);
3333
const cell = charm.getCell().asSchema({
3434
type: "object",
3535
properties: {

packages/cli/lib/charm.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ export async function setCharmRecipe(
143143
): Promise<void> {
144144
const manager = await loadManager(config);
145145
const charms = new CharmsController(manager);
146-
const charm = await charms.get(config.charm);
146+
const charm = await charms.get(config.charm, false);
147147
if (entry.mainPath.endsWith(".iframe.js")) {
148148
await charm.setIframeRecipe(entry.mainPath);
149149
} else {
@@ -158,7 +158,7 @@ export async function saveCharmRecipe(
158158
await ensureDir(outPath);
159159
const manager = await loadManager(config);
160160
const charms = new CharmsController(manager);
161-
const charm = await charms.get(config.charm);
161+
const charm = await charms.get(config.charm, false);
162162
const meta = await charm.getRecipeMeta();
163163
const iframeRecipe = await charm.getIframeRecipe();
164164

@@ -194,7 +194,7 @@ export async function applyCharmInput(
194194
) {
195195
const manager = await loadManager(config);
196196
const charms = new CharmsController(manager);
197-
const charm = await charms.get(config.charm);
197+
const charm = await charms.get(config.charm, false);
198198
await charm.setInput(input);
199199
}
200200

@@ -380,7 +380,7 @@ export async function inspectCharm(
380380
}> {
381381
const manager = await loadManager(config);
382382
const charms = new CharmsController(manager);
383-
const charm = await charms.get(config.charm);
383+
const charm = await charms.get(config.charm, false);
384384

385385
const id = charm.id;
386386
const name = charm.name();
@@ -446,7 +446,7 @@ export async function getCellValue(
446446
): Promise<unknown> {
447447
const manager = await loadManager(config);
448448
const charms = new CharmsController(manager);
449-
const charm = await charms.get(config.charm);
449+
const charm = await charms.get(config.charm, false);
450450
if (options?.input) {
451451
return charm.input.get(path);
452452
} else {
@@ -462,7 +462,7 @@ export async function setCellValue(
462462
): Promise<void> {
463463
const manager = await loadManager(config);
464464
const charms = new CharmsController(manager);
465-
const charm = await charms.get(config.charm);
465+
const charm = await charms.get(config.charm, false);
466466
if (options?.input) {
467467
await charm.input.set(value, path);
468468
} else {
@@ -480,7 +480,7 @@ export async function callCharmHandler<T = any>(
480480
): Promise<void> {
481481
const manager = await loadManager(config);
482482
const charms = new CharmsController(manager);
483-
const charm = await charms.get(config.charm);
483+
const charm = await charms.get(config.charm, true);
484484

485485
// Get the cell and traverse to the handler using .key()
486486
const cell = charm.getCell().asSchema({

packages/shell/src/views/AppView.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,11 @@ export class XAppView extends BaseView {
134134
) {
135135
return current;
136136
}
137-
const activeCharm = await rt.cc().get(app.activeCharmId, nameSchema);
137+
const activeCharm = await rt.cc().get(
138+
app.activeCharmId,
139+
true,
140+
nameSchema,
141+
);
138142
// Record the charm as recently accessed so recents stay fresh.
139143
await rt.cc().manager().trackRecentCharm(activeCharm.getCell());
140144
this.#setTitleSubscription(activeCharm);

0 commit comments

Comments
 (0)