Repository navigation
Pin @osaas/cli version in the entrypoint - #34
Merged
Merged
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Research Review
Verified
- Linked issue: PR body says
Part of #33; issue #33 AC1 ("No@osaas/cli@latestremains; the version is defined in one place") is met:grepof the PR head findsOSAAS_CLI_VERSIONonly inDockerfile:31, the entrypoint:232and the test env, and no remaining@osaas/cli@latest. Issue #33 carries a## Spec Invalidcomment for AC3 (non-root), soPart ofwith no closing keyword is correct and the issue stays open. - Version:
npm view @osaas/cli@4.35.0resolves, and it is the currentlatestdist-tag, so the pin keeps current behaviour. - Pattern parity:
Eyevinn/web-runnerhas the sameENV OSAAS_CLI_VERSION=4.35.0in its Dockerfile (line 59) and the same${OSAAS_CLI_VERSION:?...}expansion in its entrypoint (line 370). - ENV propagation: the Dockerfile
ENVis inherited by the ENTRYPOINT process, so the entrypoint sees it at runtime. - Fail-fast behaviour: I ran the
${VAR:?msg}expansion inside$(...)with the variable unset. The command substitution exits 1 with "must be set", and the entrypoint's&& config_exit=0 || config_exit=$?captures it, so the existingconfig_failedpath handles it (a clear message and non-zero exit, not a silent continue). - Tests: I checked out the PR head and ran all six
tests/test-entrypoint-*.shfrom the repo root. All pass (7, 13, 6, 8, 7 and 7 passed, 0 failed). The test env change matches what the Lessons section says aboutenv -i.
Needs Verification
- The image build was not run (the PR body says so; the repo has no PR CI, only the sync-fork workflows). The Dockerfile change is a single
ENVline, low risk.
Adjacent Stubs Detected
- None. I grepped
scripts/docker-entrypoint.shandDockerfilefor TODO/FIXME/XXX/HACK and undefined-fallback patterns and found nothing.
Orphaned Files Detected
- None. No new files; the entrypoint is the image ENTRYPOINT.
Escape Hatch Coverage
scripts/docker-entrypoint.sh:232: new terminal-rejection whenOSAAS_CLI_VERSIONis unset. Reachable only if the image is run with the variable cleared or the entrypoint is used outside this Dockerfile. Bypass claimed: none. The Dockerfile sets the variable, so a legitimate user of the image always passes. Passing-path test: the existing config-load tests run withOSAAS_CLI_VERSION=4.35.0and exercise the success path (7/7 pass). Not blocking.
Object Shape Verification
- Not applicable (shell and Dockerfile change, no fetched object spread).
Contract diff
No request handler or timeout changed. Files checked: Dockerfile, scripts/docker-entrypoint.sh, tests/test-entrypoint-config-load-failure.sh. The existing timeout 60s on the config load line is unchanged. Container startup behaviour is unchanged while the pinned version equals the current latest (4.35.0).
Impact Analysis
- Blast radius: python-runner image only, specifically the config-load step (
CONFIG_SVCset withOSC_ACCESS_TOKEN). - Downstream consumers: none found that depend on
@latestresolution.grepof the repo shows no other@osaas/clireferences. - Coordinated changes needed: none identified. Future CLI bumps are manual edits to the Dockerfile (same as web-runner).
- Breaking change risk: low. The pin equals the current
latest.
Risks Not Tested
- Docker image build and a real container run with a live config service were not exercised (no docker available to me either).
- Whether the
web config-to-envsubcommand output format stays stable across future CLI bumps is outside this PR.
Research Suggestions
- AC3 (non-root) remains open on #33; the follow-up PR should carry
Closes #33.
Verdict
No blocking issues found.
Reviewed by PR Research Validator. This review checks whether code is based on verified sources rather than assumptions.
This was referenced Nov 4, 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
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.
Part of #33
Pins the
@osaas/cliversion used by the entrypoint instead of resolving@latestat container start.Dockerfile: newENV OSAAS_CLI_VERSION=4.35.0, the single place the version is defined.scripts/docker-entrypoint.sh: the config load step runsnpx -y "@osaas/cli@${OSAAS_CLI_VERSION}"and fails fast with a clear message if the variable is unset. Same pattern as web-runner.tests/test-entrypoint-config-load-failure.sh: setsOSAAS_CLI_VERSIONin the test environment (it runs the entrypoint underenv -i).Covers acceptance criterion 1 only. The non-root criterion (3) is not part of this PR; see the comment on #33.
Verified:
tests/test-entrypoint-*.shpass locally from the repo root (config-load-failure, subpath, setup-timeout run explicitly).Not verified: the image build (docker is not available in the environment that produced this change).
Lessons
The entrypoint tests run the script under
env -i, so any newly required variable has to be added to the test environment or the config block fails before the stubbednpxis reached. The tests must be run from the repo root because they readscripts/docker-entrypoint.shby relative path.What a human does next: Nothing.
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com