Skip to content

feat(core): clearSuspensionRecord helper, cleared after CLI creates - #166

Merged
birme merged 1 commit into
mainfrom
feat-163-clear-suspension-record
Oct 17, 2026
Merged

birme merged 1 commit into
mainfrom
feat-163-clear-suspension-record

Conversation

@birme

@birme birme commented Oct 17, 2026

Copy link
Copy Markdown
Member

Closes #163

Summary

  • @osaas/client-core: new exported clearSuspensionRecord(context, serviceId, name): Promise<void>. It sends DELETE https://deploy.svc.<env>.osaas.io/mysuspended/{serviceId}/{name} with the context PAT as x-pat-jwt and no Content-Type. A 404 is a no-op, any other error is logged at debug and swallowed, and it never throws. The endpoint exists in osaas-deploy-manager src/api_mysuspended.ts (PAT JWT, 200 {message} / 404 / 500), verified on current main. No new timeout constant.
  • @osaas/cli: calls the helper after each successful create: osc create (user/cmd.ts), and web config-create for both the valkey-io-valkey and eyevinn-app-config-svc creates. The grep from the directions found no other createInstance/createJob/create*Instance/setupDatabase call sites in packages/cli/src. The valkey create is already guarded by getInstance, so the helper runs only when an instance was actually created. Output and exit codes are unchanged.
  • Dependency ranges: every package declared @osaas/client-core as ^0.26.0, while the workspace core is 0.28.0, so npm installed a nested published 0.26.0 in each package and the CLI could not see the new export. All ranges are now ^0.28.0 (lockfile drops the nested copies and links the workspace). Bumping only the CLI broke typechecking because the sibling packages returned a different Context type. No version bump or release commit.

Impact on existing consumers: createInstance and createJob are untouched (the diff does not change packages/core/src/core.ts or job.ts), so deploy-manager's recreateInstance, osaas-ai, @osaas/client-services and @osaas/client-db behave exactly as before. The only change to the public surface of client-core is one added export.

Tests

  • packages/core/src/suspension.test.ts: 200 (URL, x-pat-jwt, no Content-Type), 404, 500, network error.
  • packages/cli/src/web/config-create.test.ts: success clears for both creates; an existing instance does not clear; a failed create does not clear.
  • Run locally: lerna build passes, CLI jest suite passes, eslint and prettier clean on changed files.

Lessons

Package ranges of ^0.26.0 on a 0.x workspace package are not satisfied by the 0.28.0 workspace version, so npm silently installs a nested published copy and a new core export is invisible to the CLI (and a partial bump causes duplicate Context type errors). Bump all dependents together. After merge, the helper only reaches CLI users once a human releases client-core and the CLI.

🤖 Generated with Claude Code

Co-Authored-By: Claude noreply@anthropic.com

Co-Authored-By: Claude <noreply@anthropic.com>
@birme birme self-assigned this Oct 17, 2026
@birme

birme commented Oct 17, 2026

Copy link
Copy Markdown
Member Author

Research Review

Verified

  • Endpoint: DELETE /mysuspended/:serviceId/:instanceName exists in osaas-deploy-manager src/api_mysuspended.ts (fastify.delete, ~line 297), PAT JWT via x-pat-jwt, responses 200 {message:'deleted'} / 404 No suspended instance found / 500. Matches the helper's URL, method and 404-as-no-op handling.
  • Header format: x-pat-jwt: Bearer <pat> matches the server's extractPat regex and the existing pattern in core/src/context.ts:102 and myapp.ts. No Content-Type on DELETE (matches the Fastify empty-body rule).
  • Context.getEnvironment() / getPersonalAccessToken() exist (context.ts:83,87); createFetch and FetchError.httpCode exist as used (fetch.ts). A network error is wrapped into a FetchError with no httpCode, so it falls to the debug-log-and-swallow branch. Helper never throws, as claimed.
  • createInstance / createJob are untouched (diff does not touch core.ts or job.ts), so deploy-manager recreateInstance, osaas-ai, client-services and client-db see only one added export. The opt-in design follows the "default false" rule.
  • Call sites: grep -rnE "createInstance\(|createJob\(|create[A-Z]...Instance\(|setupDatabase\(" over packages/cli/src (non-test) gives exactly user/cmd.ts:83, web/cmd.ts:115, web/cmd.ts:136; all three are covered, after the create and before output. The valkey create is inside the existing getInstance guard, so clearing runs only when created.
  • Dependency bump: all dependents declared ^0.26.0 while the workspace core is 0.28.0 (packages/core/package.json); for 0.x, caret does not match 0.28, so nested published copies were installed. Moving all ranges to ^0.28.0 and dropping the nested lockfile entries is coherent. No release/version commit is in the diff. CI ts check passes.

Needs Verification

  • Indirect CLI create paths (non-blocking, worth a follow-up): the directions' grep was limited to packages/cli/src, but the CLI also creates instances through sibling packages: osc db -> ValkeyDb.create (packages/db/src/valkey.ts:48, createValkeyIoValkeyInstance), and vod, transcode, transcribe, packager, live, vmaf -> @osaas/client-transcode (pool.ts:122 etc. call createInstance), plus intercom. These are not covered, and the PR body says only that the cli/src grep found nothing else, without stating why those paths do not apply. birme's requirement was "every creation path". I could not tell whether the ticket intentionally scoped these out; please state it explicitly or open a follow-up ticket so the gap is tracked rather than implied closed.
  • osc create passes payload['name'] to the helper; if the user omitted name in the payload (create may default it server-side), the helper would call /mysuspended/<serviceId>/undefined. Harmless (404 swallowed) but the record would not be cleared. Consider using instance.name from the create response (the config-create test mocks do return name).

Potential Guesses Detected

  • None found.

Adjacent Stubs Detected

  • None (swept suspension.ts, user/cmd.ts, web/cmd.ts for undefined : undefined, || undefined, TODO/FIXME, empty catch). The helper's catch block intentionally logs and swallows, as specified.

Orphaned Files Detected

  • None. suspension.ts is exported from core/src/index.ts and imported by user/cmd.ts and web/cmd.ts.

Escape Hatch Coverage

  • No new terminal-rejection branch; the helper only swallows errors. N/A.

Object Shape Verification

  • No spreads of fetched/ORM objects in the diff. (jest.requireActual('./fetch') spread in the test is a module spread, not a runtime object.)

Contract diff

  • No request handler changed. This is a client library. createInstance and createJob behaviour, return types and timing are unchanged (files checked: core.ts, job.ts not in diff). CLI commands add one awaited best-effort call after create; output and exit codes are unchanged, and the helper cannot throw. Added latency is one extra DELETE per create, and no new timeout is introduced. Not a contract change; the only public-surface change is one added export.

Existing consumers

  • N/A (not compliance-driven).

Impact Analysis

  • Blast radius: client-core gains one export; CLI create commands make one extra authenticated DELETE; all packages' @osaas/client-core ranges move to ^0.28.0.
  • Downstream consumers: client-services, db, transcode, web, ai and intercom pick up core 0.28.0 as their dependency. This is the version they were already built against in the workspace, but published consumers will need core 0.28.0 released before they are republished.
  • Coordinated changes needed: human release of client-core then CLI (stated in PR). Until released, behaviour does not reach CLI users.
  • Breaking change risk: low.

Risks Not Tested

  • Live behaviour against a real deploy-manager (dev) was not exercised; the tests mock createFetch, so the 404 handling relies on the verified server source and FetchError shape, not on a live call.
  • config-create.test.ts mocks all of @osaas/client-core; it proves call ordering only.
  • I did not rebuild or rerun the jest suites locally; I relied on the passing ts CI check.

Research Suggestions

  • State, or ticket, the coverage decision for the indirect CLI create paths (db/transcode/live/vod/intercom).
  • Consider clearing with instance.name rather than payload['name'] in osc create.

Verdict

No blocking issues found.


Reviewed by PR Research Validator. This review checks whether code is based on verified sources rather than assumptions.

@birme
birme merged commit 0ba9e93 into main Oct 17, 2026
1 check passed
@birme
birme deleted the feat-163-clear-suspension-record branch October 17, 2026 07:47
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.

client-core clearSuspensionRecord helper + clear after CLI creates (Eyevinn/osaas-deploy-manager#1878)

2 participants