You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(repo): --delegation and an execution id for repo run cells - #107
Under --tools all, Claude Code can start subagents, so a repo run cell was not a single-agent baseline. Nothing recorded how the agent was started either, so a rerun with other tool arguments reused old cells.
run_cell
+ execution = {id: profile.execution_id(tools, delegation), tools, delegation}
if result.json exists and not --force
+ if its execution.id != execution.id (legacy result = native)+ raise ExecutionMismatch # reported as crashed; --force or new --batch
skip
argv = profile.argv(..., delegation=opts.delegation)
+ meta.json / result.json += execution
repo run --delegation native|none. The default native keeps today's argv. A profile without delegation.none is refused before anything runs.
The id is a sha256 prefix over the profile name, invoke, the resolved tool arguments, env_set, config_files and prompt_suffix.
Schema: optional delegation property in harness_profile.v1 (additive; schema-compat passes).
Evidence
Before: run batch b1 with --tools all, then again with other tool args → 0 ran, 1 already done: the old cell counted as the new configuration. After:test_a_cell_is_reused_only_by_the_same_execution: native run, then --delegation none → 1 crashed: the cell finished with another execution (89c98badf275, delegation native) than this run (4f14be0df49a, delegation none). With --force it reruns, and the argv ends with the profile's no-subagent args.
make ci-local: 18 pass. Bandit fails locally only because it scans a local .venv; with that excluded, bandit -r . -ll -x ./.venv is clean.
Merge Danger
Door: two-way
Blast Radius: resumed batches
After upgrading, any change to the built-in profile changes the id, so resuming an old batch reports its finished cells as crashed instead of reusing them. Old results without execution still count as native, so an unchanged resume works. --force or a new --batch is the way out.
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.68%. Comparing base (8782e1c) to head (888dfa2).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Execution fingerprints omit profile-controlled environment and guidance settings, allowing stale results to be reused.
No open issues 🎉
1 previously missed issue
In code that hasn't changed since last review
Include environment and bridge inputs in fingerprint
dokimi/dokimi/repos/harness_profile.py:119
The fingerprint omits env_allow, config_dir_env, and bridge, although they affect the agent's inputs. Changing the environment allow-list or config-directory variable changes the environment passed by run_cell; changing bridge.content changes the guidance written for guidance:bridge. These edits leave the ID unchanged, so resuming the same cell silently reuses the old result. Include these fields in the hash and add regression tests that verify such profile edits prevent reuse.
The reason will be displayed to describe this comment to others. Learn more.
The mechanism is sound: the flag, the refusal before anything runs, and the execution record all do what the description says, and make ci-local is green. Three things are worth fixing before merge: the docs for what the id covers (inline), the tests that pin it (inline), and the mixed batch in item 1 below.
Mutation run on the new code, 6 mutants: 4 pass the full suite (the run-level harness bin/version in the id, the legacy-result clause, env_set/config_files/prompt_suffix in the key, the pre-run refusal). 2 are killed (delegation= dropped from argv() and from AgentOptions).
A repro of a batch rerun with another --delegation, with the container runner faked.
Findings that sit outside the diff or span several files:
A rerun that adds cells to a finished batch under another --delegation mixes executions in one batch. Run --reps 1, then rerun with --reps 2 --delegation none: r1 is reported crashed (finished with another execution (53028ce4e5e7, delegation native) than this run (837f503d490c, delegation none)), r2 runs under none. The batch then holds a native cell and a subagent-free cell in the same arm, summarize shows missing == [], and compare.py / report.py never read execution. The exit code is non-zero, but anyone reading only the report gets a silently mixed comparison. Suggestion: refuse the whole invocation up front when any finished cell has another execution id, or have compare stamp the execution ids per batch the way it stamps harness_versions.
Non-blocking: with --tools none or --tools read, --delegation none has the same id as native.tool_args falls back to the mode's own arguments, so a --tools none cell recorded as delegation: native is reused by a --delegation none run (0 ran, 1 already done) and keeps delegation: native. The command line is identical, so reuse is right, but the contract says another --delegation does not reuse the cell. Say so in the docs, or record the delegation actually in effect.
I agree with @copilot-pull-request-reviewer that env_allow, config_dir_env and bridge are missing from the id; the test gap below would hide it.
Checked, no issues
Schema: the delegation property in harness_profile.v1.json is optional and additive; schema-compat passes.
Refusal: a profile without delegation.none is refused with declares no `delegation: none` , and no agent container starts.
Argv path:--delegation none reaches the container argv end to end (killed by two mutants).
Readers:compare.py, report.py and manifest.py ignore the extra execution key.
Changelog fragment: present, in the right directory, with Added and Changed sections.
The reason will be displayed to describe this comment to others. Learn more.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
…N_MODES, include env-shaping profile fields, use resolved binary path, improve test
- Use DELEGATION_MODES constant in repo_run.py choices instead of duplicating tuple
- Add env_allow and config_dir_env to execution_id key to detect profile env changes
- Use resolved absolute path str(harness_bin.resolve()) instead of just harness_bin.name to catch binary path changes
- Fix brittle test assertion: check "--no-agents" exists in argv, not that it's the last element
- Add test_a_changed_harness_binary_crashes_on_resume to verify binary path changes invalidate reuse
Addresses Copilot review comments on PR intel#107.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Include bridge configuration in guidance fingerprint
dokimi/dokimi/repos/harness_profile.py:120
For guidance:bridge cells, prepare_workspace() uses bridge.for, bridge.write, and bridge.content to generate guidance files, but this fingerprint omits them. Changing only the bridge configuration therefore leaves the ID unchanged, and resuming silently reuses results produced with different guidance. Include bridge in the fingerprint and add a regression test that changes only the bridge configuration.
Fix fingerprint tests to isolate the setting under test
dokimi/tests/test_repo_reach.py:240
The env_set, config_files, and prompt_suffix tests each create profiles in different directories. _profile() embeds those directories in invoke via the fake_harness.py path, so each assertion would still pass if its tested field were removed from the fingerprint. In all three tests, use the same directory for both variants and load the first profile before overwriting the file, so only the setting under test changes.
The reason will be displayed to describe this comment to others. Learn more.
Review at 522dd08. env_allow and config_dir_env are in the id now, and --delegation takes its choices from DELEGATION_MODES. Both of my threads are only partly fixed, and the switch to the resolved binary path makes the id depend on where the binary lives.
Mutation run on execution_id and execution_record, with the suite at 159 passed on the unmutated code: dropping profile, env_set, config_files, prompt_suffix, env_allow or config_dir_env from the key, one at a time, still gives 159 passed. So does .name in place of str(...resolve()) in execution_record. Dropping harness_bin or harness_version fails the suite.
Reproductions for the code findings, with the container runner faked.
I agree with @copilot-pull-request-reviewer that bridge is missing from the id and that the three execution_id_pinned_by_* tests need a shared directory. New evidence: with claude-code.yaml loaded, changing only bridge.content, bridge.write or bridge.for leaves the id unchanged (88faacbe8544), so a guidance:bridge cell is reused after the bridge text changes. egress and guidance.names are not hashed either.
Still open from my earlier review:
dokimi/docs/repo-evaluation-contract.md:689: the list now says "harness binary name (--bin file name)", but runner.py:96 hashes the resolved absolute path. The profile name, env_allow and config_dir_env are in the key and not in the list. The changelog fragment and the execution_id docstring (harness_profile.py:111-115) carry the same "binary name" wording.
dokimi/tests/test_repo_reach.py:224: the new tests do not pin what they claim. test_execution_id_pinned_by_env_set, _config_files and _prompt_suffix build their two profiles in different directories, and _profile() writes the directory into invoke, so the ids differ whatever the field holds. With the key field dropped, each still passes. Same directory, different env_set: ids differ. Different directories, same env_set: ids also differ. test_execution_id_pinned_by_profile_name loads claude-code twice and never varies the name. Still missing from my list: a hand-written legacy result.json reused by native and refused by --delegation none (replacing the or (not ran and ...) clause at runner.py:166 with or False, or with not ran, passes the suite), and the refusal test asserting that no version probe ran.
Run: full suite, ruff, mypy and bandit on the head and on the merge with main, plus Claude Code 2.1.304's system/init tool list for each --delegation.
--delegation none --tools all -> the init tools still include Workflow, which starts subagents on its own -> no subagent-starting tool. Fixed in a3b74ca (Workflow denied; test plus contract and changelog updated). A deny-list will go stale with new releases. An allow-list, or refusing a none run whose init tools list Agent/Task/Workflow, would hold.
A pre-PR result.json (no execution), rerun with --tools read or with a changed harness binary -> 0 ran, 1 already done -> not reused, as the changelog and contract §7 say. Alternatively, the docs should say that a pre-PR cell needs --force or a new --batch.
Still open: the review threads on the id inputs listed in the docs, on the untested key fields, on mixed executions in one batch, on --tools none/read under none, and on bridge/egress/guidance being left out of the id.
The reason will be displayed to describe this comment to others. Learn more.
Review at 2a9b1ad. The id now hashes the executable's bytes, so an unchanged harness resumes from any path, and bridge, egress and the Workflow deny are in. The per-field pins share one directory now and the legacy-result clause is killed. Two points are left, one in the body and one inline, plus the open thread.
Mutation run on execution_id, execution_record, run_cell and repo_run.py, on a baseline of 162 passed in test_repo_run.py, test_repo_reach.py and test_repo_runner.py. Dropping harness_sha256, env_set, config_files, prompt_suffix, bridge or egress from the key fails the suite. Replacing the legacy clause at runner.py:175-176 with or False or with not ran fails it too.
A repro of a rerun with more reps and another --delegation, with the container runner faked.
Finding that spans several files:
A rerun that adds cells to a finished batch under another --delegation still mixes executions in one batch. Run --reps 1, then --reps 2 --delegation none: r1 is reported crashed (finished with another execution (b47842628694, delegation native) than this run (cb0f56995e76, delegation none)), r2 runs under none. summarize then gives missing == [] and two passing cells, one native and one subagent-free, in the same arm. report.py and compare.py never read execution, so the report pools them. The exit code is non-zero, but a reader of the report alone sees no sign of it. This is the comparison the PR sets out to keep apart. Suggestion: refuse the whole invocation up front when any finished cell has another execution id, or have compare stamp the execution ids per batch the way it stamps harness_versions.
Still open from my earlier review:
dokimi/tests/test_repo_run.py:166: the refusal test only asserts that no spec with -p in its argv ran, so a harness_version probe is not counted. Deleting the pre-run profile.tool_args(args.tools, args.delegation) at repo_run.py:184 still gives 162 passed; with the assertion changed to setup["runner"].specs == [] that mutant fails on argv=['/eval/harness/fake-agent', '--version'].
dokimi/tests/test_repo_reach.py:228: test_execution_id_pinned_by_profile_name loads claude-code twice and never varies the name. Dropping profile and invoke from the key at harness_profile.py:117 together still gives 162 passed. No test varies env_allow or config_dir_env either. Add one _ids_differ_only_by assertion for each of profile, invoke, env_allow and config_dir_env.
Checked, no issues
Path-independent id:test_the_same_harness_bytes_resume_from_any_path and test_a_changed_harness_binary_crashes_on_resume hold; the digest read costs about 0.07 s for a 200 MB file per cell.
Schema: the delegation property stays optional and additive; schema-compat passes.
Refusal: a profile without delegation.none is refused before any cell runs.
Profile and docs agree on the deny list:claude-code.yaml and the contract example both list Agent, Task and Workflow.
The reason will be displayed to describe this comment to others. Learn more.
Review at ffdbda0. The change since 2a9b1ad is docs only, and it closes the docs thread: the contract, the changelog fragment and the execution_id docstring now list every field the key hashes. The test gaps in the other thread are untouched, so it stays open.
Mutation run on the baseline of 162 passed in test_repo_run.py, test_repo_reach.py and test_repo_runner.py: dropping profile and invoke, env_allow and config_dir_env from the key (checked one at a time, and all four together) still gives 162 passed. Deleting the pre-run profile.tool_args(...) at repo_run.py:184 also gives 162 passed.
Still open from my earlier review:
dokimi/tests/test_repo_reach.py:225: test_execution_id_pinned_by_profile_name loads claude-code twice and never varies the name; no test varies invoke, env_allow or config_dir_env. Add one _ids_differ_only_by assertion for each.
dokimi/tests/test_repo_run.py:166: the refusal test only counts specs with -p, so the version and isolation probes go unseen. Assert setup["runner"].specs == []; with that, the mutant that deletes repo_run.py:184 fails on the --version probe spec.
The mixed-batch point from my review at 2a9b1ad still applies: runner.py, report.py and compare.py are unchanged since then. I agree with @xaleryb that the contract should say a pre-PR cell is matched on delegation only.
Checked, no issues
Docs vs key: the field list in the contract, the changelog fragment and the docstring matches execution_id field for field.
CI: schema-compat, mkdocs build, mypy, bandit and the changelog fragment check pass.
The reason will be displayed to describe this comment to others. Learn more.
When delegation.none does not list the tool mode, this falls back to the native arguments, but the cell is still recorded as delegation: none. A custom profile with delegation: {none: {}} passes the pre-run check, and --tools all --delegation none then runs with Agent/Task/Workflow allowed. Please refuse a tool mode that delegation.none does not list, and list none and read explicitly in claude-code.yaml (for them the arguments are the same as native).
The reason will be displayed to describe this comment to others. Learn more.
Done in cc77c0e (option a). tool_args now treats delegation.none as an allow-list: a tool mode it does not list raises ProfileError instead of falling back to the native args. claude-code.yaml lists none and read explicitly. The cell's execution.delegation records the delegation whose args ran (native for none/read, where the args are identical), so the record matches the argv.
The reason will be displayed to describe this comment to others. Learn more.
A result.json from before execution ids passes this check for any --tools, harness binary or profile edit, because ran.get("id", execution["id"]) always matches. It is refused only under --delegation none, even with --tools read where the command line is the same. Please treat an unrecorded id as a mismatch, or say in the contract and the changelog that a pre-PR cell is matched on delegation only.
The reason will be displayed to describe this comment to others. Learn more.
Done in cc77c0e (option a). A result.json with no execution is no longer reused (ExecutionMismatch, unrecorded) unless --force or a new batch. Also, run_batch now refuses up front a batch dir that holds cells of another execution id. Tests added.
The reason will be displayed to describe this comment to others. Learn more.
This check only needs the profile, but it runs after _context(pin=True), so a refused --delegation none has already pinned the batch and may have built the image. Please move it right after load_profile(args.harness).
The reason will be displayed to describe this comment to others. Learn more.
Fixed in be33310: the --delegation check now runs right after load_profile, before the batch lock and _context(pin=True), so a refused run pins nothing and builds no image. Regression test test_a_refused_delegation_none_leaves_no_batch_behind fails on the previous commit.
Follow-up on the points in the review at cc77c0e that had no inline thread.
Mixed executions under --force: done in 14cc74b. --force now replaces only the cells of the run's plan; if the batch dir holds finished cells outside the plan with another execution id, the run is refused before anything runs (same ExecutionMismatch error and exit as other refusals). Test test_force_does_not_pool_executions_across_the_plan (--reps 2, then --reps 1 --delegation none --force) fails on the previous commit. Contract and changelog updated.
context: fork skills under --delegation none: documented in the contract (delegation section and the cell record bullet) and in the profile schema description in 14cc74b. --delegation none limits tools, not skills; a context: fork skill can still start a subagent. Tracked as follow-up #148, not fixed here.
Run: full suite, ruff, mypy, bandit at 14cc74b and on its merge with main; each earlier point's original input re-run; the delegation.none arguments checked against Claude Code's init tool list.
--force on a host that cannot run one of the planned tasks, with another --tools/--delegation -> exit 0, the kept cells of that task stay under the old execution next to the new ones -> refused like out-of-plan cells. Fixed in a483638 (second check once the unsupported tasks are known, plus a test, contract and changelog).
Schema delegation description: an unlisted tool mode "is taken to start no subagent already" -> --tools read --delegation none with only delegation.none.all is refused -> the description should say it is refused.
Contract §7 / changelog: a rerun with another id has "the cell ... reported as crashed" -> the run is refused before anything runs, 0 crashed -> the docs should describe the refusal.
Both doc points are fixed in d45da46 (which also merges main):
Schema delegation description: a tool mode the none block does not list is refused (HarnessProfile.tool_args raises ProfileError), not taken to start no subagent. The text now says so.
Contract and changelog fragment: a rerun whose execution id differs is refused before anything runs; the cell is not reported as crashed. Both now say that. Text only, no code change; the full suite passes after the merge.
The reason will be displayed to describe this comment to others. Learn more.
Review at d45da46. The --force refusal added since cc77c0e does what the contract says for the cases it names, and the doc points from the last round are fixed. One bug and one test gap are worth fixing before merge, plus two nits inline.
make ci-local: 27 passed, 1 failed, 7 skipped; 7,163 tests; changed-line coverage passes. The one failure is test_unbalanced_quote_in_hook_completes_quickly (1.34 s against a 1 s limit) while other jobs shared the machine; it passes alone (2 passed). It does not touch this PR.
Mutation run on execution_id, execution_record, tool_args, effective_delegation, check_batch_executions, the run_batch calls, the --delegation option and claude-code.yaml: 40 mutants, 4 of which pass the suite (271 passed in test_repo_run.py, test_repo_reach.py, test_repo_host.py, test_repo_runner.py). The per-cell reuse check in run_cell (if False:) is still the one I named last time; the others are the tools field of the record, the narrowed except in check_batch_executions, and choices= on the option (equivalent, tool_args raises first). The new second-check mutant is inline.
Reproductions for the code findings, with the container runner faked.
Checked, no issues
Allow-list:claude-code.yaml lists none, read and all; for none and read the arguments equal the native ones and the cell records native; for all they differ and the cell records none. A profile that lists only none refuses read and all.
Contract example: the example profile's delegation block equals the shipped YAML and works with --tools none|read|all --delegation none.
Id fields: the field list in the contract and the changelog matches execution_id field for field.
Multi-model: every model gets the same profile, tools, delegation and harness, so wanted in check_batch_executions holds one id.
Schema:delegation stays optional and additive; schema-compat passes. The fragment is in changelog.d/.
The reason will be displayed to describe this comment to others. Learn more.
A --force run refused by the second check still grows plan.json, so repo report lists cells of a run that never started as missing._record_plan runs here, before the host probe, and the refusal at runner.py:315-317 comes after it.
Batch b1 holds finished fix_add and fix_race cells (--reps 1); the host cannot run fix_race. Rerun with --task fix_add --task fix_race --force --tools read --reps 4. The run exits 1 with holds cells of another execution ... (--force replaces only the cells of this run's plan).
Afterwards _eval/plan.json has four new cells, and summarize()["missing"] is ['fix_add__guidance-raw__m-1__r3', 'fix_add__guidance-raw__m-1__r4', 'fix_race__guidance-raw__m-1__r3', 'fix_race__guidance-raw__m-1__r4']. _eval/host.json is written too.
The contract says the run "is refused before anything runs". The shipped test reuses the cell names of the earlier run, so it cannot see the plan change.
Suggestion: move _record_plan(...) below the if force and unsupported: check, and assert in the test that plan.json is unchanged after the refusal.
The reason will be displayed to describe this comment to others. Learn more.
Fixed in a6e43a3: _record_plan now runs after the second --force check, so a refused run adds no cell to plan.json. test_force_refuses_to_keep_cells_of_another_execution_for_a_task_the_host_cannot_run now runs the refused call with --reps 4 and asserts plan.json is byte-identical afterwards (it fails on the old order). host.json is still written, since the check needs the probe result.
The reason will be displayed to describe this comment to others. Learn more.
The test for the second --force check passes when the check skips the wrong cells. Both tasks are stale in this test (fix_add and fix_race ran under the old execution), so the check refuses whichever set it skips.
Mutation check: changing if t.id not in unsupported to if t.id in unsupported in the second check_batch_executions call (runner.py:317) still gives 271 passed across test_repo_run.py, test_repo_reach.py, test_repo_host.py and test_repo_runner.py. With that mutant, a run whose only stale cells belong to the supported task is refused for cells --force replaces, and stale kept cells of the unsupported task are accepted.
Suggestion: add a case where only the unsupported task's cells are stale (the supported task's cells already ran under the new --tools): the run must be refused. Add its mirror, where only the supported task's cells are stale: the run must go through.
The reason will be displayed to describe this comment to others. Learn more.
Added in a6e43a3: test_force_checks_the_kept_cells_of_the_unsupported_task_and_not_the_replaced_ones, two cases. Only the unsupported task (fix_race) stale: refused. Only the replaced task (fix_add) stale: goes through (2 ran, 2 already done). With t.id in unsupported in the second call both cases fail.
The reason will be displayed to describe this comment to others. Learn more.
I pushed a commit per nit, up to c4072dc, which fix the two nits from my review:
9d8a231: dokimi/dokimi/repos/runner.py:119: a result.json that is not an object, or whose execution or execution.id is not an object or string, now counts as unrecorded in check_batch_executions and run_cell.
c4072dc: dokimi/docs/repo-evaluation-contract.md:984: "refused before anything runs" now reads "refused before any agent starts" at both places, and in the changelog fragment.
make ci-local at c4072dc: 28 passed, 0 failed, 7 skipped.
Not approving yet. Still open: the bug and the risk from my review.
Items from the review at d45da46: the plan.json growth and the second-check test are in a6e43a3 (replied on the threads); the malformed result.json nit and the "before any agent starts" wording were fixed by Anton in 9d8a231 and c4072dc. The per-cell reuse check in run_cell now has a test that skips the up-front batch check (test_a_cell_finished_with_another_execution_is_not_reused_even_if_the_batch_check_is_skipped, in a6e43a3; it fails with the check replaced by if False:). Not done: the mutants on the tools field of the record, the narrowed except in check_batch_executions, and choices= on the option (you note the last is equivalent).
Mutants left over from the review at d45da46, pushed in 130a2fc (tests; refactors 8552a2e, growth entry 1eba297):
tools field of the record: test_the_cell_records_the_tools_mode_it_ran_with runs with --tools read and checks execution.tools in result.json and meta.json. Replacing opts.tools with a constant "all" in execution_record now fails it.
Narrowed except in check_batch_executions: test_an_unreadable_or_unparsable_result_does_not_stop_the_batch_check puts a result.json with invalid JSON (ValueError) and one that is a directory (OSError) in the batch and calls the check; except OSError alone and except ValueError alone both fail it. It also checks a parsable result of another execution is still refused.
choices= on the option: left without a test, it is equivalent. profile.tool_args(tools, delegation) runs before run_batch and raises for any value the profile does not list, so an invalid --delegation is refused with or without choices=; choices= only changes the message text, not any state or record.
Also: run_batch is back under its complexity baseline (helpers _log_cell, _probe_failed, _clear_unsupported_markers; entry removed from growth_baseline.json). Tests: 276 passed in test_repo_run/host/runner/reach; growth check clean in the simulated merge with ci(growth): take main's growth since the #93 seed into the baseline #184.
Under `--tools all` Claude Code may start subagents, so a repo run
cell was not a single-agent run, and nothing recorded how the agent
was started: a rerun with other tool arguments reused the old cells.
A harness profile may now declare `delegation.none`, per tool mode
the arguments that keep the agent from starting subagents (claude-code:
Agent and Task added to --disallowedTools). `repo run --delegation
none` uses them and is refused up front by a profile without them;
the default, native, keeps today's argv.
meta.json and result.json record `execution` (id, tools, delegation).
The id hashes the profile's invoke template, tool arguments, env_set,
config_files and prompt_suffix. A finished cell is reused only when
its id matches; otherwise it is reported as crashed and --force or a
new --batch runs it. Results from before the id count as native.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
When resuming a cell, changing the harness binary or its version should be
detected as a different execution. Previously, the execution_id() method only
considered the profile, invoke template, and tool configuration, but not the
actual harness executable or its version. This meant that resuming the same
cell with a different harness binary would incorrectly return the previous
binary's result.
Now execution_id() includes both harness_bin name and harness_version in its
hash, so cells are properly re-run when the effective harness changes.
Add a regression test that verifies different harness binaries and versions
produce different execution IDs.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…ersion
Updated repo-evaluation-contract.md to list the harness binary name (--bin file
name) and harness version as inputs to the execution id hash. Updated the
changelog to note that harness upgrades or binary changes no longer reuse old
cells.
Added four tests to verify execution_id is pinned by each input field:
- test_execution_id_pinned_by_profile_name
- test_execution_id_pinned_by_env_set
- test_execution_id_pinned_by_config_files
- test_execution_id_pinned_by_prompt_suffix
The existing test_harness_binary_and_version_change_the_execution_id already
covers binary name and version changes.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…N_MODES, include env-shaping profile fields, use resolved binary path, improve test
- Use DELEGATION_MODES constant in repo_run.py choices instead of duplicating tuple
- Add env_allow and config_dir_env to execution_id key to detect profile env changes
- Use resolved absolute path str(harness_bin.resolve()) instead of just harness_bin.name to catch binary path changes
- Fix brittle test assertion: check "--no-agents" exists in argv, not that it's the last element
- Add test_a_changed_harness_binary_crashes_on_resume to verify binary path changes invalidate reuse
Addresses Copilot review comments on PR intel#107.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The resolved path made an unchanged harness report its finished cells as
crashed after a move or a checkout change. The SHA-256 of the executable
is location-independent and also tells apart two same-named binaries.
Tests cover resume from another path, a changed binary with the same
version, and a result from before execution ids.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
Claude Code 2.1.304 offers a Workflow tool that starts subagents on its
own path, so denying only Agent and Task left a delegation: none cell
able to delegate. The init event's tool list now drops Workflow too.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
The id left out bridge.for/write/content (the guidance written into the
workspace) and egress (the container's network), so a changed bridge or
egress reused the finished cells. The three env/config/suffix pin tests
now load two profiles from one directory and vary only the field under
test, so each fails if its field drops out of the key.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The contract, changelog fragment and execution_id docstring named only
invoke, tool arguments, env_set, config_files, prompt_suffix, the
executable digest and version. The key also hashes the profile name,
env_allow, config_dir_env, egress and bridge, so editing one of those
reports finished cells as crashed without the docs saying so.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
The delegation check ran after the batch was locked and pinned, so a refused
run could leave a batch behind. Move it up to the profile load.
Pin the execution id by profile name and invoke, and cover a refused
--delegation none leaving no batch directory.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…that cannot run their task
--force keeps the finished cells of a task the host cannot run, and the
up-front check skipped them as cells of the plan, so a forced rerun with
another --tools or --delegation left both executions in one batch.
Signed-off-by: Alexander Rybkin <alexander.rybkin@intel.com>
…from run_batch; test(repo): pin the recorded tools mode and the check's tolerance of unreadable results
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
…profile schema
This text was added in the resolutions of merge commits ecea37d and d45da46, which a linear rebase drops.
Signed-off-by: Nikolay Petrov <nikolay.a.petrov@intel.com>
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
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.
antonwolfy
left a comment
Summary
Under
--tools all, Claude Code can start subagents, so arepo runcell was not a single-agent baseline. Nothing recorded how the agent was started either, so a rerun with other tool arguments reused old cells.repo run --delegation native|none. The defaultnativekeeps today's argv. A profile withoutdelegation.noneis refused before anything runs.idis a sha256 prefix over the profile name,invoke, the resolved tool arguments,env_set,config_filesandprompt_suffix.delegationproperty inharness_profile.v1(additive; schema-compat passes).Evidence
b1with--tools all, then again with other tool args →0 ran, 1 already done: the old cell counted as the new configuration.After:
test_a_cell_is_reused_only_by_the_same_execution: native run, then--delegation none→1 crashed: the cell finished with another execution (89c98badf275, delegation native) than this run (4f14be0df49a, delegation none). With--forceit reruns, and the argv ends with the profile's no-subagent args.test_delegation_none_drops_the_subagent_tools_and_changes_the_execution_id,test_delegation_none_is_refused_before_anything_runs_when_the_profile_cannot.make ci-local: 18 pass. Bandit fails locally only because it scans a local.venv; with that excluded,bandit -r . -ll -x ./.venvis clean.Merge Danger
Door: two-way
Blast Radius: resumed batches
After upgrading, any change to the built-in profile changes the
id, so resuming an old batch reports its finished cells as crashed instead of reusing them. Old results withoutexecutionstill count as native, so an unchanged resume works.--forceor a new--batchis the way out.🤖 Generated with Claude Code