[adil-a]

What does this PR do?

M9 of the Harbor tasks RFC (GitLab MR 340, 354, 367): datasets that live in Harbor's package store. Stacked on #3730.

• harbor:<org>/<name>[@<tag> | @sha256:<digest>]. nemo_gym/tasks/harbor/package_store.py resolves the dataset version through Harbor's public PostgREST API (tag or digest), lists its task packages, downloads each archive, checks it against Gym's task digest (the same record format Harbor publishes) and writes manifest.toml. A folder whose digest already matches is kept, so a rerun completes a partial fetch. harbor:terminal-bench/terminal-bench@4.0.0 materializes all 66 Terminal Bench 4 tasks into datasets/terminal-bench-4.0.0/.
• Task model. The Harbor 1.4 fields those packages use: [[verifier.collect]] hooks and typed artifacts entries (string or table, with host_path). Artifact sources and destinations may not contain .., and a destination must be relative.
• CLI. The taskset name is the fetched folder's name for both registry and package-store references.

Review fixes (2026-10-04)

• Task names from registry.json and from package-store replies pass validate_task_name before they become path components. Empty names, ., .., anything containing .., a path separator or NUL are refused. Characters real Harbor task names use, such as $, are accepted.
• contained_path checks that a folder resolves strictly inside the dataset folder before a fetch removes or replaces it. A spoofed name or a symlink cannot make a fetch touch files outside the dataset.
• A task folder whose digest differs from the store (edited locally, or republished) now stops the prepare step with both digests named instead of being replaced silently. The error says to keep the local copy, or delete the folder (or move it aside) and run again, which fetches the store's copy. There is no flag for this: discarding local edits stays a step the user takes by hand.

Validation

• pytest tests/unit_tests/test_harbor_package_store.py tests/unit_tests/test_harbor_tasks.py tests/unit_tests/test_cli_main.py environment_servers/harbor/tests: pass. TestPackageStore (tests/unit_tests/test_harbor_package_store.py) fakes the store at the HTTP layer: tag and digest resolution, the archive check against the published hash and the manifest, a rerun without downloads, the exclusion-free archive hash next to the folder digest (a package that ships a name the folder digest skips, with and without run noise), a spoofed package name refused before any filesystem use, nothing deleted outside the dataset folder, linked dataset folders and manifests refused, a folder without task.toml left alone, duplicate task names refused before writing, an edited folder stopping the fetch with both digests named, a deleted folder fetched again, the [dataset] identity check (a folder fetched for another package, a moved latest refused with both hashes), two concurrent fetches keeping the completed copy and a failed rename leaving no partial destination, an unreadable archive reported as a one-line error by gym eval run and gym dataset validate, an unreadable manifest rewritten only after the task checks, folder naming and fetch_ref dispatch. TestTaskNames covers refused and accepted names, and a registry entry with an unsafe name is refused. TestSeparateVerifierFields covers collect hooks, artifacts and artifact path containment.
• There is no --force flag. A folder whose digest differs from the store stops the fetch, and the documented recovery is to keep the local copy, or delete the folder (or move it aside) and run again, which fetches the store's copy.
• Real fetch (before the rebase): all 66 terminal-bench/terminal-bench@4.0.0 packages downloaded and every digest matched the store's content hash.
• pre-commit run --files <changed files> clean.

Rollouts

The 52 CPU tasks of the fetched dataset ran through gym dataset validate on OpenSandbox Cell-3 in the M12 PR of this stack (#3746). Those runs predate the rebase below.

Rebased onto main

Rebased onto main 128eab4 (2026-10-04).

Compatibility and benchmark impact

Registry references and local folders are unchanged, apart from the task-name check on registry.json entries. Package-store access uses Harbor's public read-only key. No credentials are needed.

Checklist

• I have read the contributing guidelines.
• The change is focused; unrelated "drive-by" edits are tracked as separate issues/PRs.
• Tests added or updated and pass locally, or N/A for docs-only / non-code changes (so CI unit/server checks pass when applicable).
• Documentation added or updated, or N/A with justification.
• Pre-commit checks pass locally (pre-commit run --all-files) (so CI lint/format checks pass).
• New source files include the required Apache-2.0 SPDX header.
• All commits have DCO sign-off (git commit -s) (so the DCO check passes).

🤖 Generated with Claude Code

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[yuhezhang-ai]

Before merging, please address the four cases below involving protection of local edits, dataset cache identity, duplicate task destinations, and compatibility with Harbor's published file hashes. Each has a local reproduction; they can erase local work, change the requested task set, or reject an intact package. The CLI-identifier and concurrent-fetch comments are nonblocking follow-ups.

Reviewed the M9 increment against adasif/harbor-m8-tb21. Focused tests and reproductions ran on 00c97bf4; after the rebase, I verified both M9 patches and the finding locations are unchanged at 2bdcada5. The rebased tree was not retested, and no live-store fetch or model rollout was performed during this review.

[adil-a]

/ok to test bdbe918

[yuhezhang-ai]

Thanks for the fixes. I've verified and resolved the folder-preservation, duplicate-name, and symlink threads.

The remaining threads still need attention: dataset cache identity, published-file hashing, concurrent fetches, and CLI errors. On bdbe9182, a fetch finishing just after the new check can still make preparation omit a task, and run_target still lacks the ValueError catch mentioned in the reply.

[yuhezhang-ai]

Nonblocking: update the validation section. It still lists --force replacement, flag-default tests, and forwarding force through prepare. At bdbe9182, those commands have no --force flag and those tests are absent. Please refresh the validation list to match the current branch and the documented move/delete recovery flow.

[adil-a]

/ok to test 5b4eb8b

[github-actions]

🌿 Preview your docs: https://nvidia-preview-pr-3743-1bc58a7388c8.docs.buildwithfern.com/nemo/gym

[adil-a]

/ok to test 9d489d6

[adil-a]

/ok to test 06cb0da

[adil-a]

/ok to test 92a5e8f

[adil-a]

Status at the new head 92a5e8f. The four remaining threads each have the fix inline:

• dataset cache identity: folder names stay readable and the identity lives in manifest.toml's [dataset] table (package, reference, resolved hash), read back before any task is touched, so alice/evals and bob/evals cannot share a folder and a moved latest is refused with both hashes (_check_recorded_identity).
• published-file hashing: the archive is checked with an exclusion-free archive_hash, the same record format over every file, so an intact package always matches the store, while Gym's folder digest with exclusions is what reruns compare against, recorded in the manifest.
• concurrent fetches: promotion is one os.rename of a sibling staging folder, a rename refused by a peer's complete copy keeps that copy, and a failed rename leaves no partial destination (test_peer_copy_promoted_after_check_is_kept, test_failed_rename_leaves_no_partial_destination).
• CLI errors: a bad archive body becomes a PackageStoreError, and run_target catches ValueError around preparation, so eval run and dataset validate print one parser error line.

You are right that the validation section was stale. I am replacing it in the description to match the branch: there is no --force flag anywhere, the recovery flow is to keep the local copy or delete the folder (or move it aside) and fetch again, and the test list names what tests/unit_tests/test_harbor_package_store.py actually holds.

[yuhezhang-ai]

Thanks for the fixes. The fixed threads are resolved. Dataset cache identity still needs addressing before merge. Added one new nonblocking finding below for interrupted HTTP downloads.

[adil-a]

/ok to test 58723ba

[adil-a]

/ok to test aaed18f

[adil-a]

/ok to test 0306fc7

[yuhezhang-ai]

Thanks for the fixes. Approved for the planned Harbor stack, including #4664 (5df89d45), where I verified the remaining cache-identity fix. The interrupted-download error is fixed in M9.

Focused tests and reproductions pass, as do the main CI checks. The optional, nonblocking harness-conformance check remains failing.

[adil-a]

/ok to test d5ff29a

[ffrujeri]

Approving d5ff29a9. The cache-identity fix now lives in M9 itself: fetch_package_dataset claims the folder through an exclusive manifest.toml create before the first download, records each task as it lands, and refuses an unreadable or foreign manifest. An interrupted fetch can no longer leave a folder another dataset reuses. The interrupted-download error and the other threads are fixed too. Package-store, Harbor, validation, CLI, config, sandbox, sessions and rollout-collection suites pass locally at this head (1514 passed, 13 skipped); CI was still running at approval time.

[adil-a]

/ok to test bcee436

[yuhezhang-ai]

Rechecked after the rebase: the reviewed M9 changes are unchanged. Approved for the planned Harbor stack, including #4664 (66bcacd0), which contains the verified remaining cache-recovery fixes. Main CI passes; the optional harness-conformance checks remain nonblocking.

[adil-a]

/ok to test 1bc58a7

[ffrujeri]

Re-approving 1bc58a73 after the restack. git range-diff shows the patch is identical to the d5ff29a9 I approved.

[github-actions]

🔄 Merge queue validation started! You can track the progress here: https://github.com/NVIDIA-NeMo/Gym/actions/runs/39680942488