Copy jam filest to dist/, load generated services from dist/#103
Merged
Copy jam filest to dist/, load generated services from dist/#103
Conversation
📝 WalkthroughWalkthroughThe PR refactors build and deploy workflows to centralize service artifact management: it introduces Changes
Sequence DiagramssequenceDiagram
participant CLI as Build Command
participant Docker as callDockerBuild
participant FS as Filesystem
participant SDK as jammin-sdk
CLI->>+SDK: buildService(service, projectRoot)
SDK->>+Docker: callDockerBuild(service, projectRoot)
Docker->>Docker: Execute Docker build
Docker-->>-SDK: Return build output
SDK->>+FS: Locate generated .jam file
FS-->>-SDK: Return .jam file path
SDK->>+SDK: copyJamToDist(jamPath, serviceName, projectRoot)
SDK->>+FS: Create dist/ if needed
FS-->>-SDK: dist/ exists
SDK->>+FS: Copy .jam to dist/{serviceName}.jam
FS-->>-SDK: Copy complete
SDK->>+FS: Write build output log via Bun
FS-->>-SDK: Log written
SDK-->>-CLI: Return relative path to dist file
sequenceDiagram
participant CLI as Deploy Command
participant SDK as jammin-sdk
participant Config as Build Config
participant FS as Filesystem
CLI->>+SDK: loadServices(projectRoot)
SDK->>+Config: loadBuildConfig(projectRoot)
Config-->>-SDK: Return config with services
SDK->>+FS: Locate .jam files for each service
FS-->>-SDK: Return .jam file paths
SDK->>SDK: Assign IDs (reuse configured or auto-increment)
SDK->>+SDK: generateServiceOutput for each service
SDK-->>-SDK: Return ServiceBuildOutput array
SDK-->>-CLI: Return service build outputs
CLI->>+SDK: generateGenesis(buildOutputs)
SDK-->>-CLI: Return genesis data
CLI->>+FS: saveStateFile(genesisData)
FS-->>-CLI: State file saved
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bin/cli/src/commands/build-command.ts (1)
1-1: Selecting the first .jam file is nondeterministic; pick the newest (or expected) artifact.
getJamFiles()can return multiple artifacts; array order is filesystem-dependent. This can silently copy a stale .jam when multiple files exist. Consider choosing the most recently modified file (or matching an expected filename).🛠️ Suggested fix (pick newest by mtime)
-import { mkdir } from "node:fs/promises"; +import { mkdir, stat } from "node:fs/promises"; @@ - const files = await getJamFiles(servicePath); - - const file = files.length > 0 ? files[0] : undefined; + const files = await getJamFiles(servicePath); + const filesWithMtime = await Promise.all( + files.map(async (filePath) => ({ + filePath, + mtimeMs: (await stat(filePath)).mtimeMs, + })), + ); + const file = filesWithMtime.sort((a, b) => b.mtimeMs - a.mtimeMs)[0]?.filePath;Also applies to: 52-59
🤖 Fix all issues with AI agents
In `@bin/cli/src/commands/build-command.test.ts`:
- Around line 135-137: The test currently invokes callDockerBuild twice without
awaiting, causing race/false-positive issues; fix it by calling callDockerBuild
once (e.g., const p = callDockerBuild(service, "/test/project")) and use await
expect(p).rejects.toThrow("Build failed for service 'failing-service'") (or
await expect(p).rejects.toThrow() then with message) so the rejection assertion
is awaited and the promise is not executed twice; reference callDockerBuild and
the failing-service assertion in the test.
In `@bin/cli/src/commands/deploy-command.ts`:
- Line 48: Fix the typo in the user-facing log call: update the p.log.message
invocation that currently prints "🎁 Genereted file: genesis.json" to use the
correct spelling "Generated" (i.e., "🎁 Generated file: genesis.json") so the
CLI shows a correct message; locate the call to p.log.message in
deploy-command.ts and change the string accordingly.
- Line 41: loadServices is currently called without context and loads artifacts
for every configured service, causing single-service deploys to fail; modify the
call site in deploy-command.ts to pass the list of services you actually built
(the selected service names), update loadServices' signature (and any callers)
to accept an optional whitelist of service names, and change its implementation
(in generate-service-output.ts / the utils that produce service outputs) to
filter artifacts by that whitelist so loadServices only validates/returns
outputs for the built services.
In `@packages/jammin-sdk/utils/generate-service-output.ts`:
- Around line 16-18: Update the top docstring that currently reads "Load a
service from the dist/ directory by name" to reflect that the module/function
loads multiple services (plural); edit the comment in
packages/jammin-sdk/utils/generate-service-output.ts to something like "Load
services from the dist/ directory by name (supports loading multiple services)"
so it accurately documents the behavior of the module/function that enumerates
and imports multiple service outputs.
- Around line 19-23: loadServices accepts a projectRoot but still calls
loadBuildConfig() which uses process.cwd(), so the build config may be read from
the wrong directory; update loadServices to resolve the config relative to the
provided projectRoot by calling loadBuildConfig with the projectRoot (or first
resolve a config path with projectRoot and pass that into loadBuildConfig), and
ensure any subsequent artifact resolution (e.g., serviceDeployConfigs and
usedIds logic) uses paths derived from projectRoot rather than the current
working directory; refer to the loadServices function and the loadBuildConfig
call to locate where to pass/derive the projectRoot.
🧹 Nitpick comments (1)
bin/cli/src/commands/build-command.ts (1)
20-43: Add JSDoc for the exported build helpers.As per coding guidelines Use JSDoc comments for public APIs (functions, classes, exported types).📝 Suggested additions
+/** + * Run the Docker build for a single service and return combined output. + */ export async function callDockerBuild(service: ServiceConfig, projectRoot: string): Promise<string> { @@ } +/** + * Build a service, copy the resulting .jam into dist/, and return the dist-relative path. + */ export async function buildService(service: ServiceConfig, projectRoot: string): Promise<string> {
skoszuta
approved these changes
Jan 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolve: #98
*.jamfiles and copy them toroot/dist/under name{service.name}.jamdist/folderbuildanddeploycommandgenesis.jsontodist/(instead of root)