From 5906a9feb695451b3bf77cccaf9452b6f24e2091 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Mon, 27 Jul 2026 16:06:56 +0000 Subject: [PATCH] docker: close four holes the review found docker-publish.yml: the llama.cpp tag resolver was the one step in the prepare job still using `curl | sed` without pipefail. The runner's default `bash -e` shell takes sed's exit status, so an unreachable github.com left TAG empty and the step published the mutable `latest`. Both arch legs re-resolve that through fetch_llama_prebuilt.py and Dockerfile.studio resolves it a third time, so a release cut mid-run can put different llama.cpp bundles under one manifest. Capture the redirect first and fail the job when it is missing or does not land on a release tag, matching the three ref resolvers below it. unsloth_nb_strip_colab.py: strip_notebook read, parsed and then unconditionally os.replace'd. The refresh child re-arms finalize after the entrypoint has execed the container command, so JupyterLab is already serving the tree and a save landing in that window was destroyed, after which migrate recorded the cleaned hash and marked the notebook pristine forever. Re-read the hash once the staged copy is complete and drop it when the file moved, the same rule the refresh publish in unsloth_sync_notebooks.sh already follows. unsloth_nb_view.py: ownership for the view teardown accepted any symlink target under DEST, but every link the tool creates points at DEST/nb. A shortcut the user made in the landing dir to their own file elsewhere in the checkout was therefore classified as ours and deleted on the next boot. Key ownership on DEST/nb instead. cellNav.ts: the edit-mode boundary test compared the cursor line against editor.lineCount, both logical, while JupyterLab wraps markdown and raw editors by default (StaticNotebook.defaultEditorConfig). A one-line markdown header renders as several visual rows, so every arrow left the cell and the wrapped rows could not be reached. Ask CodeMirror whether it can still move one visual line (EditorView.moveVertically, compared by coordsAtPos top) and keep the logical test as the fallback for a non-CodeMirror editor. New tests: 12 passed / 8 failed before, 20 passed / 0 failed after. --- .github/workflows/docker-publish.yml | 24 ++- docker/jupyter/unsloth_labext/src/cellNav.ts | 38 ++++- docker/unsloth_nb_strip_colab.py | 11 ++ docker/unsloth_nb_view.py | 28 ++-- tests/python/test_docker_labext_cell_nav.py | 73 +++++++++ .../python/test_docker_nb_strip_colab_race.py | 139 ++++++++++++++++++ tests/python/test_docker_nb_view_ownership.py | 118 +++++++++++++++ .../python/test_docker_publish_ref_freeze.py | 90 ++++++++++++ 8 files changed, 497 insertions(+), 24 deletions(-) create mode 100644 tests/python/test_docker_labext_cell_nav.py create mode 100644 tests/python/test_docker_nb_strip_colab_race.py create mode 100644 tests/python/test_docker_nb_view_ownership.py diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index 0382f3dc51..d91f61a14b 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -82,12 +82,26 @@ jobs: run: | TAG="$INPUT_TAG" if [ -z "$TAG" ]; then - TAG="$(curl -fsSL -o /dev/null -w '%{url_effective}' \ - https://github.com/unslothai/llama.cpp/releases/latest \ - | sed -n 's#.*/releases/tag/##p')" + # Same rule as the three ref resolvers below. This step has no + # explicit `shell:`, so it runs under `bash -e` WITHOUT pipefail and + # a failing curl inside `curl | sed` is lost: the step exited 0 and + # published tag=latest. Every consumer resolves that MUTABLE tag + # again -- fetch_llama_prebuilt.py once per arch leg, Dockerfile. + # studio once more -- so a release cut mid-run can put different + # llama.cpp bundles under one manifest. Fail the job instead. + if ! REDIRECT="$(curl -fsSL -o /dev/null -w '%{url_effective}' \ + https://github.com/unslothai/llama.cpp/releases/latest)"; then + echo "::error::unslothai/llama.cpp unreachable; cannot resolve the newest prebuilt tag" + exit 1 + fi + TAG="$(printf '%s\n' "$REDIRECT" | sed -n 's#.*/releases/tag/##p')" + if [ -z "$TAG" ]; then + echo "::error::/releases/latest did not redirect to a release tag (landed on ${REDIRECT})" + exit 1 + fi fi - echo "tag=${TAG:-latest}" >> "$GITHUB_OUTPUT" - echo "llama.cpp prebuilt tag: ${TAG:-latest}" + echo "tag=${TAG}" >> "$GITHUB_OUTPUT" + echo "llama.cpp prebuilt tag: ${TAG}" # Requested-ref precedence: dispatch input, else pushed tag, else trigger # sha, else main -- then frozen to one sha per the job header. diff --git a/docker/jupyter/unsloth_labext/src/cellNav.ts b/docker/jupyter/unsloth_labext/src/cellNav.ts index 4261fd2a37..a398b71fe4 100644 --- a/docker/jupyter/unsloth_labext/src/cellNav.ts +++ b/docker/jupyter/unsloth_labext/src/cellNav.ts @@ -5,6 +5,7 @@ import { JupyterFrontEnd, JupyterFrontEndPlugin } from '@jupyterlab/application'; +import { CodeMirrorEditor } from '@jupyterlab/codemirror'; import { INotebookTracker } from '@jupyterlab/notebook'; /** @@ -65,13 +66,36 @@ const cellNavPlugin: JupyterFrontEndPlugin = { ) { return; } - const line = editor.getCursorPosition().line; - // Only take over at the cell boundary; else let CodeMirror move the cursor. - if (direction === 1 && line !== editor.lineCount - 1) { - return; - } - if (direction === -1 && line !== 0) { - return; + // Only take over at the cell boundary; else let CodeMirror move the + // cursor. `lineCount` counts LOGICAL lines, but JupyterLab wraps + // markdown and raw cell editors by default (StaticNotebook + // .defaultEditorConfig: markdown/raw lineWrap true), so the first and + // last logical line can own several visual rows -- the one-line markdown + // header every notebook opens with wraps to ~7. Ask CodeMirror whether + // it can still move one VISUAL line first, else those rows are + // unreachable: every arrow leaves the cell. + const view = editor instanceof CodeMirrorEditor ? editor.editor : null; + if (view) { + const range = view.state.selection.main; + const moved = view.moveVertically(range, direction === 1); + const from = view.coordsAtPos(range.head); + const to = + moved.head === range.head ? from : view.coordsAtPos(moved.head); + // moveVertically only returns the unchanged head at offset 0 / + // doc.length; elsewhere it clamps to the document edge, so a move that + // stays on the same visual row IS the editor edge and the cell + // boundary is the next stop. + if (from && to && Math.abs(to.top - from.top) > 1) { + return; + } + } else { + const line = editor.getCursorPosition().line; + if (direction === 1 && line !== editor.lineCount - 1) { + return; + } + if (direction === -1 && line !== 0) { + return; + } } } const target = notebook.activeCellIndex + direction; diff --git a/docker/unsloth_nb_strip_colab.py b/docker/unsloth_nb_strip_colab.py index 4ffa67a0a1..8af17f2b79 100644 --- a/docker/unsloth_nb_strip_colab.py +++ b/docker/unsloth_nb_strip_colab.py @@ -136,6 +136,7 @@ def _clean_widgets(nb): def strip_notebook(path): """Return True if the notebook was modified and written back.""" try: + before = _sha256(path) with open(path, "r", encoding = "utf-8") as f: nb = json.load(f) except Exception: @@ -152,6 +153,16 @@ def strip_notebook(path): with open(tmp, "w", encoding = "utf-8") as f: json.dump(nb, f, indent = 1, ensure_ascii = False) f.write("\n") + # The refresh child re-arms this cleanup AFTER the entrypoint has execed + # the container command, so JupyterLab is already serving the tree: a save + # landing between the read above and this replace would be silently + # overwritten, and migrate() would then record the cleaned hash and mark + # the notebook pristine forever. Re-read the live file once the staged + # copy is complete (the same rule the refresh publish in + # unsloth_sync_notebooks.sh follows) and let their edit win. + if _sha256(path) != before: + os.remove(tmp) + return False os.replace(tmp, path) except Exception: try: diff --git a/docker/unsloth_nb_view.py b/docker/unsloth_nb_view.py index 99f795d83f..cc4d244def 100644 --- a/docker/unsloth_nb_view.py +++ b/docker/unsloth_nb_view.py @@ -139,8 +139,11 @@ def build_view( order.append(_OTHER) # Rebuild VIEW: drop our own symlinks/empty folders, never the user's files - # (VIEW is also JupyterLab's landing dir). - _clear_view(view, os.path.realpath(dest)) + # (VIEW is also JupyterLab's landing dir). Ownership is keyed on DEST/nb -- + # the only place our links ever point -- so a shortcut the user made to their + # own file elsewhere in the checkout survives the rebuild. + nb_real = os.path.realpath(nb_dir) + _clear_view(view, nb_real) os.makedirs(view, exist_ok = True) n_links = 0 @@ -152,7 +155,7 @@ def build_view( target = os.path.join(nb_dir, fname) rel = os.path.relpath(target, folder) # ../../unsloth-notebooks/nb/ try: - if os.path.islink(link) and _points_into(link, os.path.realpath(dest)): + if os.path.islink(link) and _points_into(link, nb_real): os.remove(link) # replace our own stale symlink elif os.path.islink(link) or os.path.exists(link): # a real user file occupies this name: keep it, skip linking. @@ -165,23 +168,24 @@ def build_view( return len(order), n_links -def _points_into(link, dest_real): - """True when a symlink resolves into the notebooks tree we link from. +def _points_into(link, nb_real): + """True when a symlink resolves into DEST/nb, the dir we link FROM. Every link this tool creates points at DEST/nb/, so this is the ownership test for cleanup: a user's own symlink (to a dataset, project, - mounted dir, ...) resolves elsewhere and must survive a rebuild. realpath - resolves a broken link's path string too, so stale links to since-removed - notebooks are still recognised as ours. + mounted dir, or their own notebook saved elsewhere in the checkout) resolves + outside DEST/nb and must survive a rebuild -- matching on all of DEST deleted + those. realpath resolves a broken link's path string too, so stale links to + since-removed notebooks are still recognised as ours. """ try: target = os.path.realpath(link) except OSError: return False - return target == dest_real or target.startswith(dest_real + os.sep) + return target == nb_real or target.startswith(nb_real + os.sep) -def _clear_view(path, dest_real): +def _clear_view(path, nb_real): # Tear down a previously built VIEW in place. It is also JupyterLab's landing # dir, so user files/symlinks must survive: unlink only symlinks we own (see # _points_into) and rmdir only emptied folders. The VIEW root is never unlinked. @@ -190,7 +194,7 @@ def _clear_view(path, dest_real): for root, dirs, files in os.walk(path, topdown = False): for name in files: p = os.path.join(root, name) - if os.path.islink(p) and _points_into(p, dest_real): + if os.path.islink(p) and _points_into(p, nb_real): try: os.remove(p) except OSError: @@ -200,7 +204,7 @@ def _clear_view(path, dest_real): p = os.path.join(root, name) try: if os.path.islink(p): - if _points_into(p, dest_real): + if _points_into(p, nb_real): os.remove(p) # our symlinked dir: unlink, never recurse else: os.rmdir(p) # succeeds only if we emptied it diff --git a/tests/python/test_docker_labext_cell_nav.py b/tests/python/test_docker_labext_cell_nav.py new file mode 100644 index 0000000000..4ec3e3eebb --- /dev/null +++ b/tests/python/test_docker_labext_cell_nav.py @@ -0,0 +1,73 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-Present the Unsloth team. See /studio/LICENSE.AGPL-3.0 + +"""Colab-style arrow navigation must not swallow wrapped-line movement. + +`cellNav.ts` owns ArrowUp/ArrowDown in the capture phase and jumps to the +previous/next cell when the cursor sits on the first/last line of the editor. +That test used `editor.getCursorPosition().line` against `editor.lineCount`, +both of which are LOGICAL (JupyterLab's CodeMirrorEditor: `get lineCount() { +return this.doc.lines }`), while JupyterLab wraps markdown and raw cell editors +by default (`StaticNotebook.defaultEditorConfig` -> `markdown: { lineWrap: true +}`, `raw: { lineWrap: true }`; the image's `docker/jupyter/overrides.json` only +sets `autoClosingBrackets`). + +So for a one-line markdown header -- what every Unsloth notebook opens with -- +`lineCount === 1`, the cursor is on line 0 == lineCount - 1 from every visual +row, and BOTH arrows leave the cell: the wrapped rows in between cannot be +reached at all. Measured in Chromium with CodeMirror 6 + EditorView.lineWrapping +at the notebook's editor width: 1 logical line renders as 7 visual rows and the +logical test hijacks the arrows on 7 of 7 rows, in both directions. The same +measurement on an unwrapped code cell shows the visual test agreeing with the +logical one on every row, so the Colab-style jump is unchanged there. + +CodeMirror's own answer is `EditorView.moveVertically(range, forward)`, which +moves "to the next line (including wrapped lines)"; it returns the unchanged +head only at offset 0 / doc.length, so a move that stays on the same visual row +(same `coordsAtPos().top`) is the real editor edge. + +Static source guard: the labextension is only built inside Dockerfile.studio +(`jlpm install && jlpm build:prod`), so there is no TS test runner in-repo. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +CELL_NAV = REPO_ROOT / "docker" / "jupyter" / "unsloth_labext" / "src" / "cellNav.ts" + + +@pytest.fixture(scope = "module") +def source() -> str: + assert CELL_NAV.is_file(), f"missing {CELL_NAV}" + return CELL_NAV.read_text(encoding = "utf-8") + + +def test_the_edit_mode_boundary_test_asks_codemirror_for_a_visual_line(source: str): + assert "moveVertically" in source, ( + "the edit-mode boundary check must ask CodeMirror whether it can still " + "move one VISUAL line (EditorView.moveVertically); a logical lineCount " + "test makes the wrapped rows of a markdown cell unreachable" + ) + + +def test_the_visual_check_compares_screen_rows(source: str): + assert "coordsAtPos" in source, ( + "moveVertically clamps to the document edge instead of returning the " + "same head, so the two positions have to be compared by visual row" + ) + + +def test_the_logical_line_test_is_only_a_fallback(source: str): + body = source[source.index("const editing = notebook.mode === 'edit'") :] + logical = re.search(r"editor\.lineCount - 1", body) + assert logical, "the non-CodeMirror fallback should still exist" + visual = re.search(r"moveVertically", body) + assert visual and visual.start() < logical.start(), ( + "the visual-line test has to run first; the logical one is only for an " + "editor that is not a CodeMirrorEditor" + ) diff --git a/tests/python/test_docker_nb_strip_colab_race.py b/tests/python/test_docker_nb_strip_colab_race.py new file mode 100644 index 0000000000..c236d16878 --- /dev/null +++ b/tests/python/test_docker_nb_strip_colab_race.py @@ -0,0 +1,139 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-Present the Unsloth team. See /studio/LICENSE.AGPL-3.0 + +"""The Colab-intro cleanup must not overwrite a save it did not see. + +`unsloth_sync_notebooks.sh` forks the GitHub refresh into a DETACHED child before +the entrypoint execs the container command, so JupyterLab is already serving +$DEST while that child runs. When the refresh copied anything the child re-arms +`finalize()`, which runs `unsloth_nb_strip_colab.py --state ... --dest ...`, i.e. +`migrate()` -> `strip_notebook()` over every owned+unedited notebook. + +`strip_notebook` read the file, parsed it, serialised the cleaned copy and then +`os.replace`d it unconditionally. A user save that landed in that window was +destroyed, and `migrate` then recorded the cleaned file's hash, so the state +machine treats the notebook as pristine forever after -- the same +check-then-write hole that was closed in the refresh loop itself (the publish +there now re-reads the hash immediately before the rename). + +Behavioural: the save is injected inside the window, while the helper serialises +the cleaned copy (the widest part of it: json parse + dump of a notebook that is +often megabytes). No docker, no network. +""" + +from __future__ import annotations + +import copy +import importlib.util +import json +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +STRIP_PATH = REPO_ROOT / "docker" / "unsloth_nb_strip_colab.py" + +INTRO = 'To run this, press "*Runtime*" and press "*Run all*" on a **free** Tesla T4 Google Colab instance!\n' + + +@pytest.fixture(scope = "module") +def strip(): + assert STRIP_PATH.is_file(), f"missing {STRIP_PATH}" + spec = importlib.util.spec_from_file_location("unsloth_nb_strip_race", STRIP_PATH) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +def notebook(*sources): + return { + "cells": [ + {"cell_type": "markdown", "metadata": {}, "source": list(src)} for src in sources + ], + "metadata": {}, + "nbformat": 4, + "nbformat_minor": 5, + } + + +def write(path: Path, nb) -> None: + path.write_text(json.dumps(nb, indent = 1, ensure_ascii = False) + "\n", encoding = "utf-8") + + +@pytest.fixture +def racing(strip, tmp_path: Path): + """Fire a user save inside the window: after strip_notebook read the file, + while it is serialising the cleaned copy.""" + real_dump = strip.json.dump + state = {"save": None, "path": None, "fired": 0} + + def dump(obj, fp, *args, **kwargs): + out = real_dump(obj, fp, *args, **kwargs) + if state["save"] is not None and state["fired"] == 0: + state["fired"] = 1 + Path(state["path"]).write_text(state["save"], encoding = "utf-8") # Ctrl+S + return out + + strip.json.dump = dump + try: + yield state + finally: + strip.json.dump = real_dump + + +def test_a_save_during_the_cleanup_is_not_overwritten(strip, racing, tmp_path: Path): + path = tmp_path / "Llama.ipynb" + write(path, notebook([INTRO, "\n", "# Llama\n"])) + + edited = notebook([INTRO, "\n", "# Llama\n", "\n", "my own notes, saved from JupyterLab\n"]) + racing["save"] = json.dumps(edited, indent = 1, ensure_ascii = False) + "\n" + racing["path"] = str(path) + + strip.strip_notebook(str(path)) + + on_disk = json.loads(path.read_text(encoding = "utf-8")) + assert on_disk == edited, ( + "the user's save landed after strip_notebook read the file and was " + "overwritten by the cleaned copy of the OLD content; the sync contract " + "is that user edits always win" + ) + + +def test_the_recorded_hash_still_matches_the_file_after_a_racing_save(strip, racing, tmp_path: Path): + # migrate() rewrites STATE with the post-strip hash. If the write above is + # allowed to clobber a save, the state ALSO says "pristine", so every later + # refresh happily overwrites the notebook again. + dest = tmp_path / "unsloth-notebooks" + dest.mkdir() + path = dest / "Llama.ipynb" + write(path, notebook([INTRO, "\n", "# Llama\n"])) + before = strip._sha256(str(path)) + state = tmp_path / ".unsloth_sync_state" + state.write_text(f"{before} Llama.ipynb\n", encoding = "utf-8") + + edited = notebook([INTRO, "\n", "# Llama\n", "\n", "my own notes\n"]) + racing["save"] = json.dumps(edited, indent = 1, ensure_ascii = False) + "\n" + racing["path"] = str(path) + + strip.migrate(str(state), str(dest)) + + recorded = state.read_text(encoding = "utf-8").split(" ", 1)[0] + on_disk = strip._sha256(str(path)) + assert json.loads(path.read_text(encoding = "utf-8")) == edited + assert recorded != on_disk, ( + "a file the user saved during the cleanup must NOT end up recorded as " + "managed-and-pristine, or the next refresh overwrites it too" + ) + + +def test_the_normal_no_race_cleanup_still_strips_and_rewrites(strip, tmp_path: Path): + # Guard the fix from over-reaching: with nobody else writing, the cleanup + # must still strip the Colab sentence and publish the result. + path = tmp_path / "Llama.ipynb" + original = notebook([INTRO, "\n", "# Llama\n"]) + write(path, copy.deepcopy(original)) + + assert strip.strip_notebook(str(path)) is True + cleaned = json.loads(path.read_text(encoding = "utf-8")) + assert cleaned["cells"][0]["source"] == ["# Llama\n"] + assert strip.strip_notebook(str(path)) is False # idempotent diff --git a/tests/python/test_docker_nb_view_ownership.py b/tests/python/test_docker_nb_view_ownership.py new file mode 100644 index 0000000000..5e645f197b --- /dev/null +++ b/tests/python/test_docker_nb_view_ownership.py @@ -0,0 +1,118 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-Present the Unsloth team. See /studio/LICENSE.AGPL-3.0 + +"""The categorized notebook VIEW may only delete the links it created. + +`unsloth_nb_view.py` rebuilds "/workspace/Unsloth Notebooks" on every boot, and +that directory is also JupyterLab's landing dir, so `_clear_view()` promises to +remove only the tool's own symlinks. Every link the tool creates points at +DEST/nb/, but the ownership predicate accepted ANY target under DEST, so a +user's own symlink into the notebooks checkout -- e.g. a shortcut to their own +notebook saved beside it, which the sync script explicitly supports ("kept +existing user file" / "In DEST but never recorded") -- was classified as +tool-owned and deleted on the next boot. + +Behavioural: builds a real DEST/VIEW pair on disk and runs build_view twice. +No docker, no network. +""" + +from __future__ import annotations + +import importlib.util +import os +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +VIEW_PATH = REPO_ROOT / "docker" / "unsloth_nb_view.py" + +README = ( + "### Main Notebooks\n" + "[Llama](nb/Llama3_2_%281B_and_3B%29_Conversational.ipynb)\n" + "### Gemma\n" + "[Gemma](nb/Gemma3_%284B%29.ipynb)\n" +) + + +@pytest.fixture(scope = "module") +def view_mod(): + assert VIEW_PATH.is_file(), f"missing {VIEW_PATH}" + spec = importlib.util.spec_from_file_location("unsloth_nb_view_under_test", VIEW_PATH) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +@pytest.fixture +def tree(tmp_path: Path): + dest = tmp_path / "unsloth-notebooks" + view = tmp_path / "Unsloth Notebooks" + (dest / "nb").mkdir(parents = True) + view.mkdir() + for name in ("Llama3_2_(1B_and_3B)_Conversational.ipynb", "Gemma3_(4B).ipynb"): + (dest / "nb" / name).write_text("{}", encoding = "utf-8") + (dest / "README.md").write_text(README, encoding = "utf-8") + # The user's own notebook, saved inside the checkout (supported by the sync + # script), plus their own folder of shortcuts in the landing dir. + (dest / "my_work").mkdir() + (dest / "my_work" / "experiment.ipynb").write_text("{}", encoding = "utf-8") + return dest, view + + +def link(target: Path, at: Path) -> None: + at.parent.mkdir(parents = True, exist_ok = True) + os.symlink(os.path.relpath(target, at.parent), at) + + +def test_a_user_link_to_their_own_file_in_the_checkout_survives(view_mod, tree): + dest, view = tree + own = view / "00 My favourites" / "experiment.ipynb" + link(dest / "my_work" / "experiment.ipynb", own) + + view_mod.build_view(str(dest), str(view)) + + assert os.path.islink(own), ( + "a symlink the user created in the landing dir, pointing at their own " + "file inside the notebooks checkout, was deleted by _clear_view" + ) + assert os.path.realpath(own) == os.path.realpath(dest / "my_work" / "experiment.ipynb") + + +def test_a_user_link_outside_the_checkout_survives(view_mod, tree, tmp_path: Path): + dest, view = tree + outside = tmp_path / "datasets" + outside.mkdir() + own = view / "datasets" + link(outside, own) + + view_mod.build_view(str(dest), str(view)) + + assert os.path.islink(own) + + +def test_the_tools_own_stale_links_are_still_cleaned_up(view_mod, tree): + dest, view = tree + view_mod.build_view(str(dest), str(view)) + generated = view / "02 Gemma" / "Gemma3_(4B).ipynb" + assert os.path.islink(generated) + + # Upstream drops the notebook: its generated link (now stale, and pointing + # into DEST/nb) has to go, and the emptied folder with it. + (dest / "nb" / "Gemma3_(4B).ipynb").unlink() + (dest / "README.md").write_text( + "### Main Notebooks\n[Llama](nb/Llama3_2_%281B_and_3B%29_Conversational.ipynb)\n", + encoding = "utf-8", + ) + view_mod.build_view(str(dest), str(view)) + + assert not os.path.islink(generated) and not os.path.exists(generated) + assert not (view / "02 Gemma").exists() + + +def test_a_rebuild_is_stable_for_the_links_it_owns(view_mod, tree): + dest, view = tree + view_mod.build_view(str(dest), str(view)) + first = sorted(str(p.relative_to(view)) for p in view.rglob("*")) + view_mod.build_view(str(dest), str(view)) + assert sorted(str(p.relative_to(view)) for p in view.rglob("*")) == first diff --git a/tests/python/test_docker_publish_ref_freeze.py b/tests/python/test_docker_publish_ref_freeze.py index 0d634bc5c7..40a7649c29 100644 --- a/tests/python/test_docker_publish_ref_freeze.py +++ b/tests/python/test_docker_publish_ref_freeze.py @@ -97,6 +97,96 @@ def test_an_unreachable_remote_never_emits_a_mutable_ref(steps: dict, step_id: s assert res.returncode != 0 +# --- the llama.cpp prebuilt tag ---------------------------------------------- +# Same hole, same job, different resolver: the tag step is +# +# TAG="$(curl -fsSL -o /dev/null -w '%{url_effective}' .../releases/latest \ +# | sed -n 's#.*/releases/tag/##p')" +# echo "tag=${TAG:-latest}" >> "$GITHUB_OUTPUT" +# +# `bash -e` without pipefail takes the exit status of `sed`, so an unreachable +# github.com made the step emit `tag=latest`. That value is NOT a pin: both +# matrix legs pass it to docker/fetch_llama_prebuilt.py, whose main() re-resolves +# "latest" per build, and Dockerfile.studio re-resolves it a third time, so a +# release published mid-run can put two different llama.cpp bundles under one +# multi-arch manifest -- with `:latest` moved onto it, because the stable-tag +# gates key off the dispatch inputs, not off whether resolution worked. + + +@pytest.fixture(scope = "module") +def llama_step() -> str: + doc = yaml.safe_load(WORKFLOW.read_text(encoding = "utf-8")) + for step in doc["jobs"]["prepare"]["steps"]: + if step.get("id") == "llama": + return step["run"] + raise AssertionError("the llama tag resolver step is missing from the prepare job") + + +def test_an_unresolvable_llama_release_fails_the_step(llama_step: str, tmp_path: Path): + res = _run_llama_step(llama_step, tmp_path, curl_exit = 6) + assert res.returncode != 0, ( + "a failed /releases/latest lookup must fail the prepare job:\n" + f"stdout={res.stdout}\nstderr={res.stderr}" + ) + + +def test_an_unresolvable_llama_release_never_emits_a_mutable_tag(llama_step: str, tmp_path: Path): + res = _run_llama_step(llama_step, tmp_path, curl_exit = 6) + emitted = (tmp_path / "github_output").read_text(encoding = "utf-8") + assert "latest" not in emitted, ( + f"the step published {emitted.strip()!r}; every consumer resolves that " + "mutable tag again, so the two arch legs and Studio can bake different " + "llama.cpp versions under one manifest" + ) + assert res.returncode != 0 + + +def test_a_resolved_llama_release_is_forwarded_verbatim(llama_step: str, tmp_path: Path): + # The fix must not break the normal path. + res = _run_llama_step(llama_step, tmp_path, curl_exit = 0) + assert res.returncode == 0, f"stdout={res.stdout}\nstderr={res.stderr}" + assert (tmp_path / "github_output").read_text(encoding = "utf-8").strip() == ( + "tag=b10107-mix-1911198" + ) + + +def _run_llama_step(script: str, tmp_path: Path, *, curl_exit: int): + bin_dir = tmp_path / "bin" + bin_dir.mkdir(parents = True, exist_ok = True) + stub = bin_dir / "curl" + if curl_exit: + # How curl reports an unreachable github.com: nothing on stdout, non-zero. + stub.write_text( + "#!/usr/bin/env bash\n" + 'echo "curl: (6) Could not resolve host: github.com" >&2\n' + f"exit {curl_exit}\n", + encoding = "utf-8", + ) + else: + stub.write_text( + "#!/usr/bin/env bash\n" + "printf '%s' " + "'https://github.com/unslothai/llama.cpp/releases/tag/b10107-mix-1911198'\n", + encoding = "utf-8", + ) + stub.chmod(0o755) + out = tmp_path / "github_output" + out.write_text("", encoding = "utf-8") + env = dict(os.environ) + env["PATH"] = f"{bin_dir}{os.pathsep}" + env["PATH"] + env["GITHUB_OUTPUT"] = str(out) + env["INPUT_TAG"] = "" # the default (push / schedule) trigger + path = tmp_path / "llama_step.sh" + path.write_text(_expand(script), encoding = "utf-8") + return subprocess.run( + ["bash", "-e", str(path)], + capture_output = True, + text = True, + env = env, + timeout = 60, + ) + + def _expand(run: str) -> str: """Replace the `${{ ... }}` expressions with the empty string the default (push to main, no dispatch inputs) trigger produces."""