Skip to content

fix(science): skip comments and blank lines in pinned requirements - #813

Merged
Ishaan Gangwani (ishaan1124) merged 2 commits into
synthetic-sciences:mainfrom
aniruddhaadak80:fix/requirements-comment-lines
Sep 29, 2026
Merged

Ishaan Gangwani (ishaan1124) merged 2 commits into
synthetic-sciences:mainfrom
aniruddhaadak80:fix/requirements-comment-lines

Conversation

@aniruddhaadak80

Copy link
Copy Markdown
Contributor

What

The hashed-requirements parser treated every line as a pin:

return requirements.split("\n").map((line) => {
  const pin = line.trim().split(/\s+/u)[0]!
  const offset = pin.indexOf("==")
  return { pin, name: pin.slice(0, offset), version: pin.slice(offset + 2), hashes: [...] }
})

A comment line becomes pin: "#", offset: -1, so name: "" and version: "h". A blank line becomes pin: "", version: "n".

That then fails its own coverage check in verifiedWheels:

const expected = requirementArtifacts(requirements)
if (expected.length !== pins.length || expected.some((item) => !pins.includes(item.pin) || item.hashes.length === 0))
  return undefined

so ensureWheelArchives treats the wheel cache as unverified, re-downloads on every call, and finally throws "Downloaded wheels do not exactly cover the hashed task requirements".

Why it matters

TaskSpec.pip_requirements is agent- or author-supplied and validated only as z.string().trim().min(1).max(100_000) — unlike capability/schema.ts:154, which does require every non-blank line to carry --hash=sha256:.

So a completely ordinary requirements.txt — a single # pinned by release 2.0 header, or a blank line between sections — can never produce a managed task environment. The cache is written but never accepted, and the error blames wheel coverage rather than the comment line.

It fails loudly, so this is a parser defect rather than a silent research-corruption bug, but it blocks a legitimate input outright.

Verification

requirementArtifacts was module-private and every test in environment-manager.test.ts that reaches this path is test.skipIf(!capabilityPlatform()), so nothing covered it. This exports it and adds a direct test.

To show the parser defect independently of the export, I ran the new test against the original logic with only the export keyword added:

(pass) requirementArtifacts > reads a name, version and hashes from each pin
(fail) requirementArtifacts > ignores comments and blank lines      <- Expected 2, Received 5
(pass) requirementArtifacts > trims indentation on a pin
(pass) requirementArtifacts > keeps every hash on a pin and returns nothing for empty input
 3 pass  1 fail

With the fix:

(pass) requirementArtifacts > reads a name, version and hashes from each pin
(pass) requirementArtifacts > ignores comments and blank lines
(pass) requirementArtifacts > trims indentation on a pin
(pass) requirementArtifacts > keeps every hash on a pin and returns nothing for empty input
 4 pass
 0 fail

The fixture is a five-line file (comment, pin, blank, pin, whitespace) so it pins both the comment and blank-line case, and the last test keeps multi-hash pins and the empty-input guard working.

The change

-function requirementArtifacts(requirements: string) {
+export function requirementArtifacts(requirements: string) {
   if (!requirements.trim()) return []
-  return requirements.split("\n").map((line) => {
-    const pin = line.trim().split(/\s+/u)[0]!
-    const offset = pin.indexOf("==")
-    return {
-      pin,
-      name: pin.slice(0, offset),
-      version: pin.slice(offset + 2),
-      hashes: [...line.matchAll(/--hash=sha256:([a-f0-9]{64})/gu)].map((match) => match[1]!),
-    }
-  })
+  // A comment or a blank line is not a pin. Counting one as a pin made the
+  // exact coverage check in verifiedWheels fail its own length test, so a
+  // requirements file with an ordinary header could never be installed.
+  return requirements
+    .split("\n")
+    .map((line) => line.trim())
+    .filter((line) => line && !line.startsWith("#"))
+    .map((line) => { /* unchanged */ })
 }

The only other reference to the symbol is verifiedWheels in the same file, and it keeps the same shape and the same return type — only the count changes.

bun run typecheck clean; touched files are Prettier-clean (checked on LF-normalized copies — this checkout has core.autocrlf=true, which makes Prettier flag every file in the repo).

Fixes #812

@vercel

vercel Bot commented Sep 28, 2026

Copy link
Copy Markdown

ANIRUDDHA ADAK (@aniruddhaadak80) is attempting to deploy a commit to the InkVell Team on Vercel.

A member of the Team first needs to authorize it.

Every line became a pin, so a comment produced a pin of "#" with no
version and a blank line one of "". The exact coverage check in
verifiedWheels compares the parsed count against the requested pins, so a
requirements file with an ordinary header failed to install and the task
environment was never built.
@ishaan1124
Ishaan Gangwani (ishaan1124) merged commit 2df9efc into synthetic-sciences:main Sep 29, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A comment or blank line in pip requirements makes a managed task environment uninstallable

2 participants