feat: [docs] clarify pinned tool delivery by AlexanderLanin · Pull Request #139 · eclipse-score/devcontainer

AlexanderLanin · 2026-08-12T22:22:00Z

Summary

clarify the consumer-facing pinned-tool runner and Bazel setup
document the Bazel and DevContainer delivery responsibilities
remove unsupported troubleshooting guidance

Scope

Documentation and a lockfile final-newline correction only.

Copilot

Pull request overview

Clarifies pinned-tool delivery while reorganizing supporting installers and documentation generation.

Changes:

Documents the unified DevContainer/Bazel runner.
Adds generated tool-version documentation.
Relocates installer utilities and updates feature integrations.

Reviewed changes

Copilot reviewed 15 out of 18 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`tools/run-tool`	Selects container or Bazel execution.
`tools/README.md`	Adds consumer-facing usage guidance.
`tools/internal/tool-delivery.svg`	Updates diagram title.
`tools/internal/sync_readme.py`	Generates the documented tool table.
`tools/internal/README.md`	Documents maintenance and delivery.
`tools/internal/devcontainer/install.py`	Relocates installer and adds catalog loading.
`tools/BUILD.bazel`	Reorders tool aliases.
`src/s-core-devcontainer/.devcontainer/s-core-local/tests/test_default.sh`	Updates installer and version-loader paths.
`src/s-core-devcontainer/.devcontainer/s-core-local/load_feature_versions.sh`	Adds feature-version loading.
`src/s-core-devcontainer/.devcontainer/s-core-local/install.sh`	Uses relocated utilities.
`src/s-core-devcontainer/.devcontainer/devcontainer-lock.json`	Adds the final newline.
`src/s-core-devcontainer/.devcontainer/bazel-feature/tests/test_default.sh`	Updates utility paths.
`src/s-core-devcontainer/.devcontainer/bazel-feature/load_feature_versions.sh`	Adds Bazel feature-version loading.
`src/s-core-devcontainer/.devcontainer/bazel-feature/install.sh`	Uses relocated utilities.
`README.md`	Clarifies container and pinned-tool usage.
`MODULE.bazel`	Reorders multitool registrations.
`.pre-commit-config.yaml`	Updates runner paths and adds documentation validation.
`.devcontainer/post_create_command.sh`	Relocates installer usage and adds tools.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

AlexanderLanin · 2026-08-12T22:35:25Z

 REPOSITORY_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd -P)"

-sudo "${REPOSITORY_ROOT}/tools/tool_installer.py" install bazelisk shellcheck yamlfmt
+sudo "${REPOSITORY_ROOT}/tools/internal/devcontainer/install.py" install bazelisk shellcheck yamlfmt uv uvx


I have no idea why, but it seems we forgot about adding uv in the past?

which would technically make this PR a feat

uv and uvx are already installed in the devcontainer:

devcontainer/src/s-core-devcontainer/.devcontainer/s-core-local/install.sh

Line 61 in e572ac6

/usr/local/share/score-tools/tool_installer.py install shellcheck ruff actionlint yamlfmt uv uvx apm opencode

You are looking at the devcontainer, which is used to build the S-CORE devcontainer. It is like devcontainers inside devcontainers. I would see improving the outer devcontainer not as important, because this only affects this repository.

Ah! Then we can just leave it in for convenience or drop uv again. Personally I dont use it anyway at the moment.

Add local pre-commit hooks that run the pinned ruff tool via tools/run-tool, mirroring the existing yamlfmt/shellcheck hooks: - ruff-format: formats Python files - ruff-check: lints Python files with --fix Also apply ruff format to tools/internal/sync_readme.py.

Remove the uv/uvx installation added to the outer (repository) DevContainer's post_create_command.sh. This PR is scoped to documentation and a lockfile newline fix; uv/uvx are already installed separately inside the generated S-CORE DevContainer (src/s-core-devcontainer/.devcontainer/s-core-local/install.sh), and no functional need for them in the outer DevContainer was identified in review (see PR eclipse-score#139 discussion).

Move load_feature_versions.sh into tools/internal/devcontainer/ so it is shared via the existing "COPY tools /usr/local/share/score-tools" mechanism, the same way install.py already is. This restores the pre-existing behavior where both the s-core-local and bazel-feature DevContainer features sourced a single shared script (previously tools/versions.sh), instead of keeping two identical copies of the file, one per feature. Both install.sh and test_default.sh scripts in each feature now source the script from /usr/local/share/score-tools/internal/ devcontainer/load_feature_versions.sh instead of a per-feature copy.

pre-commit already detects and rejects commits when a hook modifies tracked files, the same mechanism relied on by the yamlfmt and ruff-format hooks in this repo. sync_readme.py's --check mode was therefore redundant: even without it, running the script always updates tools/README.md in place when it drifts from the lockfile catalog, and pre-commit fails the commit on that modification just like any other formatter hook. Simplify sync_readme.py to always update README.md and drop --check from the pre-commit entry and its documentation.

Reword the rationale in tools/internal/README.md: DevContainers already work on Linux and macOS hosts, so that wasn't the actual gap. The runner exists to keep developers who don't run the DevContainer at all on the same pinned tool versions as those who do.

The generated table accidentally recorded shellcheck's version as 9.9.9 instead of 0.10.0. This was introduced while manually testing pre-commit's file-modification detection for the previous commit (the test copy of tools/README.md was left staged and re-checked-out from the index instead of HEAD, propagating the corruption).

Add a "description" field to every tool entry in tools/lockfiles/*.lock.json and drop the separate PURPOSES dict from sync_readme.py. Descriptions now live next to the version and binaries they document, so adding or removing a pinned tool only requires touching its lockfile instead of also editing sync_readme.py. install.py gains load_catalog_descriptions(), mirroring the existing load_catalog_versions(), backed by a shared _iter_catalog() helper. sync_readme.py uses both to render the README table and still fails loudly if a lockfile entry is missing its description. Update the "Adding or updating a tool" instructions in tools/internal/README.md accordingly.

Remove the uv/uvx installation added to the outer (repository) DevContainer's post_create_command.sh. This PR is scoped to documentation and a lockfile newline fix; uv/uvx are already installed separately inside the generated S-CORE DevContainer (src/s-core-devcontainer/.devcontainer/s-core-local/install.sh), and no functional need for them in the outer DevContainer was identified in review (see PR eclipse-score#139 discussion). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 24 out of 27 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

.pre-commit-config.yaml:52

This adds a formatter that rewrites Python files during pre-commit, which is a functional workflow change outside the declared “documentation and a lockfile final-newline correction only” scope. Remove it from this PR, or update the PR scope and validation expectations to cover the new formatting behavior.

      - id: ruff-format
        name: ruff format
        entry: tools/run-tool ruff format
        language: system
        types: [python]

.pre-commit-config.yaml:63

The generator imports its catalog-loading behavior from internal/devcontainer/install.py, but this filter does not trigger the hook when that dependency alone changes. Such a commit can bypass the local stale-table check described in tools/internal/README.md:127-128; include the imported module in the filter.

        files: ^tools/(README\.md|internal/sync_readme\.py|lockfiles/.*\.lock\.json)$

AlexanderLanin · 2026-08-13T14:55:56Z

+| Command | Version | Purpose |
+| --- | --- | --- |
+| `actionlint` | `1.7.7` | Check GitHub Actions workflow files |
+| `apm` | `0.27.0` | Manage APM packages |


oh crap. thats the perfect example of scope creep

When looking at the container image with dive I also noticed that extraction of apm installs too much files. I expected only a binary. I guess we should deal with that separately

AlexanderLanin · 2026-08-13T15:04:47Z

@lurtz should I split this up to smaller PRs or what should we do?

lurtz · 2026-08-13T15:09:52Z

You can then remove this code. It was only added for apm: b789f95#diff-6712152ff011de291c818780862bf7744ba908d4f50b9cb820316706f1238f20

lurtz · 2026-08-13T15:13:20Z

@lurtz should I split this up to smaller PRs or what should we do?

I am also fine with this one. I am also curious if you fixed this:

$ du -hs /usr/local/bin/_internal/
14M     /usr/local/bin/_internal/

My suspicion is that this directory was introduced with apm.

AlexanderLanin · 2026-08-13T15:38:34Z

My suspicion is that this directory was introduced with apm.

According to GPT this is normal apm behavior. _internal belongs to apm, and is not some erroneous artefact.

AlexanderLanin · 2026-08-13T15:40:26Z

we can store apm somewhere else and link /usr/local/bin/apm -> some other location. Then this _internal is not unter bin.

lurtz · 2026-08-13T15:44:15Z

we can store apm somewhere else and link /usr/local/bin/apm -> some other location. Then this _internal is not unter bin.

Then lets keep the current solution.

lurtz · 2026-08-13T15:46:04Z

My suspicion is that this directory was introduced with apm.

According to GPT this is normal apm behavior. _internal belongs to apm, and is not some erroneous artefact.

I just checked it and you are right. Deleting _internal will break apm.

github-actions · 2026-08-17T00:08:14Z

🎉 This PR is included in version 1.11.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

refactor: organize pinned tool infrastructure

c6fbf2a

AlexanderLanin force-pushed the tooling-documentation-cleanup branch from 11062fc to c6fbf2a Compare August 12, 2026 22:23

AlexanderLanin requested a balanced review from Copilot August 12, 2026 22:25

Copilot started reviewing on behalf of AlexanderLanin August 12, 2026 22:26 View session

Copilot AI reviewed Aug 12, 2026

View reviewed changes

AlexanderLanin marked this pull request as ready for review August 12, 2026 22:37

AlexanderLanin requested review from lurtz and opajonk as code owners August 12, 2026 22:37

lurtz requested changes Aug 13, 2026

View reviewed changes

AlexanderLanin added 7 commits August 13, 2026 10:26

AlexanderLanin requested a balanced review from Copilot August 13, 2026 13:13

Copilot started reviewing on behalf of AlexanderLanin August 13, 2026 13:14 View session

AlexanderLanin force-pushed the tooling-documentation-cleanup branch from 3385749 to bd2a0f8 Compare August 13, 2026 13:15

Copilot AI reviewed Aug 13, 2026

View reviewed changes

AlexanderLanin added 3 commits August 13, 2026 16:56

merge: update from upstream main

0c0cb4f

merge: update feature branch

3031b55

merge: update from upstream main

41182fe

lurtz reviewed Aug 13, 2026

View reviewed changes

refactor: remove obsolete archive installer branch

7189b15

lurtz approved these changes Aug 13, 2026

View reviewed changes

lurtz changed the title ~~docs: clarify pinned tool delivery~~ feat: [docs] clarify pinned tool delivery Aug 13, 2026

lurtz added this pull request to the merge queue Aug 13, 2026

Merged via the queue into eclipse-score:main with commit e6e340b Aug 13, 2026
7 checks passed

github-actions Bot added the released label Aug 17, 2026

Uh oh!

Conversation

AlexanderLanin commented Aug 12, 2026

Summary

Scope

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

AlexanderLanin Aug 12, 2026

Choose a reason for hiding this comment

Uh oh!

lurtz Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

AlexanderLanin Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

AlexanderLanin Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

lurtz Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

AlexanderLanin commented Aug 13, 2026

Uh oh!

lurtz Aug 13, 2026

Choose a reason for hiding this comment

Uh oh!

lurtz commented Aug 13, 2026

Uh oh!

AlexanderLanin commented Aug 13, 2026

Uh oh!

AlexanderLanin commented Aug 13, 2026

Uh oh!

lurtz commented Aug 13, 2026

Uh oh!

lurtz commented Aug 13, 2026

Uh oh!

Uh oh!

github-actions Bot commented Aug 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants