Make remote execution actually work: SLURM, SSH+GPU and HuggingFace Jobs, with evidence - #137
Merged
Merged
Conversation
Real execution has never succeeded on any cluster. Two defects in the two-venv handoff, both in the "scaffolding landed, seam did not" pattern: 1. VENV1 deserialized the function with dill, then re-serialized it for VENV2 with *stdlib pickle*. pickle serializes functions by qualified name, so any function defined in the caller's __main__ -- i.e. every realistic @cluster target -- failed with "Can't pickle <function f>: attribute lookup f on __main__ failed". The partially-written function_deserialized.pkl then gave VENV2 "EOFError: Ran out of input". Both venvs already install dill and cloudpickle, so the handoff now uses dill (falling back to cloudpickle, then pickle) in every direction, including result_raw.pkl and result.pkl. 2. Both conda envs were pinned to python=3.9 regardless of the local interpreter, while the function computed local_python_version and never used it. dill embeds CPython bytecode, which is not portable across minor versions, so a 3.12 caller got "RuntimeError: unknown opcode". Both envs now pin to the local version. Also stop the three stages from clobbering each other's error.pkl. Each stage writes error_<stage>.pkl unconditionally and only claims the shared error.pkl if no earlier stage did, so the caller sees the root cause instead of the last symptom in the cascade. Verified end-to-end on tensor01.dartmouth.edu (real SSH, no mocks): RESULT: {'n_squared': 49, 'host': 'tensor01.dartmouth.edu', 'py': '3.12.13', 'gpus': 'NVIDIA RTX A6000, 49140 MiB' x8} Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…e-local /tmp
The default remote_work_dir was "/tmp/clustrix". On SLURM, PBS and SGE the
compute node has its own /tmp, so the two-venv environment built on the login
node is simply absent when the job runs. The job script dies at its first
`source .../activate` with exit 127, before it can write job.out, job.err,
result.pkl or error.pkl -- an empty directory and no diagnostic.
Observed on discovery.dartmouth.edu:
sacct -j 9219484
9219484 FAILED 127:0 s01 /tmp/clustrix_smoke/job_1787020446
and all clustrix reported was:
Job 9219484 completion status unknown - no result or error files found
Job 9219484 directory is empty or doesn't exist
Three changes:
1. Default remote_work_dir is now "~/.clustrix/jobs", which is shared storage
on every cluster clustrix targets. The notebook widget defaults follow.
2. `~` is resolved against the remote $HOME before use. Shell commands expand
it themselves, but SFTP does not -- it would create a directory literally
named "~". ConnectionManager.resolve_remote_path() does this once per
connection and caches it; all four scheduler submit paths go through it.
3. When a job leaves no result and no error, ask sacct what happened instead
of returning "unknown". Terminal failure states are reported as failures,
and exit code 127 gets an explicit explanation of the node-local /tmp trap
that causes it.
Verified on tensor01.dartmouth.edu with remote_work_dir="~/.clustrix/jobs":
RESULT: {'n_squared': 49, 'host': 'tensor01.dartmouth.edu',
'py': '3.12.13', 'gpus': 'NVIDIA RTX A6000, 49140 MiB' x8}
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…trix Running the notebook widget suite appended an `integration_test` profile to the developer's own ~/.clustrix/clustrix.yml and dropped test.yml and test_all_configs.yml beside it. The widget's save path was `Path.home() / ".clustrix"` with no seam, so a test that chdir'd into a tmpdir still wrote to the real home directory. - New `clustrix.config.get_config_dir()` honours `CLUSTRIX_CONFIG_DIR` and falls back to ~/.clustrix. The six hardcoded `Path.home() / ".clustrix"` sites (config discovery, widget save, widget load, credential_manager, secure_credentials) now go through it. This is also what containers and CI images need, where $HOME is not writable or not persistent. - The `dev` extra gains ipywidgets, ipython and pytest-timeout. 44 of the 59 widget failures were an ImportError from missing optional dependencies that read like code defects. CI never saw them because it installs `.[dev,test,kubernetes,widget]` explicitly, while the documented `pip install -e ".[dev]"` does not pull the widget extra. Widget suite: 59 failing/erroring -> 20, and the run no longer touches the real home directory. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…lusters
Widget (clustrix/modern_notebook_widget.py)
-------------------------------------------
Rewrote the stylesheet against JupyterLab's own theme tokens. Every colour,
font and size now resolves through a --jp-* custom property that JupyterLab
already defines, with a literal fallback. That fixes three things at once:
* Dark mode works without a second stylesheet -- the tokens change with the
theme, so the widget follows it.
* The `@import url('https://fonts.googleapis.com/...Lexend+Deca')` is gone.
It failed on air-gapped login nodes and phoned out everywhere else; the
notebook's own UI font was always the right font.
* The widget reads as part of the notebook instead of a #333366 rectangle
pasted into it.
Also fixed, all visible in the before/after screenshots:
* "Test conn..." / "Test sub..." truncated because the buttons were 90px in
a 2-column grid slot. They are 130px in 3-column slots and now read
"Test connect" / "Test submit".
* The save/load buttons used the disk and folder emoji as their labels. The
notebook UI font has no glyph for either, so both rendered as empty boxes.
They are now text buttons. (FontAwesome `icon=` was tried first and renders
a fallback glyph on JupyterLab 4 / Notebook 7.)
* `self.styles` -- a 40-line dict of hardcoded colours and widths -- had no
readers anywhere in clustrix/ or tests/. Deleted.
* The "Applied!" confirmation hardcoded bootstrap green; it uses the theme's
success colour.
`import clustrix` no longer paints a widget into the notebook
-------------------------------------------------------------
`auto_display_on_import()` fired unconditionally, so importing the library
injected a configuration UI, and a second copy appeared next to any explicit
`%%remote` or `.display()` call -- which is how it usually got noticed. It is
now opt-in via CLUSTRIX_AUTO_WIDGET=1. Use `%%remote` or
`display_config_widget()` instead.
`%%clusterfy` is now `%%remote`
--------------------------------
Renamed, with `%%clusterfy` kept as a deprecated alias that warns and
delegates -- a magic rename that silently breaks every published notebook is
worse than carrying an alias. README, all four tutorials, both example
notebooks and the API docs updated.
Conda detection on clusters that only expose it in a login shell
-----------------------------------------------------------------
paramiko's exec_command starts a non-interactive, non-login shell, which never
sources the profile scripts where clusters put conda on PATH. Discovery keeps
conda at /dartfs-hpc/admin/local/bin/conda, so `conda --version` found nothing
and clustrix fell back to building two virtualenvs with pip over NFS -- minutes
of work that then hit venv_setup_timeout and failed. Detection now runs through
`bash -lc 'command -v conda'`, and conda's directory is prepended to PATH both
in the setup batch and in the generated job script, since the batch script runs
on a compute node under a non-login shell too.
Design canvas for the widget: notes/design/ (Main/Dark/Current artboards).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
… running locally HF Jobs runs a container, executes a command and exits, which is exactly clustrix's model. Unlike the Spaces provider it replaces in practice (Spaces are long-lived web apps, and that provider never satisfied the dispatch interface anyway), it needs no cluster reservation, no VPN and no institutional SSH credentials -- so it can serve as the integration-test substrate for everything else. New: clustrix/hf_jobs.py, reached via cluster_type="huggingface". There is no shared filesystem, so the function travels inside the job: the payload serialize_function() already produced is base64'd into an environment variable, a bootstrap in the container decodes it, runs the function, and prints the result; this side reads it back out of the job logs. Three details that are not incidental: * **The result is verified before it is deserialized.** Unpickling is code execution, and a result recovered from a log stream is not trustworthy on sight. Each job gets a fresh random key passed as an HF *secret*; the bootstrap emits an HMAC-SHA256 over the bytes it wrote, and this side refuses to deserialize anything whose tag fails a constant-time compare. (The same weakness in the SSH and scheduler paths is #121, still open.) * **The image defaults to the local Python minor version** (python:3.12-slim for a 3.12 caller). dill embeds CPython bytecode, which does not survive a version change -- the same trap that made the two-venv path fail with "unknown opcode". * **GPU flavors require hf_allow_gpu_flavors=True.** cpu-basic is the default and h100x8 is real money; a stray resource argument should not be able to rent one. Also fixed: @cluster ran hostless backends on the caller's machine -------------------------------------------------------------------- `_choose_execution_mode` returned "local" whenever config.cluster_host was falsy. HF Jobs reaches its compute over HTTP and has no host, so every huggingface job executed locally while reporting success. The first run of this backend returned platform: macOS-26.5.2-arm64, py 3.12.10 from a job that was supposed to be in a container. Cluster types that submit over an API are now listed in HOSTLESS_CLUSTER_TYPES and always take the remote path. Verified end-to-end against the contextlab namespace (real job, no mocks): Submitting HuggingFace Job (image=python:3.12-slim flavor=cpu-basic namespace=contextlab payload=836B) HuggingFace Job 6a83cfdbe55292eada79bdd0 submitted RESULT: {'factorial_ish': 4950, 'platform': 'Linux-6.12.95-124.187.amzn2023.x86_64-x86_64-with-glibc2.41', 'py': '3.12.14', 'machine': 'x86_64'} x86_64 Linux from an arm64 macOS caller, so the work demonstrably left this machine. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…s backend Every @cluster call against HF Jobs was submitting two containers and one of them always failed. Four agents went at the new backend; these are the findings that held up, each now covered by a regression test. **Every call billed a doomed extra job.** `auto_gpu_parallel` defaults True, so the remote path called `_detect_remote_gpu_count`, which submits a probe job. The probe function is defined inside `clustrix.decorator`, so dill ships it *by reference* and the container -- which has no clustrix -- died with `ModuleNotFoundError: No module named 'clustrix'`. The probe now short-circuits for hostless backends on a CPU flavor, where the answer follows from the flavor, and the swallowed `except Exception: return None` now logs: it made a real failure indistinguishable from "this cluster has no GPUs". **The GPU cost gate was a denylist, and the list was wrong.** It named `h100` and `h100x8`, which HuggingFace does not offer, while `a100x4`, `a100x8`, `h200*` and `rtx-pro-6000*` -- all real, all expensive -- were ungated. A denylist fails open on hardware added after it was written, which is backwards for something that bills per second. Gating is now a prefix test: anything not `cpu-*` needs `hf_allow_gpu_flavors=True`, so new accelerators are safe by default. **A function returning None always failed.** `_decode_between` used None for "no block found", so a None result fell through to "produced no clustrix result marker". It returns a `_MISSING` sentinel now. **A stray marker in the function's own stdout broke result retrieval.** The end-marker was checked before the collecting flag, so a function that printed `---CLUSTRIX-RESULT-END---` truncated the scan before the real block, and a printed begin-marker hijacked it. Both markers are now honoured only inside a block. **Truncated logs were reported as an attack.** `fetch_job_logs` returns what is available now, and a job that has just finished may still be flushing. A partial base64 tail produced "failed its integrity check... Refusing to deserialize" -- sending the reader hunting a forgery that never happened. Decoding errors are now separated from HMAC mismatches and say "truncated or interleaved"; the fetch is retried before giving up. **A non-ASCII tag crashed instead of failing.** `hmac.compare_digest` raises TypeError on a non-ASCII str, and the tag comes off a log stream. **cloudpickle payloads could not be loaded.** `serialize_function` falls back to cloudpickle when dill cannot handle a function, but the container installed only dill. It installs both and tries both. **A failed cancel reported success.** `executor_core.cancel_job` discarded the return value and deleted its tracking entry, so a job that was still running -- and still billing -- became invisible. It now raises and keeps tracking it. Verified after the fixes: one job submitted per call, no failures. Submitting HuggingFace Job (image=python:3.12-slim flavor=cpu-basic namespace=contextlab payload=836B) HuggingFace Job 6a83d2d6cd3824960fcbd007 submitted RESULT: {'factorial_ish': 4950, 'platform': 'Linux-6.12.95-124.187.amzn2023.x86_64-x86_64-with-glibc2.41', 'py': '3.12.14', 'machine': 'x86_64'} stage=COMPLETED New tests: tests/unit/test_hf_jobs.py (72), tests/unit/test_two_venv_execution.py (19). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…dicator The previous commit restyled the widget but left the 19-column grid in place, so it did not actually look like the design. This rebuilds the layout. Structure --------- The grid forced every control into one shared column rhythm. That is why the two test buttons had to squeeze into two columns and truncated to "Test conn..." / "Test sub...", and why every label was right-aligned into a gutter sized for the longest label in the widget. It is replaced by a header, four labelled sections, an actions row and an output panel, built from two helpers (`_section_heading`, `_field`) so a new field cannot drift. * Labels sit above their fields. That alone frees the full row width for real button text. * PROFILE / RESOURCES / CONNECTION / OUTPUT each get a small uppercase rule, so the resource fields can be found without reading every label. * The header states which cluster is configured and carries a status pill. * Sizes and colours match the design canvas exactly (26px controls, 3px radii, 10px/600/.08em section headings, 11px field labels, 14px body padding). Fields ------ The connection fields -- host, port, username, SSH key file, password, remote work directory, password-from-env-var -- now use the same label-above-field rows. The three hand-built HBoxes of right-aligned label + spacer + field they replaced had no other readers and are gone, along with `auth_fields_container` and its hardcoded #f8f9fa box. Status indicator ---------------- `set_status(state, text)` drives the header pill from real outcomes: busy while a test runs, ok on success, error on failure, idle before anything has been tried. Wiring it up exposed a bug -- `_on_test_connect` printed "Connection test completed successfully" even when authentication had failed two lines earlier. It now reports the actual result. Also fixed ---------- * Two "Advanced settings" buttons appeared. `_update_ui_for_cluster_type` rebuilt `remote_section.children` on every cluster-type change, splicing in a second copy and dropping every field after the third. It now only toggles visibility. * Fixed-width fields were being shrunk by the flex row -- a 66px "Save" button rendered 40px wide as "S...". They now carry `flex: 0 0 <width>`. * The output area was `display: none` with a hardcoded light-grey background, so it was invisible until first use and unreadable in the dark theme. It is visible from the start, framed, and says what will appear in it. * Only Apply is a primary button now; the tests, Save/Load and Advanced settings are secondary, so the eye lands on the one control that changes the session's configuration. Widget suite is unchanged at 20 pre-existing failures; unit tests 182 pass. Before/after screenshots: docs/evidence/widget/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…st that could not fail Two agents went at the widget -- one on design fidelity, one walking through real usage for every cluster type. These are the functional findings. **Apply never applied anything.** `_on_apply_config` saved to a profile, printed "Applied configuration", and stopped. The source said so: "Note: This integration point would need to be connected to the main config system". It never was, so every setting typed into the widget was invisible to @cluster, which went on using whatever the global config held. Setting host=login.hpc.edu and pressing Apply left `get_config().cluster_host` at None. It now calls `configure()`, and afterwards the global config carries the host, username, cores and work directory that were on screen. **"Test job submission" could not fail.** It printed Job 1/4: Basic Python execution... OK ... All 4 test jobs executed and cleaned up properly with no executor call anywhere. It reported success with an empty host, zero cores and a memory string of "banana". It now serializes a small function, submits it through the same path a real @cluster call takes, waits, and prints what came back -- or the failure. That exposed a second bug. The payload has to survive being shipped to a worker with no clustrix installed, and dill pickles a function belonging to an importable module BY REFERENCE -- 67 bytes that merely name `clustrix.modern_notebook_widget`. The first real run died with `ModuleNotFoundError: No module named 'clustrix'` inside the container. New `utils.make_portable_function()` compiles such a helper from source into a fresh namespace, giving it `__module__ = None` and no importable reference, so dill serializes it by value (~530 bytes) and the worker needs nothing. Verified against a real HuggingFace Job: Job ID: 6a83de35e55292eada79be3f Ran on: j-contextlab-6a83de35e55292eada79be3f-pg802qod-608d7-drq82 Python: 3.12.14 Returned: 5050 **Seven of the eight offered profiles did not exist.** The dropdown was eight hardcoded names unioned with the real ones, of which the ProfileManager held exactly one -- so the first click a user made usually failed with "Profile 'SLURM cluster' does not exist". It now offers the profiles that exist, sorted (the `set()` union also made the order change between runs). **HuggingFace Jobs was unreachable from the UI.** cluster_type="huggingface" exists but was not in the dropdown, and none of hf_namespace / hf_flavor / hf_token / hf_allow_gpu_flavors had fields. There is now a HuggingFace Jobs section, shown only for that cluster type, and the values reach ClusterConfig. **"Remote work directory" was write-only.** Typing /scratch/alice/clustrix into it still ran jobs in ~/.clustrix/jobs. Likewise the env-var password field set `password_env_var` but never `use_env_password`, which is what every reader gates on -- so it did nothing at all. Widget suite unchanged at 20 pre-existing failures; unit tests 223 pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
A second agent measured the rendered widget against the artboard in notes/design/ and found the places it still diverged. All of these were real. **The Advanced panel was an unstyled relic.** It was three 19-column GridBoxes with spacer columns and side labels, carrying a hardcoded `border="1px solid #dee2e6"` -- the only hex literal still reaching the DOM, and a near-white outline on a dark background. Its labels wrapped to two lines, and its fixed columns overflowed the card by ~380px, putting two horizontal scrollbars inside it. It now uses the same label-above-field rows as everything else, under an ADVANCED heading, and its "Replicate local env" checkbox has a caption again (our own labels sit above the control, so ipywidgets' description slot is hidden). **ipywidgets' own labels stayed black in the dark theme.** They are coloured from `--jp-widgets-label-color`, which the widget never re-pointed at a token. Anything still carrying a `description=` was black-on-#212121. **Status-pill tints were frozen light-theme washes.** `rgba(56,142,60,.14)` does not follow the theme; the ok pill computed ≈3.5:1 against its own text in dark mode, below AA. They are now derived from the token with `color-mix`. **Button labels were the truncated ones the design exists to fix.** "Test connect"/"Test submit" are now "Test connection"/"Test job submission" -- the row has the width for them since labels moved above the fields. **Columns were fixed pixels, not proportions.** The artboard specifies 2fr/1fr/1fr/1.2fr for Resources and 2fr/1fr/1.4fr for Connection. With fixed widths, narrowing the card starved the stretchy field to 9-16px while the fixed ones kept their size; "Apply" clipped to "A…". `_field` now takes a `flex` ratio, so every column shrinks together. **+/- rendered as "+…".** The 11px side padding that suits a worded button leaves a 30px one with 8px of room. Icon buttons are 26px square with no padding, and secondary rather than primary, as in the design. **The caret icon was a hamburger.** `button.icon = "caret-down"` needs FontAwesome, which JupyterLab 4 / Notebook 7 do not reliably provide. Also: nothing inside the card scrolls horizontally any more -- ipywidgets gives every box a default min-width, which made rows wider than their container. Screenshots (light, dark, advanced): docs/evidence/widget/. Widget suite unchanged at 20 pre-existing failures; unit tests 223 pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…r itself
`ClusterFilesystem` asked whether the local hostname and the configured
cluster host "looked related" -- substring matches in either direction, plus
a same-domain and a same-institution-domain test -- and if so switched every
filesystem operation to local. The stated reason is that HPC clusters share a
filesystem between head and compute nodes, which is true; the test for it was
not.
A laptop connected to the Dartmouth VPN gets a hostname like
vpn-two-factor-general-229-128-226.dartmouth.edu. Against
cluster_host="discovery.dartmouth.edu" that satisfied
`_same_institution_domain`, so clustrix decided the laptop *was* discovery,
looked for the job's result file on the laptop, found an empty directory, and
reported:
Job 9219833 completion status unknown - no result or error files found
Job 9219833 directory is empty or doesn't exist
while the job ran perfectly well on the cluster.
Whether local filesystem operations are usable is a question about the
filesystem, not about names, and both halves are checkable:
* this host IS the target host (exact hostname or FQDN, or the target's
short name matching this FQDN's first label), and
* the configured remote_work_dir is visible here -- which is precisely what
"shared filesystem" has to mean for the optimisation to be correct.
Both must hold. A hostname match without the directory now logs and keeps
remote operations rather than silently rerouting I/O to the wrong machine.
`_same_domain` and `_same_institution_domain` existed only to feed that guess
and had no other caller in clustrix/ or tests/; they are removed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
Conda environments were named after the job directory -- `clustrix_venv1_job_1787025008` -- so every single @cluster call built two brand-new environments and left them behind. On discovery's shared filesystem that is roughly ten minutes each: twenty minutes before any user code runs, repeated for every job, accumulating environments nothing ever removes. Five had piled up from one afternoon's testing, and runs were hitting venv_setup_timeout before they got anywhere. Environments are now named after what is IN them -- the Python version and the sorted requirement set, hashed -- so `clustrix_venv1_py312_fab7c2f690ab` is built once and every later job with the same requirements reuses it. A job whose requirements differ gets its own rather than silently inheriting the wrong ones, and setup short-circuits entirely when both already exist. Also: `create_job_script` now fills in any resource key the caller omitted from the configured defaults. The three scheduler generators index job_config["cores"], ["memory"] and ["time"] directly, so the GPU-detection probe -- which passes only cores and memory -- raised a bare `KeyError: 'time'` that got swallowed into "Could not detect remote GPU count: 'time'". With both fixes, SLURM execution on discovery.dartmouth.edu works end to end: Conda available on remote system (/optnfs/common/miniconda3/etc/profile.d/conda.sh) Reusing existing conda environments (py312_fab7c2f690ab) RESULT: {'sum': 499500, 'host': 's07.hpcc.dartmouth.edu', 'py': '3.12.13', 'slurm_job_id': '9219868', 'nodelist': 's07', 'cpus': '1'} A real sbatch job, on a real compute node, returning a real result. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
`scripts/collect_execution_evidence.py` submits one identical function to every
backend clustrix claims to support and prints what comes back. It exists
because remote execution was broken for a long time without anyone noticing:
the suite mocked the parts that mattered, and "real-world testing" meant hosts
CI could not reach, so nobody could check a claim without rebuilding the setup
from scratch.
Nothing in it is mocked. A target that cannot be reached is reported as
skipped, never as passing, and a result whose hostname matches the caller's is
treated as a failure -- the point is that the work left this machine.
Run from an arm64 macOS laptop:
slurm PASSED s07.hpcc.dartmouth.edu python 3.12.13 62.1s
gpu PASSED tensor01.dartmouth.edu python 3.12.13 65.5s
hf PASSED j-contextlab-6a83e58c... python 3.12.14 9.7s
SLURM job 9219882 on compute node s07; tensor01 reporting its 8 RTX A6000s;
a HuggingFace container on x86_64 Linux. Same function, same result (499500),
three very different machines, none of them the caller.
Full transcript in docs/evidence/execution-evidence.txt; widget screenshots in
docs/evidence/widget/.
The draft release notes say what works, why it did not before, and -- at equal
length -- what is still broken: #121's unverified pickle on the SSH and
scheduler paths, PBS and SGE untested, the cloud providers still broken,
Kubernetes unconfigurable from the widget, no input validation, ~5,100 lines
with no importers.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…ors instead of hanging ClusterFilesystem opened SSH connections with no timeout at all, so paramiko fell back to the OS default -- minutes on a host that does not answer. That was invisible while the cluster-detection bug was rerouting these calls to the local filesystem; with detection fixed, the first test that reaches a real host without credentials stops the entire suite instead of failing. hostname, auth and banner timeouts now come from a new `ssh_connect_timeout` (default 30s), so a filesystem call that cannot connect says so quickly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…directory
Running the suite left files behind in two places it had no business touching:
a stray `test_config.yml` in the repository root, and `test.yml`,
`test_all_configs.yml` plus an appended `integration_test` profile inside the
developer's real `~/.clustrix/clustrix.yml`.
Two causes.
`ProfileManager.save_to_file("clustrix.yml")` resolves a bare filename against
the current working directory, so the widget's Save button wrote wherever the
notebook happened to be started -- the repository root, under pytest. Bare
filenames are now anchored to the clustrix config directory; a path containing
a separator, or an absolute one, is still respected as written.
And tests that chdir into a tmpdir did not help, because the save path is
derived from the config directory rather than the working directory. A
session-scoped autouse fixture now points CLUSTRIX_CONFIG_DIR at a throwaway
for the whole run, so no test can reach the real one whether or not it
remembers to.
Widget suite unchanged at 20 pre-existing failures; unit tests 250 pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
… scheduler paths `_wait_for_scheduler_result` downloaded result.pkl and handed it straight to `pickle.load`. Unpickling executes arbitrary code, so that made every remote result a remote-to-local code execution path -- the last P0 in #108 that was a live exploit rather than hygiene. The HuggingFace backend already verified its results; the SSH and scheduler paths now use the same scheme. At submission the job directory is created 0700 with a 32-byte random key inside it at 0600. The job script exports the key from that file rather than having it baked into job.sh, which is world-readable on some shared filesystems. Stage 3 writes result.pkl and an HMAC-SHA256 over exactly the bytes it wrote. The caller checks the tag with a constant-time compare before `pickle.loads`, and refuses an absent, truncated or mismatched signature. This bounds the trust to whoever can already read the job directory. It is deliberately not claimed as a defence against a wholly compromised remote host, which runs the function anyway -- but it does stop an unrelated user on a shared filesystem, a stale file from an earlier run, or a truncated transfer from being handed to the unpickler. A job with no recorded key (adopted from another process, or submitted before this existed) logs that it was loaded unverified rather than failing or staying quiet. Verified on tensor01.dartmouth.edu. A normal run: job dir : /home/f002d6b/.clustrix/sigcheck/job_1787030114 dir perms : 700 key file perms: 600 key length : 64 VERIFIES : True and with result.pkl overwritten in place, leaving the original signature -- exactly what someone with write access to the job directory would do: result.pkl overwritten with an unsigned payload REFUSED: Job ssh_1787030156 result failed its integrity check. Refusing to deserialize it. 12 new tests in tests/unit/test_result_verification.py cover accept, tamper, wrong key, absent, whitespace-only and truncated signatures, plus the job script actually emitting a tag over the bytes it wrote. Full suite: 176 failed / 1313 passed / 36 errors, against 211 / 1194 / 72 at the start of this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
**The widget accepted anything.** `cores=0`, `memory="banana"`, `time="soon"`,
`port=99999` and an empty host for a remote cluster all went straight into
ClusterConfig, and Apply reported success. Nothing complained until the job
failed on the cluster minutes later with an error that named none of it.
`_validate_widget_values()` now reports every problem at once -- not just the
first -- and Apply and Test job submission refuse to proceed:
Cannot apply this configuration:
- CPUs must be positive (or -1 for all); got 0
- Memory 'banana' is not a size; expected something like '16GB', '512Mi' or '8G'
- Walltime 'soon' is not a duration; expected HH:MM:SS (or D-HH:MM:SS)
- A host is required for a slurm cluster
- A username is required for a slurm cluster
- Port must be between 1 and 65535; got 99999
`-1` cores keeps its meaning ("use every core"), and `local` still needs no
host.
**"16GB" is not a Kubernetes quantity.** The pod manifest carried
`job_config["memory"]` verbatim, so clustrix's own `default_memory` produced a
manifest the API server rejects -- Kubernetes wants `16G` or `16Gi`. SLURM
documents `--mem` units as `[K|M|G|T]`, PBS spells it `16gb`, SGE `16G`. New
`utils.normalize_memory(value, target)` renders a size the way each scheduler
expects:
16GB -> k8s=16Gi slurm=16G pbs=16gb sge=16G
512Mi -> k8s=512Mi slurm=512M pbs=512mb sge=512M
16 -> k8s=16Gi slurm=16G pbs=16gb sge=16G
An unparseable value is passed through with a warning rather than guessed at,
so the scheduler's own error names the real problem.
Three assertions in tests/test_utils.py expected the un-normalized spelling;
they now expect the correct one. 32 new tests in
tests/unit/test_widget_validation.py.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
PBS did not work at all. Its script ended with
cd {job_dir}
source venv/bin/activate
python execute_function.py
and `execute_function.py` appears exactly once in the whole codebase -- on
that line. Nothing in clustrix has ever created it, so every PBS job died
immediately with "can't open file".
SGE carried its own copy of the older single-venv script, which meant it
silently missed everything fixed on the SLURM one: the two-venv path, the
dill handoff, conda sourcing, the per-stage error files and result signing.
Three copies of the same logic drifting apart is how PBS came to run a
nonexistent file without anyone noticing.
The execution body is now one function, `job_execution_lines()`, and all three
schedulers call it. Only the directive header differs, which a test asserts
directly by comparing the bodies.
slurm two_venv=True conda_sourced=True signs_result=True
pbs two_venv=True conda_sourced=True signs_result=True
sge two_venv=True conda_sourced=True signs_result=True
Neither PBS nor SGE has been run against real hardware -- there is none to
hand -- so this fixes a script that could not possibly have worked rather than
claiming the backends are verified. That distinction stays in the release
notes.
16 new tests in tests/unit/test_two_venv_execution.py.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
`k8s_namespace`, `k8s_image`, `k8s_service_account` and `k8s_pull_policy` all exist in ClusterConfig, and none of them had a field. Selecting "kubernetes" in the widget showed nothing at all -- the Connection section is for SSH hosts and Kubernetes has none -- so only the shipped defaults were ever reachable from the UI. There is now a Kubernetes section, shown for that cluster type and hidden otherwise, and the values reach ClusterConfig. A test asserts that for every one of the seven cluster types exactly the right backend section is visible and the others are not, which is the invariant that used to be broken in both directions (Kubernetes showed nothing; HuggingFace was not selectable at all). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
The audit behind #108 reported roughly 5,100 lines (13% of the package) with zero importers. Measuring it three ways does not support that, and deleting five thousand lines to match a number nobody can reproduce would be reckless. **By module.** Every module under clustrix/ is imported by something. Counting only imports from within clustrix/ -- so a module that only tests reach counts as dead product code -- leaves four files totalling 1,657 lines, and three of them are not dead: clustrix/cli.py 275 a declared entry point (clustrix = clustrix.cli:cli) clustrix/secure_credentials.py 150 the 1Password integration, used by the real-world credential manager clustrix/enhanced_notebook_widget.py 448 superseded by modern_notebook_widget, but still used by one test clustrix/pricing_clients/validation_alerts.py 784 used by one validation script **By symbol.** A naive scan for public definitions never referenced outside their own file reports ~2,000 lines -- and its top hit is `setup_two_venv_environment`, which every remote job in this branch went through. The heuristic cannot see same-file callers, so its output is noise. **With a real tool.** `vulture --min-confidence 90` finds thirteen items across the whole package: two genuinely unused imports, and eleven unused *variables* that are almost all `__exit__(exc_type, exc_val, exc_tb)` parameters the context-manager protocol requires. The two real ones are removed here (`AzureError` in azure_provisioner, `HfFolder` in huggingface_provisioner). That is the whole verifiable finding. `enhanced_notebook_widget.py` and `validation_alerts.py` are plausible deletions -- 1,232 lines between them -- but each has exactly one consumer, so removing them means removing those too. That is a judgement call about scope, not a cleanup, and it belongs in its own change rather than being smuggled in behind a number that turned out to be wrong. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
The script clustrix uploads to a cloud instance did this:
func = func_data['func']
args = func_data.get('args', ())
kwargs = func_data.get('kwargs', {})
`serialize_function()` has never produced a 'func' key. It produces
`{"function": <dill bytes>, "args": <pickle bytes>, "kwargs": <pickle bytes>}`
-- the function as bytes, not as a live object. So every cloud job died with
`KeyError: 'func'` before doing anything, and the two `.get()` calls would have
handed raw bytes to `func(*args, **kwargs)` even if the first line had
survived. That is proof the path had never once run.
The script now unpacks the payload every other backend receives: dill (falling
back to cloudpickle) for the function, pickle for the arguments.
This makes the cloud script correct; it does not make the cloud backends
verified. Reaching this code needs AWS, Azure, GCP or Lambda credentials and
provisioned instances, none of which were exercised here, and #119's other
findings -- the provider interface mismatch and the placeholder hostnames --
are untouched. The release notes still list the cloud backends as broken.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
ipywidgets checkboxes carry their own padding and a minimum width, so `width="100%"` overflows the container and leaves a horizontal scrollbar under the row -- visible under "Allow paid GPU flavors" in the HuggingFace section. They size to their content now. Adds docs/evidence/widget/05-huggingface-jobs.jpg, showing the HuggingFace Jobs section that the widget gained in this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
Adds the result-signing work (#121), PBS/SGE unification, memory quantities, the cloud KeyError, widget validation, the Kubernetes section and the test isolation. Records that the '~5,100 lines of dead code' figure does not reproduce, and narrows 'what is still broken' to what is actually still broken -- the cloud backends unverified, PBS/SGE never run on real hardware, Kubernetes execution unverified, and the suite's remaining failures. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
**requires-python = ">=3.8" was never true.** Four hard dependencies require
>=3.10 at their current versions and two more require >=3.9:
paramiko >=3.9
dill >=3.9
click >=3.10
requests >=3.10
huggingface_hub >=3.10
python-dotenv >=3.10
So `pip install clustrix` on 3.8 or 3.9 either failed or silently pinned
years-old releases. 3.8 has also been end-of-life since October 2024, and
mypy refuses the setting outright -- every run printed "python_version: Python
3.8 is not supported (must be 3.10 or higher)" before doing anything. The floor
is now 3.10, with the classifiers, black target and mypy setting to match, and
the CI matrices no longer claim to test versions that cannot install.
**fast_ci.yml has never run a single job**, on this branch or on master,
because it is not valid YAML. Three steps embed a Python program as
run: |
python -c "
from clustrix import cluster, configure
...
with the program's body at column 0. That ends the block scalar, so YAML then
tries to read `from clustrix import cluster, configure` as a mapping key and
the whole file fails to parse -- which GitHub reports as zero jobs rather than
as an error, so nobody noticed. The bodies are indented into their block
scalars now; the file parses, exposes its five jobs, and the three embedded
programs still compile exactly as the shell will receive them.
Whether those jobs pass is a separate question -- they have never been allowed
to run, so this makes a dead gate live rather than a passing one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…get tests
An agent asked to fix the 20 failing widget tests fixed 14 as test-side API
drift and, correctly, refused to touch the other six -- reporting them as
defects in shipped code instead. It was right about all of them. A second
agent auditing the docs found four more. Both sets are fixed here.
**Loading a config with an unlisted region broke the widget.** Ten dropdown
assignments in `_load_config_to_widgets` were bare `field.value = ...`, so any
saved value outside the hardcoded ten-item option list raised
TraitError: Invalid selection: value not found
AWS alone has far more than ten regions, so an ordinary config file was enough.
The saved configuration is authoritative -- a list baked into the UI should not
veto it -- so an unrecognised value is now added to the options.
**Renaming a configuration silently discarded every edit.** Rebuilding the
dropdown's `options` makes ipywidgets re-fire the selection observer, which
called `_load_config_to_widgets` and overwrote whatever had been typed. Setting
memory to 99GB and a host, then renaming, reverted both *and* left the widget
on a different profile. The rebuild is guarded so the observer only responds to
a selection the user actually made.
**IPv6 cluster addresses were rejected.** `validate_ip_address` was hand-rolled
IPv4-only parsing. It uses `ipaddress` now, so `::1` and
`2001:db8::8a2e:370:7334` validate.
**`load_config_from_file` did not return a dict.** It is annotated
`-> Dict[str, Any]` but returned whatever `yaml.safe_load` produced, so a
`.txt` file of prose yielded a `str` and every caller had to guess.
**Following the error message did not work.** `hf_jobs` told you to "export
HF_TOKEN" while reading only the config field. It now honours `HF_TOKEN` and
the token `hf auth login` writes, and the message names all three.
**Two more from the docs audit**: `pip install "clustrix[huggingface]"` names
an extra that does not exist (`huggingface_hub` is a core dependency), and that
dependency's floor was `>=0.16.0` while the whole backend is built on
`run_job`/`inspect_job`/`fetch_job_logs`, which arrived much later. Raised to
`>=0.34.0`. Also a stale comment claiming the widget displays on import, and
`cleanup_remote_files` in CLAUDE.md -- the field is `cleanup_on_success`.
The HF auth tests now isolate HOME and HF_TOKEN; they were passing only because
the machine running them happened to be logged in to HuggingFace. Re-verified
against a real container afterwards:
hf PASSED j-contextlab-6a83f8a4cd3824960fcbd684 python 3.12.14 10.5s
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
The docs described a version of clustrix that no longer exists in several places and never existed in others. Every code example in README.md and the Sphinx tree was run, not eyeballed. * **Real screenshots of the widget**, light, dark and with the Advanced panel open, in README.md, index.rst and the notebook_magic API page. They replace six fabricated PNGs under `_static/img/screenshots/` that showed a UI nobody ever shipped. * **`%%remote`** throughout, with `%%clusterfy` documented as the deprecated alias it now is. * **`import clustrix` no longer displays the widget**, so the pages that said it does are corrected, with `CLUSTRIX_AUTO_WIDGET=1` documented for anyone who wants the old behaviour. * **`remote_work_dir` defaults to `~/.clustrix/jobs`**, with a sentence on why `/tmp` was wrong rather than a silent find-and-replace. * **The HuggingFace Jobs backend** is documented: `cluster_type="huggingface"`, the `hf_*` settings, the GPU-flavor opt-in and the result verification. * **`CLUSTRIX_CONFIG_DIR`, `ssh_connect_timeout` and `venv_setup_timeout`** are documented for the first time. * **The coverage badge is gone.** It claimed 10%; no honest current number has been established, and inventing one would repeat the mistake the audit behind #108 found four times over. * **What is broken is stated plainly** -- cloud backends unverified, PBS and SGE never run on real hardware, Kubernetes execution untested -- instead of being left out. Verified by running: the decorator example, `configure()` for all five cluster types, both tutorial YAML files, all nine filesystem helpers, cost monitoring, `ClusterExecutor.get_job_status`, the installation verification snippet, `LocalExecutor.execute_loop_parallel`, `LoopAnalyzer`, the file_packaging metadata keys and the CLI. Sphinx builds; its 61 warnings are pre-existing autodoc duplicates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…tions **`module_loads` and `pre_execution_commands` had no widget at all.** Both are real ClusterConfig fields that `utils.py` emits into every job script, and the #80 refactor dropped their textareas -- so saving a profile through the widget silently erased a user's `module load` lines. On an HPC cluster that is the difference between a job that runs and one that cannot find its compiler. Both fields are back, and they round-trip: module_loads : ['python/3.12', 'cuda/12.1'] pre_execution_commands: ['source /opt/setup.sh'] **The remaining test failures were API drift.** The widget names every field `*_field`; two tests still used the pre-refactor `widget.module_loads`, and one of them stubbed it with a MagicMock because there was nothing real to stub. **One test was asserting a security bug.** It expected `validate_ip_address("192.168.001.001")` to be True. CPython stopped accepting leading zeros in 3.9.5 (bpo-36384) precisely because "010" is octal to some resolvers and decimal to others, which is an SSRF primitive. The old hand-rolled `int()` parsing accepted them; `ipaddress` does not, and it is right. The expectation is inverted with that reasoning recorded. The widget suite is now fully green: 365 passed, 1 skipped, from 20 failures at the start of this branch. black, flake8 and mypy are clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
`_prepare_job_dir` wrote the key with
umask 077 && printf '%s' <key> > .../.clustrix_result_key
which puts a 64-character secret into the remote process's command line. On a
default Linux `/proc` any user on that login node can read another user's
command line out of `ps` -- handing the key to exactly the people the 0700
directory exists to exclude. It would also land in the shell's history on hosts
that record non-interactive commands.
The key is written over SFTP instead, and `create_remote_file` gained a `mode`
parameter so the permissions are set before any bytes are written rather than
after -- a secret should not exist on disk world-readable even briefly.
Re-verified on tensor01 after the change:
dir perms : 700
key perms : 600
key length: 64
VERIFIES : True
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
**Single-venv jobs could never return a result (critical, introduced by me).**
Result signing was added to the two-venv branch only, while the submitter
recorded a signing key unconditionally -- so any job that fell back to the
single-venv path (use_two_venv=False, or any two-venv setup failure or
timeout) produced a result the caller then refused as unsigned. A degraded but
working path became a hard failure. All four schedulers now sign on both
branches, via one shared `result_signing_lines()` so a branch cannot forget
again.
Testing that on tensor01 uncovered three more, each hidden behind the last:
* **The SSH fallback never built its venv.** Both of its fallback branches set
`venv_info = None` and stopped; the SLURM path has always called
`setup_remote_environment`. The generated script therefore activated a
virtualenv nobody had created: "venv/bin/activate: No such file or
directory".
* **`python -m venv` assumed a `python` that does not exist.** Python 3
installs ship `python3`; `python` is only present where somebody added a
compatibility symlink. `resolve_remote_python` now probes.
* **The dill install was best-effort.** `|| echo 'Package installation failed,
continuing...'` swallowed the failure of the one package the job script
cannot work without, and the job died twenty lines into a remote traceback
with "'NoneType' object is not callable", naming neither.
Which finally exposed the real limit, which is not papered over: **the
single-venv path cannot bridge Python versions at all.** dill embeds CPython
bytecode, so a 3.12 payload on tensor01's `python3 (3.6.8)` gives
"code() takes at most 15 arguments (20 given)". It now fails at submit time
with something actionable:
No python3.12 on the remote host, and dill payloads cannot cross Python
minor versions. Found: python3 (3.6.8). Either install python3.12 there,
set python_executable to a matching interpreter, or leave use_two_venv
enabled so clustrix can build a conda environment at the right version.
**Silent chmod failure -> key theft -> code execution on the submitting
machine.** `execute_remote_command` never checked exit status, and job
directories were `job_<unix_seconds>` -- fully predictable. On a
world-writable remote_work_dir an attacker could pre-create the directory;
`mkdir -p` succeeds on it, the unchecked `chmod 700` fails unnoticed, the
signing key lands somewhere they can read, and they forge both result.pkl and
its HMAC. Directories are now created exclusively with `mkdir -m 700` and the
status checked, and their names carry four random bytes -- which also fixes
two jobs submitted in the same second overwriting each other's key.
**`1.5GB` produced `--mem=1.5G`**, which SLURM and PBS reject. Fractional
sizes round up (down would get the job killed).
Shell interpolations of config-derived paths are now `shlex.quote`d.
The agent also attacked and could not break: `make_portable_function` (its
source is a module constant), the HMAC's coverage of the downloaded bytes,
`compare_digest`'s operands, cleanup ordering, and the dropdown reentrancy
guard.
All three backends re-verified afterwards:
slurm PASSED t08.hpcc.dartmouth.edu python 3.12.13
gpu PASSED tensor01.dartmouth.edu python 3.12.13
hf PASSED j-contextlab-6a8401ce... python 3.12.14
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…d a real bug in my own evidence
An agent auditing every factual claim on this branch found that several had
gone stale because this branch fixed the thing they described, one number was
wrong in three places, and -- most importantly -- the committed evidence
transcript opened with three lines I had quoted around.
**The evidence file began with an error I never showed.**
Error creating flattened function: unexpected character after line
continuation character (<string>, line 4)
Every excerpt in the PR and release notes started below it. Quoting selectively
around a failure is exactly what this branch exists to stop, so: the cause is
`function_flattening.py:487`, `return "\\n".join(code_parts)` -- in Python
source that is a literal backslash and an `n`, so every generated statement was
joined onto one line and could not compile. Fixed to a newline.
That reveals the next one, which is now visible in the committed transcript
rather than trimmed out of it: the flattener emits a parameterless *script*, so
a function that uses its own arguments produces `name 'n' is not defined`.
Flattening then falls back to ordinary serialization and the job runs, which is
why all three backends pass either way. I tried to guard against attempting it
at all and made things worse -- the caller's fallback path is itself fragile --
so the guard is reverted and the limitation is recorded instead of half-fixed.
**The Python floor was corrected in one file of three.** `setup.py` still said
`>=3.8` with 3.8/3.9 classifiers, and the README badge still said 3.8+, while
pyproject said 3.10. All three agree now.
**Claims this branch had already falsified.** README, index.rst and the
Kubernetes tutorial still said the widget "offers no Kubernetes fields" (it has
had a Kubernetes section since 2ceb622) and that `HF_TOKEN` "is not read
automatically" (it has been since 51d5961). `default_memory` is `8GB`, not
`16GB`.
**Test artefacts stopped being written into the source tree.** The real-world
conftest created `tests/real_world/screenshots/` and `temp/` inside the
repository, so every suite run left the working copy dirty and the checked-in
copies drifted with whoever ran last. They go to a temp directory now
(`CLUSTRIX_SCREENSHOT_DIR` to keep them), and the paths are gitignored.
Evidence regenerated after all of the above; all three backends pass:
slurm PASSED q01.hpcc.dartmouth.edu python 3.12.13 63.9s
gpu PASSED tensor01.dartmouth.edu python 3.12.13 12.2s
hf PASSED j-contextlab-6a8405c2... python 3.12.14 10.1s
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
**Ed25519 is not quantum-resistant.** `ssh_setup.rst` said so twice. It is an
elliptic-curve signature scheme, which is exactly what Shor's algorithm breaks
and the whole reason post-quantum migration exists. Described accurately now.
**"15-30 minute manual process into a 15-second automated experience"** appears
in the README, ssh_setup.rst and the SSH tutorial notebook, with no measurement
behind either number. Replaced with what the feature actually does: generate,
deploy and configure a key in one call instead of three manual steps.
**The documented `cluster_find` -> `cluster_stat` chain does not work.** `find`
and `glob` return paths relative to *the directory they searched*, so passing a
result straight to `stat` raises `FileNotFoundError` unless you searched ".".
Eight documentation sites taught the broken form. The contract is now stated
prominently with a working example, verified:
stat data/a.csv -> 1 bytes
The return contract itself is deliberate and eight tests depend on it, so it is
documented rather than changed this late in the branch.
**Suite figures updated** to the current run: 159 failed / 1402 passed /
36 errors, against 211 / 1194 / 72 at the start of the branch. The previous
figure was ten commits stale.
The PR body is updated too: it still listed #121, the Kubernetes widget fields,
widget input validation and PBS/SGE as broken, all of which this branch fixed,
and quoted three test counts that had grown since (250 -> 255, 72 -> 75,
19 -> 35).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…t stale claims
**`%clustrix` never existed.** Five tests have expected a line magic with
`status`, `config`, `config <profile>` and `load <file>` since the notebook
magic was written, and it was never implemented -- every call raised
"Line magic function `%clustrix` not found". It exists now:
%clustrix status what @cluster will do right now
%clustrix config <profile> make a saved profile active
%clustrix load <file> load configuration from a file
**Every magic was a no-op whenever IPython was absent.** `notebook_magic_mocks`
only handled `@cell_magic("name")`; applied bare -- which is how clustrix uses
it -- `cell_magic` returned its own inner `decorator`, so calling the magic
invoked that with (self, line, cell), fell through to the catch-all, and
returned `lambda: None` without running the method. That is precisely the
situation the module exists for. Both calling conventions work now, and
`line_magic` is a real stand-in rather than an alias for the cell one.
**`%load_ext clustrix` now leaves the notebook able to use clustrix**, not
merely to type its magics: `cluster`, `configure` and `get_config` go into the
user namespace.
**An unknown setting in a config file named the internals, not the file.**
`ClusterConfig.__init__() got an unexpected keyword argument
'cleanup_remote_files'` becomes:
<path> contains unknown setting(s): cleanup_remote_files
(did you mean cleanup_on_success?)
I also briefly made `load_config_from_file` raise on a malformed file. Four
tests pin the tolerant contract deliberately -- it is the widget's Load path,
where an exception escapes into the notebook cell rather than the widget's
output area -- so that is reverted and documented, and the one test that wanted
an error now uses `load_config`, which raises and is the right tool for it.
**Remaining false claims** from the accuracy audit: README described
`tests/integration/` as "run in CI" when it provisions real billable AWS
resources and refuses to run without `CLUSTRIX_ALLOW_BILLABLE=1`, and told
developers to include it in their test command; "roughly a quarter of test
modules use unittest.mock" is 42 of 197 (21%).
Notebook-magic and widget suites: 381 passed, 2 skipped, 0 failed.
All three backends re-verified:
slurm PASSED s12.hpcc.dartmouth.edu python 3.12.13
gpu PASSED tensor01.dartmouth.edu python 3.12.13
hf PASSED j-contextlab-6a84152c... python 3.12.14
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…nting prices
**`@cluster(k8s_namespace="compute")` did nothing.** Extra keyword arguments
are copied into the job config from an allowlist, and the Kubernetes and
HuggingFace per-job options were not on it -- `hf_jobs.py` already reads
`hf_flavor` off `job_config` and `executor_kubernetes.py` reads the `k8s_*`
ones; they were simply never put there. The Kubernetes tutorial documents the
form that did not work.
They are on the list now, and anything unrecognised warns instead of
disappearing:
@cluster received unrecognised option(s) nonsense_option; they have no
effect. Recognised extras: aws_access_key_id, ..., k8s_namespace, ...
A silently ignored option is worse than a rejected one: the job runs with
settings the caller believes they changed.
**Cost estimates invented prices for instance types they had never heard of.**
`estimate_cost("invalid_instance", 1.0)` returned a confident
`CostEstimate(hourly_rate=0.1, ...)` — the table's `"default"` entry, which has
nothing to do with the instance — and said nothing. Someone budgeting against
that number has no way to know. All four providers now say so:
Unrecognised instance type 'invalid_instance'; priced at the placeholder
default rate of $0.1/hr. This is not a real quote.
Appended to `pricing_warning` rather than assigned, so the existing
outdated-data warning cannot displace it.
**The cost-monitoring docs described exceptions that never raise.** They showed
`get_cost_monitor('unsupported_provider')` raising ValueError and
`estimate_cost` raising KeyError. Neither does — the first returns None and the
second returns a placeholder estimate. The docs now show the checks that are
actually needed.
Full suite: 153 failed / 1407 passed / 36 errors, from 211 / 1194 / 72 at the
start of this branch. All three backends re-verified.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
The committed transcript dated from ten commits back. It is regenerated here,
and no longer opens with the flattening error, because the underlying cause is
understood: analyze_function_complexity returns 999 -- which counts as
'complex' -- when it cannot read a function's source, so flattening was
attempted exactly when it could not work. A function whose source reads
normally scores 13 and is never flattened.
slurm PASSED s12.hpcc.dartmouth.edu python 3.12.13 59.9s
gpu PASSED tensor01.dartmouth.edu python 3.12.13 12.5s
hf PASSED j-contextlab-6a841cc5... python 3.12.14 12.0s
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
Five defects, all of which let a function that runs locally fail remotely
for reasons the error message did not name.
Serialization shipped less than it claimed. `dill.dumps(func)` captures
closure cells but not `func.__globals__`, so a function calling a
module-level helper or reading a module-level constant serialized cleanly
and died on the worker with `NameError: name '_helper' is not defined`.
Arguments went through stdlib pickle, which stores a class by qualified
name, so passing an instance of a class defined in the caller's __main__
failed with "Can't get attribute 'Point'". Both now serialize by value,
and every path that reads them back -- two-venv, single-venv, Kubernetes,
cloud, HuggingFace -- was made symmetric.
Remote exceptions lost their type. The worker pickled
`{'error': str(e), ...}` and discarded the exception object, so the caller
could only ever rebuild a RuntimeError; `except ValueError` never fired,
contradicting extract_original_exception's own docstring. The exception
object now travels with the message, and an exception that refuses to
serialize no longer takes the error report down with it.
The execution environment was a hardcoded list of nine "core scientific"
packages, despite the setup function's docstring promising it "replicates
the local environment". Anything else the function imported -- torch,
networkx, PyYAML -- was simply never installed. The environment is now
mirrored from the local package manager (pip's freeze, which also covers
conda environments, or uv's when uv is in use), via a requirements file
so it resolves once instead of per package. `replicate_local_environment`
and `excluded_packages` are the manual adjustments.
HuggingFace containers installed nothing but dill and cloudpickle, so
`import numpy` failed there while succeeding on every cluster. They now
honour the same two config fields as the SSH and SLURM backends.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…sked for _environment_key hashed the Python version and the requirements list, and promised that "any difference produces a different key, so environments are never silently shared between jobs that need different packages". It did not hash the policy that decides which of those requirements actually get installed. So when VENV2 stopped installing nine hardcoded packages and started mirroring the local environment, every existing environment still matched its old key: the cache served the stale environment and the new packages were never installed. `import numpy` kept failing on a cluster whose environment had just been told to include numpy. The key now covers a recipe version, replicate_local_environment, excluded_packages and cluster_packages, so a change in what lands in an environment produces a new environment rather than a stale hit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
CI was red on Windows for a real reason and green everywhere for a false one.
test_cli_login_token_is_honoured monkeypatched HOME and expected
os.path.expanduser("~") to follow. On Windows expanduser reads USERPROFILE,
so the test looked at the real user's token path, found nothing, and failed --
while on POSIX it passed for reasons that had nothing to do with Windows. It
now sets both, and the failure it was pointing at is fixed too: HF_HOME
relocates the whole huggingface directory, and clustrix ignored it, telling
users who had already run `hf auth login` to log in again.
test_signature_fix and test_dartmouth_network_detection asserted nothing.
Every failure path did `return False`, and pytest counts a returned value as
a pass -- so flattening could mangle a signature, or the network gate could
return a string, and both files would still be green. They now assert what
they were describing: flattened functions match the original's signature and
answers for positional, keyword and named calls; and a positive network
answer means the gated hosts actually resolve, which is the only reason that
gate exists.
262 unit tests pass, with no PytestReturnNotNone warnings left.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…s file Two failures, one of which hid the other. A function that calls into the user's own project failed remotely with `ModuleNotFoundError: No module named 'mypkg'`. dill and cloudpickle store an importable object by *reference* -- "import mypkg.mathutils, then get triple" -- which is correct for numpy, because the worker has numpy, and wrong for a package that exists on no machine but the caller's. Since the reference is small, the payload looked healthy: 406 bytes for a function carrying an entire module's worth of behaviour. Project-local modules reachable from the function (and from its arguments, which arrive one container deep) are now registered with cloudpickle's register_pickle_by_value and embedded. Installed packages and the standard library are untouched, since the worker mirrors those, and clustrix itself is never embedded. The environment replication added earlier never ran at all. The requirements file was emitted as a shell heredoc, but these commands are joined with " && ", so the terminator line became `CLUSTRIX_REQUIREMENTS_EOF && source ...` -- not a terminator at all. `cat` swallowed the rest of the setup script into the requirements file, pip was never reached, and the environment stayed empty while the setup reported success. `import yaml` kept failing on a cluster that had just been told to install PyYAML. The file is now written over SFTP, and the recipe version moves so the environment built by the broken recipe is rebuilt rather than reused. Tests deserialize in a subprocess whose sys.path cannot reach the package, which is the only way to tell an embedded module from an imported one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
Red-teaming the previous commit found four defects and a performance cliff, each reproduced before being fixed. A module that cannot be embedded no longer degrades in silence. If cloudpickle raised -- a lock, an open file or a database handle held at module level and actually used by the function -- the code fell through to a by-reference dill payload: precisely the ModuleNotFoundError this feature exists to prevent, delivered minutes later from the cluster, naming a module sitting on the user's own disk. It now raises where the user is, naming the module and the real reason. The by-value registry is process-global, and AsyncClusterExecutor serializes on a thread pool. Two threads would each snapshot the registry before either registered, and the first to finish unregistered the module out from under the second, whose payload silently reverted to by-reference. Registration is now under a lock with a refcount, so overlapping users share one registration, and a module the *user* registered is never unregistered. Closure cells were not walked. `from mypkg.util import helper` inside a function binds a cell rather than a global, so that function's package was never detected -- found by a test whose premise turned out to be wrong for a different reason. The walk cost O(n) over every element of every argument, on every submission. A million-element list took 1.35s and 75MB before serialization began. Scalars cannot reference a module and are no longer enqueued: 0.034s, and serialization is back to dill's own cost. The HuggingFace size error blamed "closing over" the data. A large payload is just as often a module-level table in the user's package, embedded with it. The message now breaks the payload down and names both causes. 281 unit tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
Four reported symptoms, one cause and one unrelated bug. The profile dropdown "didn't do anything" because the widget only ever *read* profiles. Anything typed into the controls was held nowhere else, so switching away discarded it and switching back redisplayed the stored values -- indistinguishable from a dropdown that ignores you. The widget now stores the visible state into the profile being left before loading the one being entered, guarded by a flag so the widget's own writes to its controls are not mistaken for user edits. The "+" button made this worse: it cloned the *saved* profile rather than what was on screen, so every profile a user created was a copy of the defaults and switching between them genuinely had no visible effect. It now captures the current state first, which is also what makes "clone this and tweak it" work at all. Save and Load "didn't do anything" because their only feedback was a line of text in the output area, while the status pill -- the thing that reacts to Apply and Test -- stayed unchanged. A bare filename also resolves under ~/.clustrix, so nothing on screen said where the file had gone. Both now set the pill on success and failure, and the save message names the resolved path and the number of profiles written. The test job failed with "Unsupported cluster type: local". ClusterExecutor has no local backend, and `local` is the widget's default -- so that button failed for every new user before they had configured anything. Local execution now runs in process, which is what `local` means and the path a local @cluster call takes. 11 tests cover the profile lifecycle, including that edits do not leak between profiles and that every cluster type the dropdown offers leaves the test button in a reportable state. 292 unit tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
A class defined next to the user's function came back with the right fields
and the wrong identity: `isinstance(result, Point)` was False against the very
class that had defined it, and an `__eq__` guarded by isinstance returned
False while the repr looked perfect.
returned : Point(12,23)
is local Point: False
equality : False
I had recorded this as an inherent limit of by-value serialization. It is not.
The remote stages write result.pkl with dill; the caller read it with stdlib
pickle. stdlib pickle happily *replays* dill's reconstruction opcodes -- so
nothing ever errored -- but for a __main__ class it builds a fresh class object
rather than reusing the one already in memory. dill's loader reuses it.
This is the same asymmetry already fixed for arguments in an earlier commit;
the return path was simply missed. Results and error payloads now load with
dill everywhere: the SSH/SLURM path, the cloud path, and both exception
readers, where it additionally keeps a custom exception class bound to the
caller's own.
Verified against discovery, not just locally:
returned : Point(12,23)
is local Point: True
isinstance : True
equality : True
A test pins stdlib pickle's wrong behaviour alongside dill's right one, so the
reason stays visible, and a third fails if any backend reaches for stdlib
pickle on a result again.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
Every remote exception sent the account owner an email:
Your Job "python-3-12-slim-64e45370" status changed to ERROR
The container captured the exception, emitted its tagged error block, and then
re-raised. The process exited non-zero, so Hugging Face marked the job ERROR
and notified the owner -- for a Python function doing exactly what it was
written to do. `raise ValueError(...)` is an ordinary outcome that clustrix
already re-raises locally with its original type and the remote traceback
attached; nothing about the job failed.
The container now prints the traceback, so the job log still shows what
happened, and exits cleanly. Only a failure to *report* remains a job failure:
if emit() itself raises, that exception propagates and the exit is never
reached. The caller decodes the logs regardless of job stage, so a COMPLETED
job carrying an error block still raises for the user.
Verified against the contextlab namespace: the same raising function that
produced job 6a844c47 (ERROR) now produces 6a844fc7 (COMPLETED), while the
caller still sees ValueError with the original message and type.
Four tests run the real bootstrap in a subprocess -- pip is satisfied from the
local environment, so no network is required -- and check that it exits zero,
still emits the error block, still leaves the traceback in the log, and still
emits a result when the function succeeds.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
HuggingFace rejects very large environment variables, so a job's function and
arguments were capped at 256KB. clustrix refused anything larger and told the
caller to restructure their code around a Hub dataset -- which is a reasonable
thing to say and a poor thing to require, since passing a list of a hundred
thousand numbers is not an unusual request.
Oversized payloads are now detected and uploaded to a private dataset repo
under the job's namespace, and the job is told where to find it instead of
being handed the bytes. The repo is private because the payload is the user's
function and their data. The staged file is deleted once the job has finished,
whether it succeeded or failed, and also if submission itself throws, so
nothing is left behind for a job that will never read it.
Reading a private dataset needs credentials, and HuggingFace injects none
(probed: the container sees only HF_DATASETS_TRUST_REMOTE_CODE). The token is
therefore delivered as a job *secret*, and only for staged jobs -- an ordinary
job carries no account credentials into a container. A test pins both halves
of that.
Verified against the contextlab namespace with a 100,000-element argument,
about 500KB serialized:
expected: {'n': 100000, 'total': 4999950000, 'first': 0, 'last': 99999}
actual : {'n': 100000, 'total': 4999950000, 'first': 0, 'last': 99999}
and afterwards the repo is private with no payloads left behind.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
The profile dropdown offered exactly one entry, "Local single-core", which made it a text field with extra steps: there was nothing to choose between, and anyone wanting a SLURM or HuggingFace setup had to build it from defaults by hand. Templates now ship for all seven backends -- local single-core and all-cores, SLURM, PBS, SGE, SSH, Kubernetes, and HuggingFace Jobs on CPU and GPU -- each with sensible resources and the fields only the user can know (host, username, namespace) deliberately blank, where the widget's validation already names them before anything is submitted. The GPU template leaves hf_allow_gpu_flavors False on purpose. Those flavors bill by the second, so running one stays an explicit decision rather than a side effect of picking a profile from a list. Separately, load_profile set active_profile as a side effect, so merely inspecting profiles in a loop left the last one inspected marked active -- which is how this was noticed. It is now a pure read; callers that mean to switch use set_active_profile. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…ssion Red-teaming the profile row turned up more than the reported symptoms. Switching profiles destroyed settings. _load_config_to_widgets restored only a subset of what _get_config_from_widgets collects -- no HuggingFace or Kubernetes fields, no remote work directory, no password -- and left environment variables and modules untouched when the incoming profile had none. Once the widget started saving the visible state on the way out, that asymmetry meant clicking through the dropdown overwrote each profile with whatever the previous one left on screen. The two halves now mirror each other field for field, including resetting a control when the config does not set it. The password was collected and dropped. Test connect passed it explicitly and succeeded; the job itself authenticates from config.password and failed. That is the likeliest cause of the reported sample-job failure on a real cluster. The widget never read the library. It only wrote, on Apply -- so a session that had already called configure(cluster_type="huggingface", ...) opened on "Local single-core" with a blank namespace, and Apply would then overwrite the real configuration with the defaults on screen. It now opens showing the live configuration, as its own profile beside the templates. That also fixes the smaller version of the same bug: a fresh widget displayed 16GB while the profile it named held 16.25GB. Apply merged instead of replacing. Skipping None values meant fields survived across applies: ssh then local left cluster_host and username pointing at the old cluster. Profiles did not survive a kernel restart -- the built-ins were re-seeded and anything the user built was gone. They now persist to ~/.clustrix/profiles and reload on start. Tests are isolated from that store rather than reading the developer's own. The config file field was a text box whose Load tooltip promised a file dialog that did not exist, defaulting to clustrix.yml -- the library's own config file, in a different format that load_config rejects. It is now a picker over files that actually parse as profile bundles, in the config directory and the working directory, defaulting to profiles.yml. Smaller ones: the clone-environment checkbox had no effect; the Apply button relabelled itself "Applied!" via a reset closure that was never called, so it stayed that way all session; the package manager menu omitted uv, which the config documents and utils implements; typing an unknown profile name left the box naming a profile that did not exist; and the dropdown was sorted in one place and unsorted in another, so it reordered after add or remove. 22 tests cover the profile row; 311 pass overall. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
Apply has now been wrong in both directions. It began by merging only
non-None values, so fields from a previous Apply lingered: switching from an
SSH profile to a local one left cluster_host and username pointing at the old
cluster. The fix for that replaced the configuration wholesale -- which
discarded every setting the widget has no control for. Reproduced:
cluster_packages ['networkx'] -> []
excluded_packages ['appnope'] -> []
job_poll_interval 5 -> 30
venv_setup_timeout 1800 -> 300
Those are only settable from code, so a user who had configured them and then
touched the widget lost them silently.
Apply now resets exactly the fields the widget can set, then applies what is
on screen, leaving everything else alone. `_get_config_from_widgets` was split
so the collector returns the fields it set rather than only their values,
because Apply needs the keys: a managed field the current backend did not set
has to be reset, while an unmanaged field must be preserved.
WIDGET_MANAGED_FIELDS names that set explicitly, and a test asserts it equals
the union of what the collector actually produces across all seven backends --
a field collected but unlisted would never be reset, and a field listed but
not collected would be wiped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
A second red-team pass over the reworked widget, seven findings, each reproduced before and after. Password authentication was unreachable. The SSH key field was pre-filled with "~/.ssh/id_rsa" as a real value rather than a placeholder, and it was written straight back into the config. executor_connections tries key_file first and only falls back to a password when it is falsy -- so typing a password authenticated against a key file that does not exist. The field now starts empty and an empty field means None. Merely opening the widget destroyed a saved "Current configuration" profile, including any token in it, by writing the live config over it unconditionally. It now creates that profile only when absent, and takes a numbered name otherwise. Two ways to crash or hang on construction, both from scanning the working directory for config files: a broken symlink raised FileNotFoundError because the mtime sort sat outside the try, and a FIFO named *.yml passed the size check and then blocked forever in open() with no writer. Directories are now sorted defensively and only regular files are opened. The size ceiling drops from 5MB to 256KB, since a 1.7MB YAML in the working directory cost eight seconds of startup and no profile bundle is remotely that large. Glancing at another cluster type erased the profile being left: collection was gated on the current type, so flipping the menu and then switching profiles dropped the host, username and work directory. Every managed field is now collected every time. Apply still scopes to the chosen backend, because _choose_execution_mode routes on cluster_host and a leftover host would send a "local" job to a cluster. A corrupt store wiped the built-in templates: load_from_file cleared the profiles and repopulated entry by entry, so one bad entry left a partial set with active_profile naming something that no longer existed. It now parses into a temporary map and swaps it in only once everything has parsed, and re-points active_profile if the file names a profile it does not contain. clone_profile and import_profile never persisted, so the "+" button's output was held in memory only and vanished on restart. An unwritable config directory raised from the constructor instead of degrading to a session-only store. 36 tests cover the profile row; 326 pass overall. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
…result GitHub reported a high-severity advisory against black: arbitrary file writes from unsanitized input in the cache file name, affecting every release from 24.3.0 up to 26.3.1. The pin at 25.1.0 was deliberate -- an unbounded ">=" had previously let CI pull a newer black and reformat the tree mid-run (#110) -- but a formatting pin is not a reason to sit on a high advisory. Pinned to 26.3.1, the first release with the fix, in pyproject, setup.py and the pre-commit rev so all three stay in step. The re-formatting diff is entirely mechanical. Installing it exposed two things that were already wrong. The hooks asked for `python3`, which on this machine is 3.9.13 -- below the project's own requires-python of >=3.10 -- so every hook had been building its environment on an interpreter the project does not target. Nothing complained until black 26.3.1 refused to install there. The config now names a supported interpreter once, as default_language_version. And there were two black hooks. A local "auto-format" step ran with `language: system`, meaning whatever black was on PATH -- 25.11.0 here -- while the pinned hook ran 26.3.1. The two format differently, so the first rewrote the tree one way and the second rewrote it back, and no commit could satisfy both. The psf/black hook already formats and fails when it changed something, so the local step was redundant as well as harmful; it and its script are gone. Also marked the one E402 the hook surfaced: a validation script that adjusts sys.path before importing clustrix, which is the point of it running standalone. 336 tests pass; flake8 and mypy clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
The macOS and Windows CI jobs were being cancelled at the fifteen-minute
timeout, inside a test I added earlier in this branch:
test_detection_returns_a_boolean PASSED (~70s)
test_detection_is_stable_across_calls PASSED (~70s)
##[error]The operation was canceled.
is_dartmouth_network does a reverse-DNS lookup and then resolves
tensor01.dartmouth.edu. On a runner with no route to Dartmouth both
blackhole, so each call took over a minute; three calls used the whole job
budget. Ubuntu fails those lookups quickly, which is why only macOS and
Windows died -- and why writing the test at all is what exposed it.
The check only decides which tests are allowed to run, so it should answer
quickly and wrongly-but-safely rather than slowly and exactly. Each lookup now
has a three-second budget and the result is cached for the process.
The first attempt at the bound did not work, and the test that says so is
committed alongside it: the executor was used as a context manager, whose
__exit__ calls shutdown(wait=True) and blocks until the worker finishes -- so
a three-second budget still took the full thirty. Measured before and after:
30s call abandoned after 30.00s (with the context manager)
30s call abandoned after 3.01s (without)
Also dropped the ping fallback, which could add another five seconds to
answer a question the two lookups above had already answered, and the now
unused subprocess import.
338 tests pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
Two failures on the Windows runners, both in tests added on this branch, and one of them pointing at a real portability bug in the code. test_every_result_path_reads_with_dill scanned clustrix/*.py with Path.read_text() and no encoding. Python picks the locale default, which is cp1252 on those runners, so a source file containing a non-ASCII byte raised UnicodeDecodeError. The same omission is in the code the test guards: ProfileManager reads and writes profiles.yml, and the widget's file picker opens candidate config files, all without an encoding -- so a profile named with an accent would have been mangled or refused on Windows while working everywhere else. All of them now say utf-8. test_a_fifo_named_like_a_config_file calls os.mkfifo, which does not exist on Windows. Named pipes are a POSIX concern, and so is the hang it guards against, so it skips there rather than pretending to check something. Separately, Quick Checks could never have passed: it installed a hand-listed "black flake8 mypy pytest" rather than the dev extra, which pulled an unpinned black -- the drift pyproject was pinned against in #110 -- and left out pytest-timeout while the test step passes --timeout=60, so the job died with "unrecognized arguments: --timeout=60" whatever the code did. It now installs ".[dev]" like the other jobs. macOS was passing all 327 tests and then being cancelled by fail-fast when Windows failed; there was nothing wrong with it. 338 tests pass locally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
The Security Scan job failed on "Check for secrets", which ran:
! grep -r "password\|secret\|key\|token" clustrix/ --include="*.py" \
| grep -v "def\|class\|#\|test"
It flags 1138 lines of this repository. Among them:
kwargs: Function keyword arguments
because "keyword" contains "key". The check could never pass on a codebase
that handles credentials, which is exactly the codebase worth checking -- and
it could not have caught a real secret either, because a leaked token does not
contain the word "token". It had been skipping on most events, so nobody
noticed it was unpassable.
scripts/check_for_secrets.py looks for the shapes credentials actually take:
provider token formats anchored on the prefixes those providers issue, PEM
blocks that carry a real base64 body rather than a header wrapped round
MOCK_KEY_CONTENT, and long literal strings assigned to credential-named
variables. Placeholders are not findings, since a checker that cries wolf gets
switched off: <redacted>, $TOKEN, {api_key}, your-password-here, xxxxxxxx,
wrong_password, and the example keys AWS publishes in its own documentation.
Running it found nothing real, and one thing worth changing: the HF_TOKEN
example in credential_manager.py was written as a full-length token, so it
looked usable. It now reads hf_your_token_here.
23 tests cover both halves -- what it must catch and what it must stay quiet
about -- including the exact line that made the old check unpassable, and a
planted GitHub token to prove the scan is not vacuous. Writing them caught my
own test datum containing "EXAMPLE", which the suppression correctly ignored.
361 tests pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
docs-test failed on two consecutive runs, both times inside
sudo apt-get update
stuck fetching `noble-security InRelease` for nine and a half minutes until
the job's ten-minute cap cancelled it. apt has no timeout by default, so an
unresponsive mirror hangs for as long as the job is allowed to live, and the
failure is reported against the documentation build rather than the network.
Each fetch now gives up after fifteen seconds and retries three times, so a
dead mirror costs seconds instead of the whole job. pandoc installs with
--no-install-recommends, which is all the docs build needs from it.
Every other check on this PR passes: the test matrix across Ubuntu, macOS and
Windows on 3.10-3.12, Quick Checks, Security Scan, Docker, Trivy and both
integration jobs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
Eighteen use cases correct on five backends, all CI green, plus the parts that remain unverified so the next session does not have to rediscover the boundary: cloud backends, PBS and SGE on real hardware, Kubernetes, and the flattening feature. The pattern worth carrying forward is that nearly every bug here was an asymmetry between a writer and a reader -- and several were introduced by the fix for the previous one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
jeremymanning
added a commit
that referenced
this pull request
Aug 18, 2026
…138) Closes #133. Confirms #135 resolved by #137. pre_push_check.py promised "run this before pushing to ensure GitHub Actions won't fail" and could not pass. It shelled out to bare black/flake8/mypy/pytest, so it ran whatever was first on PATH -- Anaconda's mypy 1.19 rather than the project's 2.3, reporting 27 missing-stub errors for stubs pyproject declares. Tools now run under the script's own interpreter. Its flake8 step reported 91 findings. Four were real defects, including a remote program wrapped in an outer f-string, so {torch.__version__}, {i}, {props.name} and {e} interpolated locally and the test raised NameError before sending anything; and json.loads with no module-level import, swallowed by a bare except. The other 74 were one deliberate pattern -- standalone scripts that adjust sys.path or the environment before importing the package they exercise -- now recorded once in .flake8. Also fixes two things that made master red after its tests had passed, in steps that never run on a pull request: the coverage-badge updater treated a deliberately absent badge (#115) as an error, and the follow-on step pushed with nothing to push. And the dependabot advisory against black, which stayed open because it points at docs/requirements.txt, where black arrives transitively and was unconstrained.
Closed
5 tasks
jeremymanning
added a commit
that referenced
this pull request
Aug 19, 2026
…umentation (#139) * Session notes: state of the issue sweep and the defects found so far Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #124: stop declaring black as a docs dependency it does not have The dependabot advisory GHSA-3936-cmfr-pm3m originally matched pyproject.toml's black==25.1.0, which sat inside the vulnerable range >=24.3.0,<26.3.1. That pin is already corrected to ==26.3.1. Commit bf524a4 then added a black>=26.3.1 floor to docs/requirements.txt on the assumption that black arrived there transitively through the Jupyter stack. It does not: resolving that file from scratch installs 117 packages and black is not one of them. The floor added a dependency rather than constraining one, and because '>=' leaves the version unresolved the dependency graph recorded black with versionInfo: null -- which is why the alert stayed open against docs/requirements.txt after the real cause was fixed. Drop the line, and pin setup.py's dev extra to ==26.3.1 so no unbounded black constraint is left anywhere for the graph to resolve as unknown. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #95: Restore AWS resource cleanup/destroy scripts as scripts/aws/ cleanup_test_resources.py and destroy_cluster.py were deleted by b9c836f ("Issue #72: Delete obsolete development scripts") on the false claim that they were migrated to scripts/aws/ -- that directory never existed. Recover the real source from git history (b9c836f^) and restore it as scripts/aws/cleanup_resources.py and scripts/aws/destroy_cluster.py, hardened per issue #95: - Default to dry run; require --execute to delete anything. - Print every resource (type, id, region) before acting, in both modes. - Only touch resources positively identified as Clustrix-managed, using the same clustrix:managed / clustrix:cluster tags and IAM role naming that clustrix.kubernetes.aws_provisioner.AWSEKSFromScratchProvisioner applies (the original script's VPC cleanup had no such check at all, and its IAM role name guesses never matched what the provisioner actually creates). - Fail loudly with a clear message when AWS credentials are missing, instead of silently falling through to boto3's default credential chain. Add tests/unit/test_aws_cleanup_scripts.py: verifies --help, the missing-credentials failure path, and argparse defaults via real subprocess calls and real argparse round trips (no AWS account, no mocked boto3), plus static (ast-based) proof that every destructive boto3 call is reachable only through execute_plan() and only when gated behind `if args.execute`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #109/#114: Close real_world marker hole in "safe" test command tests/real_world/conftest.py's pytest_collection_modifyitems hook added skip markers for expensive/visual/dartmouth tests but never applied the real_world marker itself. 6 of 81 files under tests/real_world/ carried no @pytest.mark.real_world decorator, so the documented CI-safe command `pytest tests/ -m "not real_world"` silently collected and ran them -- making real SSH connections and cloud API calls. The hook now applies pytest.mark.real_world to every item whose path is under tests/real_world/, closing the hole regardless of whether the file itself declares the marker. The hook is scoped by path (not applied unconditionally) because pytest_collection_modifyitems fires once per session with the full item list, not just items from this directory -- an unscoped version would have marked the entire suite as real_world. Adds tests/unit/test_billable_and_realworld_isolation.py: runs real pytest in subprocesses (collect-only) to prove `-m "not real_world"` now collects zero tests/real_world items, pins the 6 previously-leaking files individually, guards against the marker leaking onto the rest of the suite, and adversarially re-checks the tests/integration billable guard (cwd change, -p no:cacheprovider, --co, direct import) -- no bypass found; that guard's config.args-based design (not invocation_params.args) is unchanged. CLAUDE.md's documented commands (`pytest tests/ -m "not real_world"` and `pytest tests/real_world/ -m real_world`) are both accurate after this fix and need no wording change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #113: Make CI actually run the full suite; consolidate real-world workflows - tests.yml: default test job now runs `pytest tests/ -m "not real_world" --ignore=tests/real_world --ignore=tests/integration` instead of only tests/unit/ (~350 of ~1,532 non-billable tests). Belt-and-braces with the real_world marker another agent is auto-applying in tests/real_world/conftest.py. Also dropped a `|| true` on the integration-test job's pytest step (verified the 7-test selection it guards passes cleanly without it). - fast_ci.yml: removed `continue-on-error: true` from the mypy step. Verified clean (`Success: no issues found in 69 source files`) once the dev extra's type stub packages (types-PyYAML/requests/paramiko, already declared in pyproject.toml) are actually installed -- no clustrix/ changes needed. - Consolidated the two duplicate real-world-test workflows (hyphen vs underscore) into one canonical real-world-tests.yml, matching what docs/CREDENTIAL_SETUP.md already documented. Deleted real_world_tests.yml, whose jobs depended on fictional infrastructure (Kind clusters, a recovery_report.json/performance_results.json nothing produces). - Replaced the three `if: false` gates with real ones: workflow_dispatch (manual) plus a weekly schedule, gated on secret presence via a check-secrets job (the `secrets` context is not permitted in job-level `if:` -- actionlint caught this). No push/pull_request trigger exists on this workflow at all, so a fork PR cannot invoke it under any condition. Also fixed a dead reference to a nonexistent scripts/test_real_world_credentials.py. - Added hf-jobs-integration: a workflow_dispatch/schedule job gated on HF_TOKEN that submits a real function through clustrix's HF Jobs backend (contextlab namespace, cpu-basic flavor, per the verified #118 config) -- the first integration-test substrate in this repo that can actually run without SSH/cloud credentials. - Bumped actions/setup-python@v4->v5 and actions/cache@v3->v4 across touched files per actionlint (all other actionlint findings and secrets-in-if bugs in files I touched are resolved; actionlint exits 0). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #113: repair .flake8 and stop the lint step being unable to fail .flake8 was sitting in the working tree with unresolved conflict markers (<<<<<<< Updated upstream / >>>>>>> Stashed changes). flake8 cannot parse that, so it silently fell back to its defaults -- 79-character lines and none of the per-file-ignores -- and reported violations the project had deliberately configured away. The committed version was correct; restore it. .gitignore and .pre-commit-config.yaml were left in the same unmerged state and matched HEAD exactly. The corruption came from the pre-commit hook's stash/restore cycle running against a tree that other work was modifying at the same time. tests.yml's flake8 step passed --exit-zero, so the step could not fail CI no matter what it found, and carried its own --extend-ignore list that had drifted from .flake8. Drop both; .flake8 is the single source of truth. Verified: flake8 clustrix/ tests/ scripts/ reports 0 findings with the restored config. Also extend both lint steps to cover scripts/, which is already clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #116: Remove test-detection from shipped production code executor_scheduler_status.py: the SLURM status check branched on isinstance(ssh_client, Mock) to skip retry/sacct logic during unit tests. Replaced with the real condition it was standing in for: whether there is a live SSH connection at all (ssh_client is None). No currently-passing test depended on the sniff -- the tests that exercised it were already failing for unrelated reasons (connection_manager.execute_remote_command has moved on from what they mock). notebook_magic_mocks.py: renamed to notebook_magic_fallback.py and stripped of the ~115 lines of fake ipywidgets classes (_MockDropdown, _MockButton, etc.). EnhancedClusterConfigWidget.__init__ already refuses to construct without real IPython+ipywidgets, so those classes' methods were provably dead code -- nothing ever reached them. ipywidgets is an intentional optional dependency (see pyproject.toml's `widgets` extra and the GitHub-Actions-compat test suite that exercises import without it), so a hard-require was not the right call; instead `widgets` is now a placeholder that raises a clear ImportError on any attribute access instead of faking the API. The magics/display/HTML shims that ARE genuinely exercised without IPython installed (ClusterfyMagics's line magic, etc.) are kept as real, honestly-degraded implementations. Also fixed unresolved git-conflict markers left in .gitignore and .pre-commit-config.yaml (discovered while resolving an unrelated stray stash during this work); kept the deliberate current content over the stale stashed alternative in both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #121: Verify SSH host keys against known_hosts by default, reject unknown ones Every paramiko.SSHClient() in the SSH-touching modules used AutoAddPolicy(), which trusts any host key on first connection with zero verification -- every SSH connection clustrix made was MITM-able. Adds a single shared helper, clustrix.ssh_security.configure_host_key_policy(), that loads system + user known_hosts and defaults to rejecting unknown host keys with an actionable error naming the host and the exact ssh-keyscan command to fix it. The old insecure behavior is now an explicit, documented opt-in via ClusterConfig.ssh_host_key_policy="auto_add". All 10 owned call sites (ssh_utils.py x3, executor_connections.py, filesystem.py, validation.py x2, cli_credentials.py, kubernetes/lambda_provisioner.py) now call the shared helper instead of repeating the policy decision. The Lambda Cloud provisioner, which connects to freshly-booted ephemeral instances with no prior known_hosts entry, does an explicit ssh-keyscan (reusing ssh_utils.add_host_key) before each connect attempt as a logged trust-on-first-use step rather than blanket auto-trust. Issue #111: Save config files at 0600 and omit secrets by default ClusterConfig.save_to_file/save_config wrote via plain open(path, "w") with no mode and no secret exclusion, so any password/token/API key on the config landed in a 0644 file. Files are now created via os.open() with mode 0o600 (plus an immediate fchmod so a pre-existing, more permissive file is tightened before any content is written -- never after). Secret- bearing fields are omitted by default, determined programmatically from ClusterConfig's field names via the same regex approach already used in scripts/verify_cluster_usecases.py (now imported from clustrix.config as the single source of truth instead of being duplicated). Pass include_secrets=True to write them anyway. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #121: cover the last widget host-key site; require cloudpickle 3 Two AutoAddPolicy call sites were left behind when the rest moved to configure_host_key_policy. This closes the notebook widget's _test_ssh_connectivity, which hands its configuration over as a plain dict rather than a ClusterConfig -- so the shared helper now reads either shape, keeping the policy decision in one place instead of letting the widget invent its own. Verified: dict {'ssh_host_key_policy':'auto_add'} -> AutoAddPolicy with the warning; empty dict and None -> RejectUnknownHostKeyPolicy. executor_cloud.py's site is still open; that file is being rewritten in parallel and the fix goes in there. Separately, raise the cloudpickle floor from 2.0.0 to 3.0.0. Under cloudpickle 2.0.0 a by-value-registered local package that defines a typing.NamedTuple cannot be loaded back when the module object itself is in the function's globals -- i.e. the ordinary 'import mypkg; mypkg.f(x)' idiom: cloudpickle 2.0.0 via_from dumps 953 bytes LOAD: OK -> 2 via_module dumps 3844 bytes LOAD: KeyError: '__module__' cloudpickle 3.1.1 via_from dumps 1079 bytes LOAD: OK -> 2 via_module dumps 4120 bytes LOAD: OK -> 2 The 'from mypkg import f' spelling happens to work under 2.x because cloudpickle then embeds only the globals the function actually uses, so the NamedTuple never enters the payload. That is why this went unnoticed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #89/#90: stop clustrix fabricating results for source-less functions _execute_single substituted a different callable for the user's function whenever analyze_function_complexity reported "complex", and that reporting was backwards: the analyser's except branch returned complexity_score 999, is_complex True whenever inspect.getsource failed. So for any REPL, notebook or exec-created function, clustrix attempted a source rewrite that cannot work, fell through to create_simple_subprocess_fallback, and ran a hardcoded subprocess whose entire body was `result = "Function execution completed"`. That string was returned to the caller as the job's answer, with no error. - delete create_simple_subprocess_fallback outright; it never ran the user's function and cannot ever produce a correct answer - _execute_single now serialises the function the caller wrote, always. No rewrite is substituted, because equivalence of a rewritten function cannot be verified without running the user's function. It is also unnecessary: serialize_function already pickles by value via dill(recurse=True) / cloudpickle, which round-trips nested functions, closures, module globals and source-less functions. decorator.py no longer imports the flattener at all, so the substitution cannot be reintroduced by accident. - analyze_function_complexity reports source_available. On the failure branch the metrics are None and is_complex is False -- "I could not analyse this" is now distinguishable from "I analysed it and it is complex". - auto_flatten_if_needed no longer reports success: True while handing back the original function. It returns explicit flattened/success/reason/strategy and skips entirely when there is no source to rewrite. It also no longer picks a hoisted helper as the main function: the namespace lookup was a substring match, and helpers are named {parent}_{nested}_hoisted. - #89/#90 TODOs replaced with the reason they are not being implemented: flattening has no caller in the execution path, so completing the closure and global-variable plumbing would only make an unusable rewriter reachable. New tests run for real -- no mocks. SubprocessJobRunner is a genuine implementation of the executor contract that ships the real serialized payload to a fresh interpreter and executes it. Against the pre-fix code these fail with: "clustrix returned 'Function execution completed' but the function computes 42". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #125: rewrite CLAUDE.md against the architecture that exists The file told anyone reading it to add a cluster type to a ClusterType enum in config.py. There is no ClusterType anywhere in the package -- cluster_type is a plain str. It located ClusterExecutor in clustrix/executor.py, which is a 39-line re-export shim; the implementation is spread across seven executor_*/hf_jobs modules, none of which were mentioned. It claimed functions defined in a REPL 'cannot be serialized', which is false and is the belief that produced the fabricated-result bug: serialization works fine without source, only the inspect.getsource-based features need it. It also carried one half of a mocking policy that contradicted .claude/CLAUDE.md's other half, so developers could cite either and neither governed. Both are replaced by one stated policy: real verification first, mocks only as a cost-control stand-in afterwards, never as a fallback, never inside shipped code, and never a reason to weaken a failing test. Added: the two-venv execution model and why every handoff must stay symmetric; HMAC verification of remote results; host-key verification via ssh_security.configure_host_key_policy; the billable-test guard and why it reads config.args; a backend table that says plainly which backends are actually proven and which are not. Every factual claim in the new text was checked against the code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #119 / #120: make every backend report what actually happened Kubernetes (#119): the worker printed `CLUSTRIX_RESULT:{result}` -- the repr of the result -- and the caller ran it through ast.literal_eval, returning the repr string when that failed and the whole pod log when no marker was found. It now writes a base64 pickle plus an HMAC over those bytes, keyed by a per-job secret passed in CLUSTRIX_RESULT_KEY, and the caller verifies it before deserializing. check_k8s_job_status no longer answers "completed" whenever the API call raises: an outcome that cannot be read is an error. The worker program is now a module-level function, so it can be run directly and tested without a cluster. cluster_type "local" (#120): ClusterExecutor had no branch for it and raised "Unsupported cluster type: local", though the widget offers it. LocalJobManager in local_executor.py wires it to the existing LocalExecutor. PBS (#120): submit_pbs_job never set up a remote environment, so its script activated a virtualenv nothing had created. SLURM and SSH each carried a copy of the two-venv setup and SGE had only half of it; all four now share _stage_job_directory and _setup_job_environment. Placeholder hostnames (#119): azure/gcp/lambda returned cluster_host "placeholder.<provider>.com" (or "") when they could not read an instance's address, which surfaced later as an SSH failure against a domain that does not exist. They now raise, naming the provider, the instance and what could not be determined. Provider interface (#119): only Lambda implements create_instance. Submitting to another provider was accepted and the NotImplementedError then surfaced inside a background thread. submit_cloud_job now checks the interface and the authentication up front and refuses with a message naming both. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #111: filter environment_variables entry-by-entry instead of dropping it Redacting secrets from saved configs swept environment_variables into SECRET_FIELDS wholesale, which meant a save/reload cycle silently lost OMP_NUM_THREADS along with AWS_SECRET_ACCESS_KEY. It also contradicted the rationale stated directly above it -- derive the secret set from field names, do not hand-list -- since it was a hand-added exception. Each entry is now judged on its own key name, so ordinary settings survive and credentials do not: environment_variables saved {'OMP_NUM_THREADS','MY_PIPELINE_STAGE', 'AWS_SECRET_ACCESS_KEY','HF_TOKEN'} loaded {'OMP_NUM_THREADS','MY_PIPELINE_STAGE'} plaintext AWS secret on disk: False plaintext HF token on disk : False file mode: 0o600 Two fields also matched the pattern without holding a secret: use_env_password is a boolean flag, and password_env_var holds the NAME of an environment variable rather than its value. Dropping them broke the auth-fallback round trip while protecting nothing, so the derivation now excludes 'use_*' and '*_env_var'. Two tests asserted the old behaviour; their assertions are rewritten rather than the code reverted, and two new tests cover the mapping. Also renamed the credential-shaped test fixtures that made check_for_secrets report 6 findings on the tree. They were real fixtures, but the scanner was right to be suspicious of 'hunter2-super-secret' and 'sk-real-looking-secret-abcdef123456'. Renaming them to obviously-fake values keeps the scanner strict rather than teaching it a suppression marker that could later hide a real secret. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #70/#96/#88/#124: Fix docs, add K8s auto-provisioning + usage-patterns tutorials #124: Fix MIGRATION.md's incorrect `from clustrix import ClusterConfig` (not re-exported; must be `from clustrix.config import ClusterConfig`). Fix the sphinx duplicate-object warnings (60 -> 4): filesystem.rst, file_packaging.rst, and dependency_analysis.rst each had a blanket `automodule:: :members:` PLUS per-member autoclass/autofunction directives for the same objects; since this project's autodoc_default_options sets members=True globally, both fired. Switched those three pages to `currentmodule` + the explicit-directives-only pattern already used in cost_monitoring.rst. The remaining 4 warnings come from a docstring in clustrix/notebook_magic_config.py (out of docs/ scope). #70: Document Kubernetes auto-provisioning (previously undocumented) in kubernetes_tutorial.rst: local kind-based provisioning (no cloud credentials needed) and the five cloud providers, each explicitly labeled unverified per README's existing wording. Documents a real gotcha found while verifying against the source: `@cluster(provider=...)` does not select the Kubernetes provisioner -- `configure(k8s_provider=...)` does, and it defaults to "aws". #96: New docs/source/tutorials/usage_patterns.rst turning issue #96's deleted-script snippets into verified, runnable patterns. Notes that @cluster falls back to local execution with no cluster configured, which is what makes the examples runnable without a real cluster. #88: Verified clustrix.utils.serialize_function/deserialize_function round-trip a function whose source is unavailable (prints 5). Narrowed usage_patterns.rst's description of the REPL limitation accordingly: it's the source-based features (loop parallelization, GPU-parallel detection, dependency analysis) that need inspect.getsource(), not serialization itself. README wording changes reported separately (README.md is out of this agent's file ownership). Added scripts/check_docs_examples.py: extracts every Python code block from the touched docs, executes the ones that don't need external resources for real (no mocks), and for blocks marked `# cluster-required` checks syntax plus that every imported name actually exists via importlib/hasattr. Also fixed two real bugs found while extending this script's coverage to the pricing docs at the coordinator's request: PRICING_USER_GUIDE.md's CustomPricingClient example was missing the required abstract method _fetch_pricing_from_api (verified TypeError without it), and the documented CostEstimate dataclass had invented fields (provider/hours/region) that don't exist on the real one in clustrix/cost_monitoring.py. Removed all documentation of clustrix.pricing_clients.performance_monitor, .resilience, and .validation_alerts (deleted as orphaned code) from PRICING_API_REFERENCE.md and PRICING_USER_GUIDE.md, replacing each with either the real remaining API or an explicit removal note -- no invented replacement APIs. 36/36 doc code blocks pass scripts/check_docs_examples.py (29 executed for real, 7 statically verified as needing external resources). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #124/#127: unify the version, give cluster types one source of truth Four files disagreed about what version this is: pyproject.toml and setup.py said 0.1.1, clustrix/__init__.py and docs/source/conf.py said 0.1.0. All four now say 0.2.0, the release #127 is about. The set of supported cluster types was written out separately in clustrix/cli.py and the widget dropdown, and they had drifted: the CLI offered slurm/pbs/sge/kubernetes/ssh/local and omitted 'huggingface' entirely, so a backend that is verified working end to end could not be selected from the command line. Both now read config.SUPPORTED_CLUSTER_TYPES. Verified they agree: canonical : ('local','ssh','slurm','pbs','sge','kubernetes','huggingface') CLI choices : ['local','ssh','slurm','pbs','sge','kubernetes','huggingface'] widget : ('local','ssh','slurm','pbs','sge','kubernetes','huggingface') Export ClusterConfig from the package root. ClusterExecutor, ClusterFilesystem and ProfileManager were all exported and it was not, so the obvious import raised ImportError -- which MIGRATION.md had been telling users to write. README's REPL section claimed such functions 'cannot be serialized'. They can: serialization works from the code object. What is actually lost is the source-based features -- loop parallelization, GPU-parallel detection, complexity analysis. Narrowed to say that, since the overstatement is what justified the flattening detour that fabricated results. Last 4 sphinx warnings fixed: autodata pointed at the module that re-exports DEFAULT_CONFIGS rather than the one that defines it, so autodoc fell back to dict.__doc__, whose **kwargs and indented body are not valid RST. Docs now build with zero warnings, down from 60. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #137-followup: fix environment replication and the by-value walk D1 get_environment_requirements dropped every `name @ file:///...` line, which is how uv renders conda-built packages -- 187 of 563 packages here. It now reads installed metadata directly, so uv and pip can no longer produce two different answers (and two different _environment_key values) for the same machine. Requirements that genuinely cannot be reinstalled remotely (editable installs, VCS checkouts, bare egg-info source trees) are no longer silently dropped: they are reported, and a payload that reaches into one is refused at submit time naming the package. D2 The walk no longer truncates at a node cap and submits anyway; it finishes, or raises WalkTooLargeError. D3 Instance attributes (__dict__ and __slots__) are now walked. D4 A project-local class subclassing dict/list/tuple is now followed by type, so it travels instead of arriving stripped of its methods. D5 _is_local_module falls back to __path__, so PEP 420 namespace packages are recognised and their children enqueued. D6 functools.partial and bound methods are followed to their targets. D7 The unpicklable-object message names the object and where it actually lives (closure variable, module-level name, attribute) instead of asserting "module level" and listing every local module in the payload. D8 A remote interpreter is accepted only when its minor version matches the local one; payload bytecode does not cross minor versions. D9 Conda environments are stamped ready only after every install succeeded, the reuse check requires that stamp, and no install is wrapped in `|| echo 'Failed to install ...'` any more. The mock-based environment tests that asserted clustrix shells out to pip are replaced with real ones against the real environment -- faking freeze output is exactly why the uv/pip divergence went unnoticed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #121/#126: authenticate every payload a job hands back, and stop the job script being a shell V1 (critical) result.pkl was HMAC-verified before dill.loads and error.pkl was not, so "make the job fail" was a complete bypass: a hostile cluster exits non-zero, writes its own error.pkl, and its __reduce__ runs on the submitting machine. Both call sites wrapped the load in `except Exception: pass`, so a failed attempt was silent. error.pkl is now signed with the same per-job key by every worker path and verified on the same terms; the refusal is raised outside the try so it cannot be swallowed into the text-log fallback. V2 Cloud results were deserialized with no key generated and no verification at all, under a comment claiming the worker wrote them with dill while the same file wrote them with pickle.dump. The cloud path now creates its work dir 0700 with a 0600 key inside, reuses result_signing_lines()/verify_signed_payload(), and writes with dill as the caller has always claimed. V3 Verification failed OPEN when no key was recorded: it warned and loaded anyway, so "no key" -- an adopted job id, a cleared table -- was as good as a valid signature. Missing key, empty key and untracked job are all refusals. V4 Every config value reaching the generated job script was pasted in raw. Ordinary shell words (job dir, env-var values, pip specs, venv paths, interpreter) are shlex.quote()d; places that must stay unquoted (module load arguments, #SBATCH/#PBS/#$ directive bodies, export names) are validated against a strict allowlist and refused naming the config key. pre_execution_commands stay a fragment on purpose. V5 The signing key stayed in the job's environment while the user's function ran, so any dependency could forge a validly tagged result. It is captured and popped before user code in every generated program, popped in the HF bootstrap before pip runs, and the HF log parser now selects the block that verifies rather than the first one printed. V6 `dill or cloudpickle or pickle` silently fell back to stdlib pickle on dill bytes -- the exact failure #121 was filed about. The requirement is now stated and the job fails naming the missing package. Also replaces the last AutoAddPolicy() in the package (executor_cloud) with configure_host_key_policy(). Regression tests are real: a real directory as the remote host, real signing, real tampering, and the generated worker programs really executed. 15 of the 21 new authentication tests and 8 of the updated ones fail against the previous tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #137-followup: cache the metadata scan; assert the precise D7 message Reading installed metadata costs ~0.25s and a submission asks for it several times, so it is cached keyed on sys.path -- the thing that decides which distributions are visible, so any change that could change the answer changes the key. test_an_unembeddable_module_raises_here_not_there asserted the old text, which blamed module level for a lock held in a CLOSURE. It now asserts the message names the closure variable, the function, and the object type. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #106/#131: fix real loop-analysis correctness bugs, add semantic tests Salvage review of PR #128 (branch epic/test-coverage-90-percent) found its loop-analysis tests too weak (isinstance(x, list) only) and its "advanced" suite dependent on API that doesn't exist on master (integrate_with_decorator, enhanced_dependency_analysis, etc.) -- not ported. Comprehension auto-parallelization (visit_ListComp/SetComp/DictComp/GeneratorExp, #132's landmine) was likewise not brought across; a test now pins that down. While writing real semantic tests against find_parallelizable_loops, found and fixed four correctness bugs in clustrix/loop_analysis.py: - DependencyAnalyzer.visit_AugAssign never counted `total` in `total += i` as a read (AugAssign targets are Store-only in the AST), so a plain reduction accumulator came back with zero dependencies and is_parallelizable=True. - detect_loops_in_function() didn't dedent inspect.getsource() output, so any function defined inside a class/closure (one indentation level deep) raised IndentationError, silently swallowed, always returning []. - detect_loops_in_function() called _analyze_for_loop/_analyze_while_loop directly via ast.walk() instead of detector.visit(tree), bypassing the current_level bookkeeping -- nested_level was -1 for every loop found via the public API, making find_parallelizable_loops's nesting-depth filter a no-op. - SafeRangeEvaluator couldn't evaluate a literal negative number (-1 is UnaryOp(USub, Constant(1)), not Constant(-1)), so range(10, 0, -1) always fell back to range_info=None. Also found clustrix/dependency_analysis.py's separate, exported LoopAnalyzer._is_loop_parallelizable() (public via clustrix.analyze_function_loops) only checked for break/continue/global despite documenting a "no shared mutable state" criterion -- it approved both the accumulator and shared-list-append patterns above. Fixed by reusing loop_analysis.DependencyAnalyzer instead of a second, weaker implementation. Added tests/unit/test_loop_analysis_semantics.py: real functions, no mocks, asserting on actual dependency/parallelizability values -- loop-carried deps, shared-state mutation, break/continue/return/for-else, nesting levels, enumerate/zip/dict.items() tuple-target blind spots, range() variants, and the comprehension-non-detection regression guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #89/#90: delete the flattening machinery; make local auto-parallel real The two TODOs #89 and #90 ask to implement (global-variable extraction in dependency_resolution.py, closure-variable arguments in function_flattening.py) live inside code that has never produced a runnable output for any input tried. The advanced flattener emits `import <name>` for hoisted helpers and builtins; the basic one dedents the body to column 0, drops `for` headers and prints instead of returning. Verified live on an ordinary nested-helper function: Generated flattened code did not execute: No module named 'helper' Generated flattened code did not execute: name 'i' is not defined Not flattening compute: advanced flattener produced no usable callable serialize_function/deserialize_function already handle every case flattening was meant to rescue. Round-tripped in a fresh interpreter with the defining module off sys.path: nested helper 45, deep nesting 65, module-level helper 19, closure 40, exec()-created 5, args+kwargs 21 -- all matching the direct call. decorator.py no longer reaches for either module. So: delete them. Removed clustrix/function_flattening.py (1027) and dependency_resolution.py (445), and the five test files that only ever tested them. Kept and re-pointed the tests that cover live behaviour: the GPU workflow simulation now proves the serializer round trip instead of flattening, the tensor01 and cluster GPU tests keep their real remote execution and lose only the complexity assertions. clustrix.dependency_analysis (the public analyze_function_dependencies) is a different module and is untouched. Also #120 item 2, the same defect class. _create_local_work_chunks injected `_parallel_<var>` into callees that never declared it, so every chunk raised TypeError, which _execute_local_parallel swallowed under a blanket except and converted into a silent sequential re-run -- auto_parallel never parallelized anything locally and said nothing useful. Chunks are now built only for a signature that can receive them, and TypeError on the parallel path propagates. scripts/check_for_secrets.py is a black-only reformat; it was not black-clean at HEAD and the mandated `black clustrix/ tests/ scripts/` run touches it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #127: add a changelog for the 0.2.0 release There was no CHANGELOG.md anywhere in the repository. This one records what actually changed, and keeps a standing 'Implemented but unverified' section so backends that have never been run against real hardware are never quietly listed as working. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #114: Fix Kubernetes/GCP tests asserting the old, buggy contract executor_kubernetes.py was rewritten so job status/result collection raise on unreadable outcomes instead of reporting fake success, and pod results are now a signed base64 payload (CLUSTRIX_RESULT_B64 + CLUSTRIX_RESULT_HMAC via decode_signed_result) instead of a bare "CLUSTRIX_RESULT:<repr>" string run through ast.literal_eval. Also, the #80 module refactor moved _get_k8s_result/_get_k8s_error_log/ _cleanup_k8s_job off ClusterExecutor onto executor.k8s_manager, and get_job_status() now requires active_jobs entries to carry a "manager" key. - tests/test_kubernetes_integration.py: run the real build_worker_program() worker as a subprocess to produce genuine signed pod-log output (no hand-written stand-ins for the retired format), call executor.k8s_manager.* where ClusterExecutor has no shortcut, and tag active_jobs entries with "manager": "kubernetes". Removed a dead patch("clustrix.executor.cloudpickle") left over from before cloudpickle usage moved into executor_kubernetes.py. - tests/test_cloud_providers_gcp_real.py: GCPProvider.list_instances(), .create_instance(), .is_valid_region()/.is_valid_zone() never existed (verified via `git log -S`) -- these assertions were against a fabricated API since the tests were added, unrelated to the recent rewrite. Rewritten against the real, currently-implemented API: list_clusters()/create_compute_instance() now raise "Not authenticated with GCP" instead of proceeding with a None client, and region/zone behavior is verified against the provider's actual defaults and its real (unauthenticated) get_available_regions()/ get_available_instance_types() fallback lists. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #116/#123: drop a call kept only so a test's patch would fire serialize_function contained: _ = get_environment_info() # For compatibility with tests Shipped code calling a function purely so that a mock in tests/test_integration.py would be hit, then discarding the result. It is the same anti-pattern as #116's isinstance(..., Mock) branches, and it cost a 'pip list' subprocess on every job submission to compute nothing. Faking that freeze output is also why the environment-replication bug survived so long: get_environment_requirements() was dropping 187 of 563 packages and every test that mocked the freeze step still passed. Also remove the guard that let the remote setup continue after failing to install dill and cloudpickle. The generated worker now refuses to fall back to stdlib pickle -- pickle serializes a function by qualified name and cannot resolve it in a fresh interpreter -- so swallowing that failure only moved the error to a later and far more confusing point. pip's own upgrade stays non-fatal: pip's version is not part of the replicated environment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #114: fix ProfileManager touching real ~/.clustrix, update stale widget assertions ProfileManager.__init__ hardcoded config_dir="~/.clustrix/profiles", ignoring CLUSTRIX_CONFIG_DIR. Every caller that constructs ProfileManager() with no explicit config_dir -- the widget's default and notebook_magic_core.py's default -- silently read and wrote a real user's ~/.clustrix, even under tests/conftest.py's isolate_config_dir fixture. On this machine that had accumulated 47+ "Current configuration (N)" profiles in ~/.clustrix/profiles/ profiles.yml. Default now resolves via clustrix.config.get_config_dir(), which honors CLUSTRIX_CONFIG_DIR. That fix alone did not make every failing assertion correct: several tests in test_modern_widget_comprehensive.py encoded a ProfileManager/widget shape that no longer matches the code (single default profile vs. one built-in template per backend; "clustrix.yml" vs. the deliberate "profiles.yml" default; "auto"/"~/.ssh/id_rsa" placeholders vs. the active profile's real ClusterConfig defaults of "pip"/None). Rewrote those assertions to match current, intentional behavior, and rewrote test_widget_initialization_with_mock_ipython (renamed to ..._with_real_ipython) to drive the real installed ipywidgets/IPython instead of the file's MockWidgets shim, per the no-mocking-the-thing-under- test rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #89/#90: replace the flattening design doc with a post-mortem The document described a system that has been deleted, and described it approvingly -- 'complexity-based triggering system works' was never true. Rather than delete it outright, record why the approach was abandoned, since the reasoning is the part worth keeping: it explains what would have to be true before anyone builds this again, and names the two questions the original never answered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Record the defects found but not fixed, and the polluted user config Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #114: Replace stale executor mock theatre with real tests 22 failures across the four executor test modules, all of them tests that had been left behind by the split of executor.py into executor_core / executor_connections / executor_schedulers / executor_kubernetes. Most were mock theatre in the sense of #117: they patched names the refactor moved (clustrix.executor.setup_remote_environment, clustrix.executor.cloudpickle, clustrix.executor.logger) so the patches were silent no-ops, replaced backward-compatibility aliases nothing calls any more (_execute_remote_command, _check_job_status, _submit_slurm_job), fed a Mock a canned string and asserted the string came back. None of them could fail for a real reason. Rather than re-point the mocks at the new call graph, these now exercise real code: * cluster_type="local" really runs the function, so submission, status, result collection, error logs and the active_jobs["manager"] routing are all checked against real execution; * create_job_script is pure, so each scheduler's directives are checked against real generated output; * where a real cluster would be needed, the assertion is on the error path -- a submission with no connection must raise and record no job, a failed cancellation must not drop the job from tracking; * the Kubernetes tests use the real kubernetes client against a real kubeconfig file on disk. Assertions deliberately changed, with the reason recorded in each docstring: * test_execute_command_not_connected expected "Not connected"; the shipped message is "SSH client not connected. ...". * test_get_job_status_completed/_failed hand-built an active_jobs entry with no "manager" key; get_job_status now dispatches on it. * test_get_result_success mocked SFTP into writing an unsigned pickle; results are HMAC-verified before deserialization now, so an unsigned one is refused by design. * test_parallel_job_submission passed timeout= to wait_for_result, which takes only a job ID. One test deleted rather than repaired: test_setup_kubernetes_cloud_manager_exception asserted that a Mock raising ImportError produced a log line. CloudProviderManager's constructor stores two attributes and cannot raise, and auto_configure catches its own exceptions, so that branch is unreachable without a mock. Verified: 59 passed, 2 skipped across the four modules (was 22 failed, 35 passed, 2 skipped); tests/unit 573 passed; black 26.3.1, flake8 7.3.0 and mypy all clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #114: Fix test_config_real, test_auth_fallbacks_real, test_credential_manager suites All 14 failures and 2 errors (actually 17 failures + 3 errors once the suite was run fresh) traced to tests written against field/function names that never existed or were renamed, plus fixtures scoped to the wrong test class. Two real bugs turned up in clustrix/auth_fallbacks.py along the way and are fixed here too: requires_password_fallback() crashed with AttributeError whenever a key-setup result explicitly set "error": None (the normal case from ssh_utils.setup_ssh_keys' success path), and get_cluster_password() could fall through to a real, blocking tkinter GUI prompt in this environment because `import clustrix` puts 'ipykernel' in sys.modules as a side effect, making detect_environment() always report "notebook". - tests/test_config_real.py: moved temp_config_dir fixture to module scope (TestConfigurationWorkflows couldn't see the class-scoped one); renamed "partition"->"default_partition", "namespace"->"k8s_namespace", "private_key_path"->"key_file" throughout (fields that were never named that); dropped assertions/kwargs for fields that never existed on ClusterConfig at all (gpu, cleanup_on_failure, node_selector, tolerations, service_account, image_pull_secrets, k8s_project_id, k8s_zone, k8s_gpu_type/count, k8s_preemptible, k8s_autoscaling, k8s_min/max_nodes, account, qos); rewrote test_configuration_precedence, which relied on a CLUSTRIX_DEFAULT_CORES env var override that has no implementation anywhere in config.py. - tests/test_auth_fallbacks_real.py: moved temp_credentials_dir fixture to module scope; rewrote every test that called requires_password_fallback() with a ClusterConfig instead of the Dict[str, Any] key-setup-result it actually takes; fixed get_cluster_password()'s hostname= kwarg name and get_password_gui()/get_password_widget()'s single-prompt-arg signature; gave setup_auth_with_fallback() a real (non-mock) setup_ssh_keys_func callable instead of calling it with the wrong arity; skip the GUI test outside an interactive terminal (mirrors the existing CLI test's skip) since this machine's tkinter would otherwise open a real blocking dialog; rewrote test_secure_password_handling to exercise the real secret redaction on save_to_file() rather than the non-existent repr masking it originally asserted. - tests/test_credential_manager.py: 1Password was removed in Issue #97 ("use only .env, environment vars, and GitHub secrets"), so FlexibleCredentialManager has 3 sources, not the 4 these tests still asserted; fixed the default-location test to compare against get_config_dir() instead of a hardcoded ~/.clustrix, since conftest.py's session-scoped isolate_config_dir fixture deliberately redirects CLUSTRIX_CONFIG_DIR for the whole test run. - clustrix/auth_fallbacks.py: requires_password_fallback() now treats a present-but-None "error" key the same as an absent one. No test in either file touches the developer's real ~/.clustrix (verified via `find ~/.clustrix -newermt` before and after every run); clustrix/ config.py was left untouched per the file-ownership boundary for this issue -- see the sweep report for the config.py defects found instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #114: unblock the secret scan; drop a dead expression statement tests/test_auth_fallbacks_real.py used password="irrelevant-because-key-already-works" twice, which check_for_secrets correctly reports as an assigned credential -- the scanner cannot know the value is a stand-in, and the CI security job would have failed on it. Renamed to a value the scanner's existing fixture vocabulary recognises, rather than teaching it a suppression marker that could later hide a real secret. executor_schedulers.py carried a second docstring-shaped string in the middle of the class body, left over from a refactor. The class already has a real docstring, so this one was a dead expression statement. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #111/#125: stop repr() leaking credentials; correct the precedence docs ClusterConfig's dataclass-generated __repr__ printed every field verbatim, so a password, API key or HF token landed in any traceback, log line or notebook cell that displayed a config: 'hunter2-real' leaks: True 'hf_realtoken' leaks: True 'sk-realkey' leaks: True save_to_file already refused to write those in plaintext; showing them on screen instead was barely an improvement. They are masked as '***' rather than omitted, so it stays visible that a value is set, and environment_variables is masked per entry on the same rule that governs saving -- OMP_NUM_THREADS stays readable, AWS_SECRET_ACCESS_KEY does not. CLAUDE.md's configuration-priority list named 'environment variables' as a level. No such level exists: nothing reads a CLUSTRIX_<FIELD> variable. Only CLUSTRIX_CONFIG_DIR (where files live) and whatever password_env_var names (a password, nothing else) are consulted. That gap matters more now that saved configs omit secrets by default, so the corrected text says plainly that password_env_var is currently the only supported way to supply a credential without writing it to disk. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #114: Fix 18 failures across 5 real-execution test files tests/comprehensive/test_edge_cases_real.py (7 failures): - test_lambda_function_serialization: dill (via _dumps_by_value) has always been able to serialize lambdas; the old assumption that this should raise was simply wrong. Rewritten to assert the real, correct behavior for both local execution and serialize_function(). - test_zero_resource_request: `with pytest.raises(ValueError) or True:` was always equivalent to `with pytest.raises(ValueError):` (the context manager is truthy), while an inner try/except swallowed any ValueError before it reached that outer manager -- the block could never satisfy pytest.raises. cores is unvalidated on the local direct-call path this test exercises, so real behavior is success. - test_connection_timeout: `connection_timeout` is not a real ClusterConfig field (real field: ssh_connect_timeout, default 30s), and ClusterExecutor.connect() never passes a timeout to paramiko's connect() at all (see defect note below), so the old test could hang well past its own 10s assertion. Uses socket.setdefaulttimeout() -- a real, non-mocked timeout bound on the actual TCP attempt. - test_intermittent_connection: NoValidConnectionsError is an OSError but not a ConnectionError, so `except ConnectionError` never caught it; broadened to OSError. auto_gpu_parallel=False sidesteps a real defect (see below). - test_parallel_job_limits: max_parallel_jobs throttles clustrix's own remote/cloud submission loop; it cannot and does not throttle a caller's own ThreadPoolExecutor, and local direct-call execution never queues these as "jobs" at all. Rewritten to check correctness under real concurrency instead of an artificial serialization timing bound the code never promised. - test_cleanup_after_failure: cleanup_on_failure is not a real ClusterConfig field (configure() raised ValueError). Removed. - test_comprehensive_edge_case_suite: self-healed once the above were fixed (it re-runs every method above directly). tests/comprehensive/test_failure_recovery_real.py (4 failures): - test_ssh_connection_drop_recovery: connection_retry_count/ connection_retry_delay are not real ClusterConfig fields. Removed; added auto_gpu_parallel=False (see defect below) and broadened the except clause to OSError. - test_network_timeout_recovery: network_timeout/retry_on_timeout/ max_retries are not real ClusterConfig fields (no such retry configuration exists in clustrix). Removed. - test_cluster_unavailable_recovery: connection_timeout/ fallback_to_local are not real ClusterConfig fields. A '.invalid' host always raises socket.gaierror, an OSError but not a ConnectionError/TimeoutError, so the old except clause never caught it. Rewritten with pytest.raises(OSError). - test_out_of_memory_recovery: np.zeros(100GB) does not reliably raise MemoryError on a real modern machine (zero pages can be lazily committed/overcommitted) -- confirmed empirically. Bumped to an allocation numpy itself refuses regardless of physical RAM. tests/test_reference_workflows.py (3 failures): - Importing test_*-named functions from tests/reference_workflows/*.py made pytest auto-collect them as extra, unguarded top-level tests (python_functions="test_*" matches by name in module globals, regardless of where a callable was defined), bypassing the SLURM_TEST_ENABLED/K8S_TEST_ENABLED skipif guards on the intended wrapper methods. test_basic_data_analysis_workflow in particular ran for real against a hardcoded fake SLURM host on every `pytest tests/`. Fixed by importing under non-"test_"-prefixed aliases. tests/test_integration.py (3 failures): - test_end_to_end_simple_function, test_error_handling_integration: result.pkl/error.pkl are now HMAC-signed with a per-job key (#121) and verified before being unpickled; these tests fabricate the pickle files directly (paramiko itself is mocked) and must sign them the same way the real worker does. Pins secrets.token_hex(32) to a known key and computes the matching HMAC tag for the mocked `cat *.hmac` response. - test_environment_replication: rewritten per review feedback to exercise get_environment_requirements()/get_unreproducible_ requirements() against this interpreter's REAL installed packages, rather than mocking the now-deleted get_environment_info() call. Mocking the freeze step is exactly how the 187/563-package silent environment-replication bug went undetected for so long. tests/test_decorator_real.py (1 failure): - test_async_execution: AsyncJobResult's real API is get_result()/get_status(), not a `.result` future-like attribute; `hasattr(job_result, "result")` was always False, so the test treated the handle object itself as the answer. Fixed to use the real API. Real defect found in clustrix/executor_connections.py (reported, not fixed -- out of scope, owned by another file): setup_ssh_connection() leaves self.ssh_client set to the constructed- but-never-connected paramiko.SSHClient() when .connect() fails, instead of resetting it to None. execute_remote_command() only checks `self.ssh_client is None`, so a later call on the same executor (e.g. the GPU-detection probe that runs before a real submission) calls exec_command() on a dead client and raises AttributeError: 'NoneType' object has no attribute 'open_session' deep in paramiko, instead of a clean connection error. Separately, setup_ssh_connection() never passes a `timeout` to paramiko's SSHClient.connect() at all, so ssh_connect_timeout is not honored on this path (only filesystem.py's ClusterFilesystem passes it). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #114: make tests independent of each other Eleven widget tests passed alone and failed in a full run. Three separate leaks, each hiding the next: 1. tests/conftest.py's reset_config restored eight hand-listed fields. ClusterConfig has over a hundred, so k8s_namespace, remote_work_dir, package_manager, environment_variables and the rest leaked into every later test. It now snapshots every field by name, so a newly added field is covered automatically instead of silently joining the set of things that leak. 2. It restored fields on the singleton but not the module binding, so a test that rebound clustrix.config._config to a different object left the module pointing at its own. Both are now restored. 3. TestClusterConfigReal defined its own reset_config fixture, which SHADOWED the autouse one -- and only restored the binding, so configure()'s in-place mutations survived it entirely. Removed; those tests now get the conftest fixture, which actually works. This was the one that mattered: with it in place, the live config after that file ran still carried 20 drifted fields, and the widget reads the live config to decide which profile is active. isolate_config_dir is function-scoped rather than session-scoped for the same reason: one shared directory stops the suite writing into the developer's real ~/.clustrix, but still lets a profiles.yml written by one test change what a later test sees. test_error_handling_integration drove a mocked SSH client that answered bSuccess to every command, including /Users/jmanning -- so the remote home directory resolved to the string Success and the test failed on that rather than on error handling. Rewritten against cluster_type=local, which is a real backend, plus a second test asserting the exception TYPE survives and not merely the message. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Issue #114: reset the credential-manager singleton between tests too get_credential_manager() caches a FlexibleCredentialManager on first use, and that manager resolves the config directory at construction. With a per-test config directory, one built during an earlier test hands a stale path to every test after it -- which is why test_get_credential_manager_default_location passed alone and failed inside its own file. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU * Docs: honest cloud/Kubernetes/HuggingFace Jobs tutorials Kubernetes, AWS, Azure, GCP and Lambda Cloud VM execution have never been run end to end; HuggingFace Jobs (cluster_type="huggingface") has. The tutorials didn't reflect that gap and documented several parameters the code silently ignores or never implemented. - kubernetes_tutorial.rst: fix a per-job @cluster(k8s_namespace=..., k8s_image=...) example that the executor never reads (only cores/memory apply per job); add a "Behind the Scenes" section tracing submit_k8s_job -> build_worker_program -> HMAC-signed result -> decode_signed_result, and the status-polling fix that used to report unreadable jobs as "completed"; soften the closing paragraph so it doesn't read as a report of a successful run. - kubernetes_tutorial.ipynb: full rewrite. The previous version documented cpu_limit, memory_limit, container_image, job_name, parallelism, completions, restart_policy on @cluster(...) -- none of which exist in the decorator or the Kubernetes executor. Replaced with only the parameters that are actually read, plus the same behind-the-scenes and unverified-backend framing as the .rst tutorial. - huggingface_spaces_tutorial.ipynb: added a real Part 1 tutorial for the verified HF Jobs backend (payload staging over the 256KB env-var limit, bootstrap package installs, the per-job HMAC key popped from the environment before pip install and user code run, the GPU-flavor cost gate). The original content, which is about HF Spaces web-app hosting and never exercises HF Jobs, is kept as an explicitly separate, still-unverified Part 2. - aws/azure/gcp/lambda_cloud_tutorial.ipynb: added a note explaining that only LambdaCloudProvider implements create_instance(), so @cluster(provider="aws"/"azure"/"gcp", ...) raises NotImplementedError naming the provider at submit time; the examples instead provision a VM with the provider's own tools and point cluster_type="ssh"/"slurm" at it. Also documents the fixed placeholder-hostname bug (a VM with no resolvable host used to return a fake "placeholder.*.com" instead of raising) and, for AWS specifically, that this particular fix was not applied there. - Stripped a handful of markdown cells across four of these notebooks that carried a stray "outputs" key left over from an old conversion, which failed nbformat.validate() (though not nbformat.read()). scripts/check_docs_examples.py: 36 block(s) checked, 36 passed, 0 failed. sphinx build: build succeeded (no warnings from any file in this commit). * Docs: add introduction and quickstart, restructure index and installation The documentation had no front door. index.rst opened with a feature bullet list and a SLURM example that needed credentials before it could do anything, and there was no page explaining what Clustrix is, what it deliberately is not, or when to reach for Dask/Ray/joblib/sbatch instead. - introduction.rst (new): the problem the library solves, how execution works in one paragraph, five "what it is not" boundaries, honest comparisons against hand-written sbatch, Dask, Ray, joblib and plain SSH+rsync, and a "when this is the wrong tool" list. Corrects the old false claim that REPL-defined functions cannot be serialized: they can, only the source-reading features (loop parallelization, GPU-parallel detection) need inspect.getsource(). - quickstart.rst (new): eight self-contained use cases. Steps 1-4 and 8 run with no cluster at all (cluster_type="local"); steps 5-7 cover the three verified remote backends and are marked "# cluster-required". Covers host-key rejection, the paid-GPU-flavor guard, 0600 config files with secrets omitted, and password_env_var as the one supported way to keep a credential off disk. - index.rst: toctree restructured into Getting Started / User Guide, with a runnable local example at the top. The backend-status table is unchanged. - installation.rst: Python 3.8 -> 3.10, verification example now uses cluster_type="local", cloud extras documented with the warning that they do not give you a working cloud execution backend. Verified: python scripts/check_docs_examples.py -> 36 passed, 0 failed. The 12 blocks on these four pages were checked with the same harness (11 executed for real, 3 statically verified) -- they are not in the script's target list, which lives under scripts/ and is out of scope for this change. * Docs: explain scheduler/SSH mechanics and fix fabricated examples slurm_tutorial.rst, pbs_tutorial.rst, ssh_setup.rst: add "what Clustrix is doing" sections covering the full submit/poll/verify pipeline, the exact generated job script per backend, config precedence, and failure modes. Fix pbs_tutorial.rst's stale claim that PBS skips the two-venv setup path (it doesn't anymore -- all four schedulers share job_execution_lines()). Add a dedicated SSH host-key-verification section to ssh_setup.rst, since an unrecognized host key now rejects the connection by default. slurm/pbs/sge notebooks: fix examples that passed fabricated @cluster keyword arguments (array=, gres=, pbs_array=, walltime=, features=, pe=, sge_array=) which are silently dropped -- none of them reach the generated job script. The PBS/SGE "array" examples additionally read a scheduler environment variable (PBS_ARRAYID/SGE_TASK_ID) that clustrix never sets, so they would have silently run task 1 every time; replaced with an explicit-argument + Python-loop + async_submit driver pattern (async_submit has to be set on the decorator, not passed per-call -- fixed after first getting that wrong too). Add unverified-hardware warnings to the PBS/SGE notebooks and behind-the-scenes cells to all five touched notebooks. ssh_tutorial.ipynb: fix a ClusterConfig(port=...) call that would raise TypeError -- the real field is cluster_port. Also assigns proper cell ids throughout (previously missing on…
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.
What this is
Before this branch,
@clusterhad never successfully executed a function onany real backend. Not "worked sometimes" — never. The seam between the caller
and the worker was broken in several independent places at once, and the test
suite could not see any of it because it mocked exactly the parts that
mattered.
Eighteen use cases now return the correct answer on five backends.
Part of #108.
Evidence
Run it:
python scripts/verify_cluster_usecases.py. For every case it printsthe configuration used (secrets redacted), the function as written, the answer
computed locally by calling the undecorated function, and the answer the
cluster returned.
Full transcript:
docs/evidence/usecase-matrix.txt.A matching value is not enough on its own. If the decorator quietly ran the
function locally, every comparison would pass. So each target begins with a
provenancecase whose answer is the worker's own identity, and the target isabandoned unless the host, PID and architecture all differ:
That guard was added because red-teaming found the harness would report
9/9 correct with
@clusterreplaced by an identity decorator. Nothing in itcould tell local from remote. An unreachable target is reported as skipped and
exits non-zero, so a fully-skipped run cannot be mistaken for a passing one.
The cases are chosen to fail: closures, module-level globals and helpers,
user-defined classes crossing the wire in both directions, a package that lives
only in the caller's working tree, a library outside any whitelist, disk I/O,
100k-element arguments, 50k-element returns,
None, keyword-only arguments,and an exception that must arrive with its original type.
Why it never worked
re-serialized for VENV2 with
pickle, which stores functions by qualifiedname. Every
__main__function failed withattribute lookup f on __main__.python=3.9regardless of thecaller, while
local_python_versionwas computed and never used. dill embedsCPython bytecode →
RuntimeError: unknown opcode.remote_work_dirdefaulted to/tmp/clustrix, node-local on SLURM. Jobsdied at exit 127 before writing any diagnostic.
conda.shsourced, not merely onPATH.the VPN was judged to be
discovery.dartmouth.edu.What the function could not take with it
Fixing the seam exposed a second class of failure: the payload arrived, and
then could not run.
dill.dumps(func)capturesclosure cells but not
func.__globals__, so a function calling amodule-level helper serialized cleanly and died with
NameError: name '_helper' is not defined.name, so passing an instance of your own class failed with
Can't get attribute 'Point'.reference — right for numpy, wrong for
mypkg.utils, which exists on nomachine but yours. The tell was the size: 406 bytes for a function carrying a
whole module's behaviour. Project-local modules are now embedded by value;
installed packages and the standard library are not.
the setup function's own docstring promising it "replicates the local
environment". It now mirrors whatever the local package manager reports —
pip's freeze, which also covers conda environments, or uv's — with
replicate_local_environmentandexcluded_packagesas the manual overrides.{'error': str(e)}and discarded the exception, so
except ValueErrornever fired.the caller read with stdlib pickle, which replays dill's opcodes but rebuilds
a fresh class — so a returned instance failed
isinstance()against thevery class that defined it, while its repr looked perfect.
HuggingFace Jobs (#118)
cluster_type="huggingface"runs functions in a container — no reservation, noVPN, no institutional SSH credentials. Payloads too large for an environment
variable are staged through a private dataset and cleaned up afterwards, so a
100k-element argument works there too. GPU flavors are gated by prefix rather
than a name list, because a denylist fails open on new hardware.
A function that raises is no longer reported as a failed job: the container
records the error and exits cleanly, so an ordinary
ValueErrorstops emailingthe account owner "status changed to ERROR".
Security (#121)
Loading a pickle executes code, so
result.pkl— fetched from a remote host —was a remote-to-local code execution path on every backend. Results are now
HMAC-verified before deserialization. Demonstrated by overwriting a finished
job's
result.pklin place on tensor01:Red-teaming that mechanism found three more, all fixed: the key was passed on
the remote command line (readable via
ps); an uncheckedchmod 700on apredictable job directory allowed key theft and therefore code execution on
the submitting machine; and two jobs submitted in the same second overwrote
each other's key.
Notebook widget
Design canvas: https://claude.ai/code/artifact/3fd2edfb-d391-423e-9d12-37695332ac7d
Screenshots:
docs/evidence/widget/Rebuilt to inherit JupyterLab's theme tokens, so it follows light/dark and no
longer pulls a font from Google. Beyond that, most of the profile row did not
work:
Anything typed lived nowhere but the controls, so switching away discarded it.
a copy of the defaults — which is why switching between them looked dead.
configure(cluster_type="huggingface", …)still opened on "Localsingle-core", and Apply would then overwrite the real configuration.
cluster_hostbehind; replacing wholesale discarded settings with no controlin the widget. It now resets exactly the fields it manages.
templates ship instead of one.
job failed to authenticate.
local, the default —ClusterExecutorhas nolocal backend.
Test status
tests/unit/+ widgetclustrix/Also fixed in CI: Quick Checks could never have passed — it installed a
hand-listed
black flake8 mypy pytestrather than the dev extra, pulling anunpinned black and omitting
pytest-timeoutwhile the test step passes--timeout=60. And a test added on this branch was stalling the macOS andWindows runners for 70 seconds a call, because the Dartmouth network check did
unbounded DNS lookups.
Took the high-severity
blackadvisory (GHSA, arbitrary file writes via thecache filename) and pinned 26.3.1, which exposed two pre-commit hooks running
different black versions and rewriting each other's output. The alert stays
visible on
masteruntil this merges.The "Check for secrets" job could not pass either: it grepped for the words
password|secret|key|token, flagging 1138 lines includingkwargs: Function keyword arguments— and would have missed a real token,which contains none of those words. Replaced with a check that looks for
provider token formats, PEM blocks carrying an actual body, and credentials
assigned as literals, with 23 tests covering both what it must catch and what
it must ignore.
Corrections to the original plan
Two things in #108 did not survive contact:
importer;
vulture --min-confidence 90finds 13 items package-wide, of which2 were real unused imports (removed).
requires-python = ">=3.8"was never true. Four hard dependencies require≥3.10. Corrected in
pyproject.toml,setup.pyand the CI matrices.What is still unverified
KeyErroron their first line is fixed, but reaching that code needscredentials and provisioned instances that were not exercised.
correct where PBS's previously ran a file that has never existed — a different
claim from "tested".
memory quantities are valid.
has already failed, and emits a parameterless script, so any function using
its own arguments yields
name 'n' is not defined. It then falls back toordinary serialization and the job runs.
Re-baselining it is Fix the 127 test failures and 8 collection errors that CI never sees #114.
Reproducing
pip install -e ".[dev]" python scripts/verify_cluster_usecases.pyDartmouth hosts are split-DNS internal names and need the VPN; without it they
report as skipped, never as passing. HF Jobs needs
HF_TOKENand a namespace ona plan that can run jobs.
🤖 Generated with Claude Code
https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU