Skip to content

feat(sleep): add AgenticReplayBackend with three-valued outcome and isolated worktree (#155) - #391

Open
RohithPariki wants to merge 2 commits into
microsoft:mainfrom
RohithPariki:feat/155-agentic-replay-slice
Open

RohithPariki wants to merge 2 commits into
microsoft:mainfrom
RohithPariki:feat/155-agentic-replay-slice

Conversation

@RohithPariki

@RohithPariki RohithPariki commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Implements the workspace-lifecycle and three-valued outcome slice for Issue #155:

  1. Three-Valued Outcome Protocol (skillopt_sleep/types.py):

    • Extended ReplayResult with outcome (pass | fail | inconclusive), inconclusive_reason (timeout, infra, fixture_failure, budget_exhausted, insufficient_evidence), and diff.
    • Preserves backward compatibility for all callers via __post_init__ default inference ("pass" when hard >= 1.0, else "fail").
  2. Opt-In Scratch Workspace & Hardened Lifecycle (skillopt_sleep/agentic_replay.py):

    • Narrowed to the workspace lifecycle + injectable agent_runner path (agent_cmd / live CLI binary execution and sandbox_type removed; enabled=False by default, strict is True check).
    • Disposable scratch copy (AgenticWorkspace, not a linked worktree and with no OS/container security boundary or network enforcement):
      • Pinned git fixtures materialize via git archive --format=tar <sha> without copying .git; unpinned copies reject .git files, symlinks escaping root, and objects/info/alternates.
      • Diffs are computed against an independent external bare harness.git with -c core.hooksPath=<empty> -c core.fsmonitor= -c diff.external= -c commit.gpgsign=false --no-ext-diff --no-textconv and strict scrubbed env (GIT_CONFIG_NOSYSTEM=1, GIT_CONFIG_GLOBAL=<empty_file>), failing cleanly with FixtureSetupError (fixture_failure) rather than swallowing git errors.
      • Allowlisted child environment (PATH, locale/OS minimal vars) redirecting HOME, USERPROFILE, TMPDIR, XDG dirs, CLAUDE_CONFIG_DIR, CODEX_HOME, GIT_ASKPASS, SSH_ASKPASS outside the diffed workspace tree.
      • Mined prompts (intent, context_excerpt, skill, memory) delivered via stdin / external JSON payload file; fixtures accepted only from explicit agentic_fixture_root with realpath containment validation.
      • Subprocess helper terminates the full process group on timeout and caps buffered stdout/stderr and unified diffs.
    • TaskVerifier & Budgets: No-verifier defaults to inconclusive (insufficient_evidence); non-zero subprocess exits classify as infra; enforces thread-safe per-night replay/token/turn caps and decidable action floors.
  3. Pairwise Gate Exclusion & Wiring (replay.py, consolidate.py, dream.py, cycle.py, backend.py, config.py, __init__.py):

    • Added pairwise_aggregate_scores() and updated consolidate() to score baseline and candidate over the exact intersection of decidable validation tasks, exclude inconclusive tasks from train reflection, and abstain (abstain_inconclusive) below min_decidable_coverage.
    • Wired agentic_* configuration keys from SleepConfig through cycle.py / build_backend / get_backend with per-night budget resets (reset_run_budget()) and lazy import in skillopt_sleep/__init__.py.
  4. Tests (tests/test_agentic_replay.py):

    • 18 unit & fake-agent subprocess tests covering opt-in gating, config wiring, git worktree add & alternates isolation, .git/config diff.external defense, flag-like intent/fixture containment, allowlist scrubbing, process-group timeout kills, byte caps, pairwise gate abstention/acceptance, and reflection filtering.

Closes #155 (first workspace-lifecycle slice).

@RohithPariki
RohithPariki marked this pull request as ready for review November 10, 2026 10:14
@Yif-Yang

Copy link
Copy Markdown
Contributor

Thanks for picking up #155. The three-valued outcome, the reason codes and the list-argv invocation are good foundations. Measured against the acceptance bar agreed on #155 (lifecycle first, agent execution disabled by default, no sandbox claims, inconclusive separated by cause), there are a few issues to resolve before this can merge. I'm listing all of them at once.

  1. Execution is not gated by an opt-in. AgenticReplayConfig.enabled, sandbox_type, max_turns, max_tokens and the new agentic_* config keys are never read. AgenticReplayBackend(AgenticReplayConfig(enabled=False, agent_cmd="/path/to/agent")) still executes the agent. sandbox_type="container" silently runs on the host. Could this PR drop the agent_cmd branch and ship only the workspace lifecycle plus fake-runner tests, as Proposal: AgenticReplayBackend — replay mined tasks with a real agentic CLI in a throwaway worktree, gate on the diff #155 suggested? Execution can come later behind an explicit flag and a real container/OS sandbox.
  2. Fixtures whose .git is a file (linked worktrees, submodules) reach the user's real repository. copytree keeps the gitdir: pointer. The harness's own git checkout <pinned_commit> detaches the user's worktree, and git add -N . writes to its index. An agent running git update-ref refs/heads/main … or writing $(git rev-parse --git-common-dir)/hooks/* changes the user's real branches and hooks. Please materialize fixtures without .git (e.g. git archive <pin> | tar -x), reject .git files and alternates, and add a test with a git worktree add fixture.
  3. The agent can run code in the harness with the host environment. get_diff() and setup() run git without env=, and they honour the workspace .git/config. An agent that adds diff.external=<script> gets that script executed by the harness's git diff HEAD, with the unscrubbed host env and the real HOME. Please compute the diff without consulting repository config (or at least use --no-ext-diff --no-textconv -c core.hooksPath=/dev/null -c core.fsmonitor=) and run harness git with a scrubbed env, GIT_CONFIG_NOSYSTEM=1 and GIT_CONFIG_GLOBAL=/dev/null.
  4. Mined text reaches argv and the filesystem. task.intent is passed as a positional argument, so an intent starting with -- becomes a CLI flag. Please pass it via stdin or a file, or after --. task.context_excerpt is used as a directory to copy whenever os.path.isdir() is true, including relative paths resolved against cwd. Please only take fixtures from an explicit fixture root, with a realpath containment check.
  5. Env scrubbing is a denylist. For example CLAUDE_CONFIG_DIR, CODEX_HOME, GOOGLE_APPLICATION_CREDENTIALS, KUBECONFIG, DATABASE_URL, GIT_CONFIG_GLOBAL, GIT_ASKPASS, DOCKER_HOST, DBUS_SESSION_BUS_ADDRESS, IDENTITY_HEADER/IDENTITY_ENDPOINT and lowercase https_proxy/no_proxy all reach the child. Please switch to an allowlist and point config directories at empty locations. Note that the HOME override does not stop a same-uid process from reading ~/.ssh or ~/.claude by absolute path (OpenSSH uses the passwd home).
  6. Network isolation is advisory. Only uppercase proxy variables are set, and a direct socket connection succeeds with isolate_network=True. Please remove the flag and its "disable external network by default" wording, or rename it and document it as non-enforcing.
  7. Naming and docs. Per Proposal: AgenticReplayBackend — replay mined tasks with a real agentic CLI in a throwaway worktree, gate on the diff #155, please don't describe the workspace as a sandbox: the module docstring, sandbox_type, .sandbox_home, "SkillOpt Agentic Sandbox", and "no host credential leakage". It is a disposable scratch copy (not a git worktree) with no security boundary.
  8. Gate semantics. aggregate_scores(exclude_inconclusive=True) is never used, so the gate still scores inconclusive results as 0. With the default max_replays_per_run=5, the baseline gets the budget and the candidate's later replays become budget_exhausted zeros (in a 4-task example, baseline 1.0 vs candidate 0.25). Please exclude inconclusive results pairwise (the same task ids on both sides) and abstain below a coverage floor. Related points:
    • With no verifier configured, every replay passes hard=1.0 whatever the skill. This should be inconclusive/insufficient_evidence.
    • On the agent_cmd path the skill and memory are never given to the agent.
    • A non-zero agent exit is labelled insufficient_evidence rather than an infrastructure cause.
    • Git setup failures are swallowed (e.g. with commit.gpgsign=true there is no baseline commit and the diff is empty, but the run is still scored).
    • A bad pinned_commit raises out of replay_batch instead of returning fixture_failure.

Smaller items: kill the whole process group on timeout (a backgrounded grandchild currently survives), cap captured stdout and diff size (300 MiB of agent output raised harness RSS to about 920 MiB), keep the agent's HOME outside the diffed tree (files written there end up in ReplayResult.diff), drop the fixed 500-token estimate or label it, make replays_run thread-safe and per-night, lazy-import AgenticReplayBackend in __init__.py, restore os.environ in test_workspace_isolation_and_git_baseline, and make the reflection test assert the filtering. Tests with a fake agent binary for each of points 1-6 would lock these properties in. Happy to re-review a narrowed version.

…nv isolation, and add pairwise gate exclusion
@RohithPariki

Copy link
Copy Markdown
Contributor Author

Thank you for the thorough review Yifan Yang (@Yif-Yang)! I have updated the PR branch (c3afe25) to narrow the slice and address every point raised:

  1. Strict opt-in & config wiring (Point 1):

    • Removed the agent_cmd live CLI execution branch and sandbox_type; AgenticReplayConfig.enabled defaults to False with strict is True normalization and gates replay_agentic_task before touching any workspace or runner.
    • Wired all retained agentic_* keys (agentic_replay, agentic_max_turns, agentic_max_tokens, agentic_timeout_sec, agentic_max_replays, agentic_isolate_env, agentic_min_actions, agentic_fixture_root, agentic_min_coverage) from config.py → cycle.py → build_backend() → get_backend("agentic") (only forwarded to get_backend for agentic targets so wrapper signatures like OpenClaw's stay compatible) and actively enforced max_turns and max_tokens.
  2. Worktrees, .git files, and alternates (Point 2):

    • Pinned git fixtures now materialize via git archive --format=tar <sha> extracted safely in-process without copying any .git.
    • Both pinned and unpinned paths reject objects/info/alternates (checked on --git-dir and --git-common-dir); unpinned copies also reject linked .git files and symlink escapes. Added a git worktree add subprocess test verifying real-repo refs/heads/main and hooks cannot be touched.
  3. Harness git isolation (Point 3):

    • AgenticWorkspace initializes an independent bare harness.git outside workspace_path and runs all harness git commands with --git-dir/--work-tree, -c core.hooksPath=<empty> -c core.fsmonitor= -c diff.external= -c commit.gpgsign=false --no-ext-diff --no-textconv, and scrubbed env (GIT_CONFIG_NOSYSTEM=1, GIT_CONFIG_GLOBAL=<empty_file>).
  4. Mined text containment & prompt passing (Point 4):

    • Stopped interpreting task.context_excerpt as a directory path. Fixtures load only from explicit agentic_fixture_root with os.path.realpath containment validation (FixtureSetupError on escape).
    • Prompts (intent, skill, memory, context_excerpt) are delivered via stdin and an external payload JSON outside the diffed directory rather than bare positional CLI arguments.
  5. Allowlist env scrubbing (Point 5):

    • Switched build_isolated_env() from a denylist to a minimal OS allowlist while redirecting HOME, USERPROFILE, TMPDIR/TMP/TEMP, XDG_*, CLAUDE_CONFIG_DIR, CODEX_HOME, GIT_ASKPASS, and SSH_ASKPASS to empty locations outside work. Added an explicit docstring note that same-UID absolute path reads (e.g., ~/.ssh) are outside the env boundary.
  6. Removed advisory network isolation flag (Point 6):

    • Removed isolate_network / agentic_isolate_network and dummy proxy env vars, and documented that network isolation is not enforced in this scratch-copy slice.
  7. Naming & docs (Point 7):

    • Updated module docstrings, comments, and identifiers (e.g. home outside work, SkillOpt Agentic Scratch) to describe a disposable scratch copy with no OS/container security boundary.
  8. Gate semantics, verifier, exit codes, and smaller items (Point 8):

    • Added pairwise_aggregate_scores() (skillopt_sleep/replay.py) and wired it through consolidate.py (_score_val_comparison) so baseline and candidate are scored on the exact intersection of decidable validation task IDs, inconclusive train rows are filtered out of reflection, inconclusive val task rows are reported as "inconclusive", and the gate abstains (gate_action="abstain_inconclusive") below min_decidable_coverage.
    • TaskVerifier.verify() with no checks configured now returns (OUTCOME_INCONCLUSIVE, 0.0, 0.0, ..., REASON_INSUFFICIENT_EVIDENCE).
    • Non-zero subprocess exits map to REASON_INFRA; git setup failures and bad pinned_commit raise FixtureSetupError and return REASON_FIXTURE_FAILURE instead of raising out of replay_batch.
    • AgenticReplayBackend.reflect() now matches the Backend.reflect signature (edit_budget, evolve_skill, evolve_memory, **kwargs returning List[EditRecord]) and its delegation is asserted via a spy.
    • Process-group termination (os.killpg / CREATE_NEW_PROCESS_GROUP + taskkill /F /T), streaming capped stdout/stderr/diff buffers, home outside work, removal of the fixed 500-token constant, thread-safe + per-night reset_run_budget(), lazy AgenticReplayBackend import in skillopt_sleep/__init__.py, restored os.environ in tests, and fake-agent subprocess tests for points 1–6 are all in place.

Validation run after the final edit:

  • python -m pytest tests/test_agentic_replay.py -v: 18 passed
  • python -m pytest tests/test_agentic_replay.py tests/test_plugin_sync.py tests/test_gate_no_regression.py tests/test_holdout_integrity.py tests/test_sleep_dream_metric.py tests/test_consolidation_edit_bookkeeping.py tests/test_sleep_adopt_skill_subset.py tests/test_backend_opencode.py -q: 268 passed, 4 skipped
  • python -m ruff check on all new/updated modules (__init__.py, agentic_replay.py, config.py, consolidate.py, cycle.py, dream.py, types.py, tests/test_agentic_replay.py): All checks passed

This branch has not been deployed

No deployments
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.

Proposal: AgenticReplayBackend — replay mined tasks with a real agentic CLI in a throwaway worktree, gate on the diff

2 participants