Issue 298 prerelease fixes - #304
Merged
Merged
Conversation
`teardown-guard` swept only the invocation cwd, but the write guard now
arms inside each per-(group, condition) task env. Run mid-campaign it
printed "No write guard was installed — nothing to remove" while both
envs held live guards — the most costly moment for a false all-clear,
since the command exists for hand-editing files the guard would block.
It already accepted `CommonArgs`; the dispatch discarded them. Thread
them through and walk the iteration's staged envs the way `teardown` and
the `finalize` reminder already do, still without touching the staged
skill set or the workspace. Where those flags resolve no run, the sweep
now names the scopes it actually checked and warns that the env guards
were not among them.
Before:
$ eval-magic teardown-guard --iteration 1
No write guard was installed — nothing to remove.
After:
$ eval-magic teardown-guard --skill demo --workspace-dir ws --iteration 1
🛡 Write guard removed: 2 task envs in iteration 1.
$ eval-magic teardown-guard # from an unrelated cwd
No write guard was installed — nothing to remove (checked the invocation cwd).
⚠ Task env guards were not checked, so any that were armed still are: …
Add the run's target flags, or run `eval-magic teardown`.
Verified: cargo test, cargo clippy --all-targets -- -D warnings,
cargo fmt --check, plus a real run → teardown-guard against a scaffolded
skill.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NJjf8XMm1e1XtRZaGnkmvr
Every dispatch prompt designates `<eval-root>/tmp` for temporary work and tells the agent to use it. An agent that complied then had everything it put there measured as part of its change: in one prerelease run `tmp/` accounted for 3 of 11 files touched and 228 of 296 lines added. That trips `diff_scope` budgets on throwaway notes, puts scratch files in the `diff.patch` a judge reads as the deliverable, and does so asymmetrically — only in the arm that happened to use the directory it was told to use. `.eval-magic-outputs/` already never counts, for the same reason. The scratch directory now shares that treatment, on both surfaces: the env's `.git/info/exclude` and each harness's `framework_ignore_paths`. Both were spelling the outputs entry separately, and the diff-scope test fixture spelled it a third time — so the fixture could not fail when the rule changed. `sandbox::framework_owned_entries` is now the one definition all three read. Only files created under `tmp/` are affected: gitignore rules never apply to tracked paths, so a codebase that genuinely tracks a `tmp/` directory keeps its files measured. Verified: cargo test, cargo clippy --all-targets -- -D warnings, cargo fmt --check, plus a real `run` — a scratch file under `tmp/` is invisible to `git status` in the staged env while the agent's own file still shows, and both arms get the same `.prettierignore` block. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJjf8XMm1e1XtRZaGnkmvr
Teardown's kept-iteration warning offered `.eval-magic/<skill>/` as the
directory to delete. That was right before the eval home moved out of the
skill repo; since then no such path exists, and the hint printed it one
line below a `promote-baseline` command carrying the correct absolute
`--workspace-dir`.
`ctx.workspace_root` was already in scope and already rendered correctly
by `command_target_args` in the same message.
Before:
eval-magic promote-baseline … --workspace-dir /home/u/.local/share/eval-magic/skills-c61a1930 …
or delete .eval-magic/working-with-tdd/ manually to discard.
After:
or delete /home/u/.local/share/eval-magic/skills-c61a1930/working-with-tdd/ manually to discard.
The existing teardown test could not catch this: it runs with
EVAL_MAGIC_WORKSPACE_DIR=.eval-magic, where the two spellings coincide.
The new case puts the workspace elsewhere.
Verified: cargo test, cargo clippy --all-targets -- -D warnings,
cargo fmt --check.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NJjf8XMm1e1XtRZaGnkmvr
Review follow-ups on the #298 fixes. `envs_phrase` read the iteration out of `checked` with `unwrap_or_default`, which no call site can reach — but a future one would silently print "iteration 0". Take the iteration as an argument so the invariant is in the signature. The task repository's exclude file now covers more than the outputs dir, so its failure message says "framework path exclusion". Verified: cargo test, cargo clippy --all-targets -- -D warnings, cargo fmt --check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NJjf8XMm1e1XtRZaGnkmvr
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
1 participant
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.
Closes #298