Skip to content

feat: accept --enable-infiniband flag on sf nodes create#264

Merged
sigmachirality merged 3 commits into
mainfrom
indent-2026-05-15-enable-infiniband-flag
May 21, 2026
Merged

feat: accept --enable-infiniband flag on sf nodes create#264
sigmachirality merged 3 commits into
mainfrom
indent-2026-05-15-enable-infiniband-flag

Conversation

@sigmachirality
Copy link
Copy Markdown
Member

@sigmachirality sigmachirality commented May 20, 2026

Reopen of #263 — that PR's branch was force-recreated during a restack, so GitHub wouldn't let it be reopened. Identical intent, rebased onto the restacked #262.

Summary

Adds an --enable-infiniband flag to sf nodes create that forwards _preview_enable_infiniband: true on the create call. The flag is gated behind the infiniband-preview PostHog feature flag and is only mounted when that flag is enabled for the caller's account — accounts without it see no surface change. Flagged accounts do see the option in sf nodes create --help; preview/experimental expectations are set out-of-band ("white glove") for those customers. Server-side allowlisting (per (account, instance SKU)) does the real enforcement, so non-allowlisted callers still get a 403.

Stacked on #262 (depends on @sfcompute/nodes-sdk-alpha@0.1.0-alpha.31 which exposes _preview_enable_infiniband on NodeCreateParams).

Description of changes

  • src/lib/posthog.ts: extend FeatureFlags with "infiniband-preview".
  • src/lib/nodes/create.ts: createCreateCommand now takes { enableInfiniband }; when true, the option is added to the command and _preview_enable_infiniband: true is spread into the SDK params.
  • src/lib/nodes/index.ts: resolves the feature flag and threads it into the factory.
  • src/index.ts: hoist account_id hydration above the register* calls so feature-flag-gated surfaces resolve on the very first CLI invocation after login (previously they only appeared on the second run, once the cache was seeded).

Testing

  • bun run lint — passes (same pre-existing warnings as main).
  • Confirmed the option only appears in sf nodes create --help when the feature flag resolves true.
  • Confirmed the option appears on the first invocation after a fresh login (post-hoist).

@semanticdiff-com
Copy link
Copy Markdown

semanticdiff-com Bot commented May 20, 2026

Review changes with  SemanticDiff

Changed Files
File Status
  src/lib/nodes/create.ts  98% smaller
  src/index.ts  67% smaller
  src/lib/nodes/index.ts  5% smaller
  src/lib/posthog.ts  5% smaller

@indent
Copy link
Copy Markdown
Contributor

indent Bot commented May 20, 2026

PR Summary

Adds a feature-flag-gated --enable-infiniband option to sf nodes create so accounts with the infiniband-preview PostHog flag can opt into the preview InfiniBand passthrough surface. When set, the CLI forwards _preview_enable_infiniband: true to the nodes SDK (already supported in @sfcompute/nodes-sdk-alpha@0.1.0-alpha.31). Mirrors the gating pattern already used by procurements and zones.

  • src/lib/nodes/create.ts: createCreateCommand now takes { enableInfiniband }; when true, mounts the --enable-infiniband option and the action conditionally spreads _preview_enable_infiniband: true into the SDK request.
  • src/lib/nodes/index.ts: calls isFeatureEnabled("infiniband-preview") during registerNodes and passes the result into createCreateCommand.
  • src/lib/posthog.ts: widens the FeatureFlags union with "infiniband-preview".

Issues

All clear! No issues remaining. 🎉

4 issues already resolved
  • Help text omits experimental/preview status: --enable-infiniband describes itself as a normal toggle even though the underlying SDK field _preview_enable_infiniband is marked "Experimental — subject to change or removal without notice"; flagged users have no signal that the surface is preview-only.
  • First-run UX hides --enable-infiniband until account_id is cached: trigger — a fresh login (or cleared ~/.sfcompute/feature-flags) means registerNodes runs before account_id is hydrated in src/index.ts, so isFeatureEnabled("infiniband-preview") short-circuits to false and the option isn't mounted on the very first sf nodes create invocation. (fixed by commit c5d6435)
  • sf nodes images --help and sf vm images --help now print example commands prefixed with sf images … (e.g. sf images upload, sf images list, Use sf images list --json …) instead of the path the user actually invoked, since the shared factory hard-codes sf images in help/next-steps strings. (fixed by commit 9fb7eb8)
  • New commit refactor!: drop createImagesCommand options introduces a BREAKING change to sf nodes images list --json and sf vm images list --json output shape (bare array → envelope object), but the PR title/description say nothing about images and only describe the InfiniBand feature — reviewers and downstream script consumers won't see this coming. (fixed by commit 9fb7eb8)

CI Checks

All CI checks pass on 56cb42d (encapsulated infiniband-preview flag check inside createCreateCommand): biome lint, bun run check (tsc --noEmit), and bun run test (vitest) are all green.

Comment thread src/lib/nodes/create.ts
Comment thread src/lib/nodes/index.ts Outdated
Copy link
Copy Markdown

@capy-ai capy-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 1 comment

Comment thread src/lib/nodes/create.ts
@sigmachirality sigmachirality force-pushed the indent-2026-05-15-bump-nodes-sdk-alpha-29 branch from 78f3d98 to 81109ce Compare May 20, 2026 23:35
@sigmachirality sigmachirality force-pushed the indent-2026-05-15-enable-infiniband-flag branch from b620823 to 7b33bf5 Compare May 20, 2026 23:36
@sigmachirality sigmachirality force-pushed the indent-2026-05-15-bump-nodes-sdk-alpha-29 branch from 81109ce to 2e1124c Compare May 20, 2026 23:37
@sigmachirality sigmachirality force-pushed the indent-2026-05-15-enable-infiniband-flag branch from 7b33bf5 to e5c0c0f Compare May 20, 2026 23:37
Comment thread src/lib/images/index.ts
@sigmachirality sigmachirality force-pushed the indent-2026-05-15-enable-infiniband-flag branch 2 times, most recently from 9fb7eb8 to 87776f2 Compare May 20, 2026 23:59
@indent indent Bot force-pushed the indent-2026-05-15-bump-nodes-sdk-alpha-29 branch from d7ebf2b to db73844 Compare May 20, 2026 23:59
@indent indent Bot changed the title feat(nodes): accept undocumented --enable-infiniband flag on sf nodes create feat(nodes): add feature-flag-gated --enable-infiniband to sf nodes create May 21, 2026
@sigmachirality sigmachirality changed the title feat(nodes): add feature-flag-gated --enable-infiniband to sf nodes create feat: accept undocumented --enable-infiniband flag on sf nodes create May 21, 2026
@sigmachirality
Copy link
Copy Markdown
Member Author

@indent intelligently resolve conflicts with main

sigmachirality and others added 2 commits May 21, 2026 00:07
…create`

Convert `create` to a `createCreateCommand({ enableInfiniband })` factory
so the call site can thread in the result of the `infiniband-preview`
feature-flag check. When the flag is enabled for the caller's account,
the option is mounted on `sf nodes create` and forwards
`_preview_enable_infiniband: true` to the nodes SDK. When the flag is
disabled, the surface is unchanged.
…gates resolve on first run

Previously `registerNodes` (and other `register*` calls that consult
`isFeatureEnabled`) ran before `account_id` was hydrated, so the very
first CLI invocation after login silently saw `false` for every flag
and hid gated surfaces like `--enable-infiniband` until the cache was
seeded by a subsequent run. The hydration is also now awaited so the
analytics block reuses the same identifier without racing the config
write.

Generated with [Indent](https://indent.com)
Co-Authored-By: sigmachirality <sigmachirality@users.noreply.github.com>
@indent indent Bot force-pushed the indent-2026-05-15-enable-infiniband-flag branch from c5d6435 to fd12807 Compare May 21, 2026 00:08
@indent indent Bot changed the base branch from indent-2026-05-15-bump-nodes-sdk-alpha-29 to main May 21, 2026 00:08
@indent
Copy link
Copy Markdown
Contributor

indent Bot commented May 21, 2026

Rebased onto origin/main and re-targeted the PR base from indent-2026-05-15-bump-nodes-sdk-alpha-29 to main.

The base branch was effectively a no-op against main (git diff origin/main..origin/indent-2026-05-15-bump-nodes-sdk-alpha-29 is empty after #262 was squash-merged), so:

  • The chore: biome-sort imports commit was auto-skipped as "previously applied" — its content is already on main.
  • The two PR-specific commits (feat(nodes): add feature-flag-gated --enable-infiniband and fix(cli): hoist account_id hydration above register*) replayed cleanly with no conflicts.

PR now shows a clean 4-file diff against main:

  • src/index.ts (+15/-12) — account_id hoist
  • src/lib/nodes/create.ts (+391/-353) — factory conversion + InfiniBand option
  • src/lib/nodes/index.ts (+5/-4) — feature flag check + factory wiring
  • src/lib/posthog.ts (+1/-1) — FeatureFlags union extension

bun run check (tsc --noEmit) is clean on the touched files; biome only reports pre-existing warnings unrelated to the PR.

…ateCreateCommand

The factory now resolves its own feature-flag state instead of having
the call site pre-compute it and pass `{ enableInfiniband }` in, so the
gating concern stays co-located with the option it controls. `registerNodes`
just does `await createCreateCommand()`.

Generated with [Indent](https://indent.com)
Co-Authored-By: sigmachirality <sigmachirality@users.noreply.github.com>
Copy link
Copy Markdown

@apkumar-sfc apkumar-sfc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a small nitpick, otherwise lgtm

Comment thread src/lib/nodes/create.ts Outdated
export function createCreateCommand({
enableInfiniband,
}: {
enableInfiniband: boolean;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick, this could be named to make it more obvious it's a feature flag rather than like a cli flag.

shouldDisplayEnableInfiniband or enableInfinibandFeatureFlag?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by encapsulating feature flag call in createCreateCommand in 56cb42d

@sigmachirality sigmachirality changed the title feat: accept undocumented --enable-infiniband flag on sf nodes create feat: accept --enable-infiniband flag on sf nodes create May 21, 2026
@sigmachirality sigmachirality merged commit 763cdf2 into main May 21, 2026
2 checks passed
@sigmachirality sigmachirality deleted the indent-2026-05-15-enable-infiniband-flag branch May 21, 2026 00:14
Copy link
Copy Markdown

@capy-ai capy-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added 1 comment

Comment thread src/index.ts
const config = await loadConfig();
let exchangeAccountId = config.account_id;
if (!exchangeAccountId) {
const client = await apiClient(config.auth_token);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[🟡 Medium] [🔵 Bug]

The new pre-registration account hydration runs an unguarded network call before any command is parsed, so transient network/DNS failures can throw and terminate the process even for commands like help/version. ts // src/index.ts if (!exchangeAccountId) { const client = await apiClient(config.auth_token); const { data } = await client.GET("/v1/account/me", {}); if (data?.id) { This matters because command surfaces now depend on startup network reliability instead of failing open when feature-flag resolution is unavailable. Wrap this hydration block in try/catch (or otherwise treat failures as non-fatal) so CLI startup proceeds when account lookup cannot be completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants