Report the task directives read while the command is rendered

[bentsherman]

Rendering the task command reads the directives it interpolates off the task config; record those reads so that an executor adjusting the resources at schedule time can tell whether the rendered command carries a value it is about to change.

Being observed rather than inferred, this covers the script, a shell block, a template file and a dynamic directive value the command interpolates, in both parsers, without any of them being known to the check. A directive resolved after the command has been rendered is not covered.

nf-seqera submits the affected tasks with prediction model none.

[netlify]

✅ Deploy Preview for nextflow-docs canceled.

Name	Link
🔨 Latest commit	e08759c
🔍 Latest deploy log	https://app.netlify.com/projects/nextflow-docs/deploys/6a86b4090792ac00081d5738

[pditommaso]

Reviewed b148cfbaf..f7bb8a80c with test runs and a few throwaway probes. I like this approach a lot — observing the reads is both smaller and more precise than collecting references statically, and it picks up the config ext.args idiom for free.

Two subtle details are already right, and both would have been easy to miss: recording happens before the cache lookup (TaskConfig.groovy:168), and ext isn't masked by the cache since TaskConfig#cache holds the LazyMap rather than the resolved value. I confirmed both with probes, along with no false positives on memory (a plain echo hello script records nothing). I also suspected a thread-visibility race between the rendering and submitter threads, but it's fine — TaskPollingMonitor#schedule hands off through a LinkedBlockingQueue, which gives the happens-before edge. All existing tests pass.

One thing I'd fix before merge — clone() drops the reads but keeps the rendered script.

TaskConfig#clone() calls newCache(), which now also resets reads, while TaskRun#clone() is a shallow copy that carries script over. So a copied task has the memory-baked command and an empty set:

PROBE (makeCopy): script == 'java -Xmx8g', isDirectiveReferenced('memory') == false

This reaches TaskProcessor.groovy:1064-1069, where the ProcessRetryableException / CloudSpotTerminationException path calls makeCopy() and then submits without re-resolving. The ordinary RETRY path at :1184 does re-resolve, so it's unaffected. Nothing in nf-seqera raises either exception today, so it's latent rather than live — but it fails silently in exactly the OOM direction the feature exists to prevent. The underlying cause is that the value cache and the access log have different lifetimes, so moving the reset out of newCache() and copying the set in clone() should do it.

A couple of smaller things:

• The "single funnel" comment at TaskConfig.groovy:169 isn't quite accurate — eval() reaches LazyMap#getValue via eval0 and bypasses recording (it's what TaskHasher:207 uses), and getRawValue reads target directly. Nothing on the rendering path uses them today, so no live bug, but it might be worth naming the exceptions so a future read routed through eval() inside the window doesn't silently go unobserved.
• The config-file case isn't covered by a test — every case in TaskDirectiveReadsTest builds new TaskConfig(...) directly, so no config parser is exercised. The mechanism is parser-agnostic by construction, so the risk is low, but since the withName: FOO { ext.args = {...} } idiom is the main advantage here it would be nice to pin it.

Minor, for later: a cached top-level dynamic directive could mask a read (containerOptions = { "...${task.memory}..." } resolved before resolve() returns from cache) — I confirmed the mechanism but found no live trigger, so perhaps just a comment at the cache; task-array tasks never go through resolve() so they always answer false (TaskArrayCollector.groovy:135), which could join the beforeScript caveat in the javadoc; and return config?.isDirectiveRead(...) from a boolean method is fine under dynamic Groovy but would become an unboxing NPE under @CompileStatic.

On naming, one suggestion: record tends to read as a noun in this codebase (TraceRecord, ProgressRecord, RecordMap, plus the Java 17 keyword in nf-lang), and read is a little ambiguous in a config class. access sidesteps both, and it's already the vocabulary here — TaskRun.groovy:991 describes getGlobalVars as "the set of task variables accessed in global script context":

private transient Set<String> accessedDirectives
private transient boolean trackingAccess

void trackDirectiveAccess(boolean value)
boolean isDirectiveAccessed(String directive)

Two optional extras: bracketing the window as config.trackDirectiveAccess { ... } would remove the try/finally and make a leaked flag impossible; and since the query really means "was accessed during the most recent resolve()" — empty before resolve, after clone, and for exec: tasks — a javadoc line saying it returns false when resolve() hasn't run would help the next caller. I'd leave the public TaskRun#isDirectiveReferenced name alone for now, since it's shared with the other PRs.

---

Update. I've pushed 30f08bf to this branch with the clone() fix, the access rename, the extra comments, and the new test cases — including the config-file ext.args idiom driven through both ConfigParserV1 and ConfigParserV2. Your local branch will be behind, sorry for the interruption.

Two of the suggestions above didn't survive contact with the tests, so I left them out:

• Bracketing the window was a bad idea. Passing the render step as a closure to TaskConfig means a mocked config stubs out trackDirectiveAccess and silently skips rendering altogether — it broke 3 existing TaskRunTest cases. And your try/finally already gave the safety I was claiming for it. Kept as a toggle, renamed only.
• I was wrong about the lazyName() fallback, which I'd flagged as broken. lazyName() returns early when name is set (TaskRun.groovy:383), so task.processor?.name ?: task.lazyName() degrades fine in the case that matters and only fails if processor and name are both null, which doesn't happen on a real TaskRun. Your version is strictly more robust than what I proposed — reverted, and please disregard that point for Disable resource prediction for processes referencing task.memory #7483/Report the task directives referenced by the task script #7505/Detect task directive references in dynamic config values #7507 too.

Also worth noting the ?. in resolve() is load-bearing, which I'd questioned above: stubRun && config.getStubBlock() short-circuits, so removing it NPEs a unit test that resolves a body with no config. I've added a comment saying so.