Skip to content

🤖 feat: preserve sub-agent uncommitted work via worktree diff artifact - #3765

Open
ibetitsmike wants to merge 1 commit into
mike/fix-memory-cache-invalidationfrom
mike/fix-subagent-dirty-loss
Open

🤖 feat: preserve sub-agent uncommitted work via worktree diff artifact#3765
ibetitsmike wants to merge 1 commit into
mike/fix-memory-cache-invalidationfrom
mike/fix-subagent-dirty-loss

Conversation

@ibetitsmike

@ibetitsmike ibetitsmike commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Fork-isolated sub-agents that end with uncommitted work no longer lose it silently: patch artifact generation now also captures a worktree diff (tracked + untracked, binary-safe) via a temporary git index, task_apply_git_patch applies it after the commit series as uncommitted changes, and the parent is told when a child ended dirty (or when capture was skipped by the size cap).

Top of the stack (base: #3764).

Background

Normal report cleanup force-removes the fork worktree, and the patch artifact was commit-only (git format-patch BASE..HEAD; zero commits = skipped, no mbox). An exec child that forgot to commit lost everything with no signal to the parent. Scope is the normal-completion path only: task_terminate remains intentional discard, and wait-timeout/hard-interrupt paths already preserve workspaces.

Implementation

  • Capture (gitPatchArtifactService.ts): after mbox generation, detect dirty state with git status --porcelain --untracked-files=all, then snapshot the full worktree diff through a temporary GIT_INDEX_FILE (read-tree HEADadd -Adiff --cached --binary HEAD) so capture never mutates the child's index or worktree. The diff streams into the artifact file with a hard byte cap (10 MB, src/constants/subagentPatch.ts); an oversized stream is cancelled mid-flight (never fully written to disk) and recorded as worktreePatchSkippedReason. Artifact metadata gains hadUncommittedChanges, worktreePatchPath, worktreePatchBytes, worktreePatchSkippedReason, and these fields survive all post-capture failure paths (e.g. format-patch failure).

  • Apply (task_apply_git_patch.ts): after git am of the commit series, the worktree patch applies with git apply --3way --binary, leaving the result as uncommitted changes: --3way implies --index, so newly staged entries are unstaged after a successful apply (unrelated staged entries survive), and the reverse-check completion path unstages patch paths a failed earlier attempt left staged before clearing the recovery marker. Dry-run mode exercises both stages in a temporary worktree. Failures surface which artifact failed and leave the repo recoverable; a worktree-only artifact (zero commits, dirty child) is now applyable instead of skipped. Applying a project whose uncommitted changes were NOT captured fails (dry runs too) unless the new acknowledge_uncaptured_changes arg is set, so workflows cannot checkpoint past the missing work; acknowledged applies still carry the warning note. Automatic cleanup defers while an artifact records unrecovered uncaptured changes (no appliedAtMs), preserving the child worktree, the only copy of that work, until an acknowledged apply stamps the project or the workspace is removed manually.

  • Reporting: task_await surfaces the artifact fields to the parent, so a dirty child end (or size-guard skip) is visible in the task result.

  • Safety: worktree patches are preflighted against the actual target's dirty state (mirroring the mbox overlap check, and covering rename and copy destinations from patch metadata) so dry runs stay predictive and overlap failures happen before git am. If the commit series lands but the worktree patch still fails, the artifact is marked applied with a persisted appliedPartial flag: retries cannot replay the commits, and a workflow cannot checkpoint past the missing uncommitted changes. A retry instead routes to a worktree-only completion path: it reverse-checks for already-present content first (acknowledging manual recovery, even uncommitted; also during dry runs, so workflow dry-run-first retries work), otherwise applies the pending worktree patch, and clears the partial marker on success (replay targets clear their target-local record, keeping the shared ancestor artifact untouched). For commit-bearing partials the completion path replaces the caller's expected_head_sha fence (which the earlier git am necessarily advanced) with the marker's recorded fence HEAD: it must still be an ancestor of the target HEAD, so a reset/rebased target fails instead of clearing the marker while the commit series is missing. Commit-free (worktree-only) partials never moved HEAD, so the caller's exact pin stays enforced before the ancestry check. Worktree-only apply failures record the same durable partial marker, since git apply --3way can leave part of a multi-file patch applied. An explicit acknowledge_partial_recovery arg clears the marker for manual resolutions the reverse check cannot recognize (e.g. intentionally merged parent+child content); an all-project sweep skips siblings with no recorded partial so it reaches the projects that need acknowledging, and fails if nothing was acknowledged. Replay targets (descendant or reconciliation workspaces applying an ancestor's artifact) record the same partial state target-locally in their own session dir, keeping the shared ancestor artifact replayable for other targets. Partial state is persisted BEFORE the worktree patch is attempted, covering rejects (cancellation, runtime failure) and crashes mid-apply; if the marker itself cannot be written (ENOSPC, EACCES), the commit series is rolled back with git reset --keep so the failure stays cleanly retryable, and an unrollbackable state is reported as do-not-reapply. The worktree-only path persists its marker before the irreversible git apply the same way (a marker-write failure there applies nothing and stays retryable). The commit series itself gets the same protection: a durable am-started marker (with the pre-am HEAD as fence) is persisted before git am runs, closing the crash window where the series lands but no state records it, which would let a retry replay the mbox. Retrying an am-started marker reconciles instead of completing worktree-only: an in-progress am session fails with continue/abort instructions, an unchanged HEAD proves nothing landed and retries fresh, and a missing fence or moved HEAD fails closed (resolvable via acknowledge_partial_recovery after manual inspection). The pre-am write does not stamp appliedAtMs, so an interrupted attempt cannot read as already applied. Stage markers (am-started/commits-applied) are persisted in both the shared artifact and replay-local state; an absent stage means commits-applied for markers written before the field existed, while a present-but-unreadable stage degrades to a read-side unknown that always fails closed (dropping it would misread an interrupted am-started record as commits-applied and skip git am). An acknowledge_partial_recovery sweep skips a marker-free sibling only when it is proven fully applied; an untouched project (an earlier partial stopped the loop before it was attempted) falls through to a normal apply instead of being skipped, so the sweep cannot report success while a whole project artifact remains unapplied. Replay targets prove sibling application via a target-local completion record, written in the same atomic write that clears the partial marker (a successful replay never stamps the shared ancestor artifact's appliedAtMs, which other targets still replay). Ordinary retries honor the same record: a valid replay completion reads as already applied (workflow retries with allowAlreadyApplied no-op instead of replaying the mbox), while a malformed completion degrades to a read-side unknown that fails closed until acknowledged or forced, never manufacturing proof of application. Completion records (shared-artifact and target-local) also persist the post-apply HEAD, and replay-safe retries validate the record before skipping: the recorded HEAD must still be an ancestor of the target HEAD, and the uncommitted-changes patch must still be reverse-applicable or its paths must deviate from the recorded post-apply state (so legitimate later edits or commits of the applied work still skip, while a discard, which restores pristine patch paths, fails the retry instead of checkpointing past missing work). Acknowledged completions keep the ancestry check but skip the content check, since a merged manual resolution is not reverse-applicable. A sweep with nothing to acknowledge anywhere fails before touching any repository. The target-local state reader fails closed on present-but-malformed containers at every level (matching its file-level corruption policy); a malformed partial record, including structurally empty ones like {} or [], degrades to a stage-unknown marker (the legacy absent-stage default is reserved for records with a valid timestamp). Artifact roll-up into the parent runs the freshness decision, file replication, and metadata replacement under the parent's artifact-file lock, so a stale child roll-up can neither overwrite a newer parent artifact's patch bytes nor race a lock-respecting concurrent writer. Replication stages all copies in a same-filesystem temp dir and swaps them into place (with backup restore) only after every copy in the task succeeds, so a partial failure never leaves retained metadata pointing at partially replaced files. Swap backups survive until metadata persistence completes: a failed artifacts-file write restores every completed swap before the failure propagates, so surviving old metadata always points at the original bytes; when a restore itself fails (e.g. transient EACCES), the staging root holding the un-restored backups is preserved instead of cleaned up, so the bytes the surviving metadata points at stay recoverable. Replication failures, including a failed parent metadata write after successful copies, keep the previous parent metadata and abort the workspace removal entirely (even forced): the child stays registered in config, since the session dir is only reachable through workspace lineage, and a retried removal re-runs the idempotent roll-up. Capture forces standard a/ b/ diff prefixes so user config like diff.noprefix cannot make artifacts unapplyable. A failed dirty-state inspection (git status error) is reported as uncaptured work, never as a clean tree. Workflow allowedPathPrefixes validation covers the worktree diff (and worktree-only artifacts), not just the mbox. Patch path extraction shares the apply-tool's parsers (gitPatchPathParsing.ts): unquoted diff --git headers with spaces are parsed via every candidate split (over-approximating instead of missing paths) and quoted paths go through the git C-quoting decoder, so a space-containing file name cannot slip past the allowlist. Dirty submodules are detected, excluded from the temporary index (a moved gitlink would reference a commit that exists only in the child's clone and becomes unfetchable after cleanup), and reported as explicitly uncaptured (a superproject patch cannot represent submodule contents) instead of a misleading empty-diff message, and a failed submodule or embedded-repo probe records a conservative possibly-uncaptured warning instead of passing as clean. Capture runs before the fallible commit-metadata checks, so a failed artifact still records and preserves uncommitted work.

  • Failure hardening: untracked embedded git repositories are excluded from capture (their gitlinks reference commits no target can resolve) and reported as NOT captured, with detection parsing git status --porcelain -z records so porcelain-quoted names (e.g. containing spaces) are still caught; capture stages blobs into a throwaway GIT_OBJECT_DIRECTORY so a huge dirty file cannot permanently grow .git/objects even when the byte cap rejects the artifact; a du-based preflight over the dirty paths additionally skips capture before staging when their on-disk size exceeds a 256 MiB bound, and a failed probe fails closed (only vanished paths are tolerated; any other du failure skips capture as uncaptured work instead of reading as small), so a multi-gigabyte dirty file cannot fill /tmp before the diff cap can trigger; a capture that throws is converted into conservative uncaptured metadata (hadUncommittedChanges: true + skip reason) instead of letting a dirty child be cleaned up as clean; failures persisting a partial-application marker propagate so a retry cannot replay already-applied commits (clearing stays best-effort since a stale marker fails closed); malformed local partial-state records degrade to a conservative marker (fence-less, epoch timestamp) instead of being dropped, since key presence itself proves commits landed, so a corrupted record still routes retries to worktree-only completion while valid siblings survive untouched; the shared artifact reader sanitizes corrupted dirty-capture/partial fields the same way (a corrupt appliedPartial stays fail-closed, a corrupt fence SHA falls back to the already-supported fence-less completion instead of throwing in shellQuote and bricking every retry, numeric fields are held to the schema's integer/nonnegative bounds, and the boolean appliedPartial marker alone routes retries to worktree-only completion since appliedAtMs is optional metadata); the worktree-patch resolver probes the canonical patch location even when path metadata was sanitized away, like the mbox resolver, so a captured patch on disk is never silently skipped; the workflow allowlist validator probes the same canonical location so that patch cannot bypass validation either; dirty-submodule detection parses -z records so a core.quotePath-quoted name (e.g. non-ASCII) is still detected instead of probing the quoted literal and silently missing the submodule. Embedded-repo excludes use :(exclude,literal) so a glob-like name cannot drop dirty siblings from the capture, and the diff-header parser handles mixed unquoted/quoted rename headers instead of rejecting in-prefix artifacts with the unparseable sentinel. The nested-artifact roll-up (parent preserving grandchild artifacts when an intermediate workspace is removed) no longer skips a project whose patch-path metadata is missing: it falls back to the canonical project dir as the copy source, so a captured worktree.patch survives child cleanup even when only the canonical file remains. The local partial-apply state file is read fail-closed (only ENOENT maps to empty state), so an unreadable file cannot make a retry replay already-applied commits; the roll-up always merges the canonical project dir and additionally merges each recorded patch path's directory that resolves inside the child session dir (mbox and worktree patch independently), so split legacy layouts survive child cleanup; rewritten parent metadata preserves the original basename for safe paths so the merged copy stays reachable. A capture exception that fires after the diff stream wrote the patch file removes the leftover file, so apply-side canonical probing cannot pick up an incomplete capture. Capture also deletes any stale canonical patch when a rerun finds a clean tree or the dirty-state inspection fails, so no exit that records no worktreePatchPath leaves a previous run's patch behind for canonical probing to reapply. Malformed local partial-apply JSON (unparseable file) fails closed like unreadable files; the parent roll-up always merge-copies the canonical project dir alongside any safe metadata-derived dir so noncanonical metadata cannot orphan the canonical worktree.patch. Metadata-referenced patch files are copied first with overwrite and directory merges only fill in missing siblings, so a same-named canonical file with different contents cannot mask the file the rewritten metadata actually points to; when the mbox and worktree patch are safe noncanonical paths whose basenames collide, or a single metadata path is named like the other kind's canonical file (both canonical basenames stay reserved even when the counterpart field is absent), the colliding copy gets a kind-prefixed destination name and the metadata rewrite follows the same plan, so artifacts cannot merge into or mask one another; a renamed source's original basename is also excluded from that directory's fill-missing merge, so the file cannot reappear under the reserved canonical name for apply-side probing to misinterpret as the other kind. A corrupt shared-artifact appliedPartial marker (non-boolean, including falsey corruption) sanitizes to a fail-closed partial marker instead of being erased by truthiness coercion, and a corrupt appliedAtMs degrades to a partial/unknown marker rather than silently dropping the only evidence that an application happened. The roll-up reads the child's patch index fail-closed: an existing-but-unreadable or malformed index reports replication failure, aborting the removal, instead of reading as empty and letting removal delete the only patch copies (a missing file still reads as empty; other callers keep the self-healing read). Entry-level normalization failures propagate the same way during cleanup: a malformed individual entry rethrows instead of silently reading as absent, so its patch files survive. Every successful replay records target-local completion and clears the pre-apply marker, including a fresh worktree-only replay, so a later retry reports already-applied instead of routing the finished apply through partial recovery. When a strictly newer child entry replaces an earlier roll-up of the same task, seeded canonical-kind files are deleted from staging before the copies re-add whichever kinds the child actually provides, so an obsolete canonical patch (e.g. stale uncommitted changes after a clean-tree rerun) cannot survive for canonical probing to apply, and a newer canonical file cannot lose to stale seeded bytes under the fill-missing merge. Local partial-record timestamps validate as positive integers, matching completion records, so an out-of-range appliedAtMs degrades to the fail-closed unknown stage instead of qualifying for the legacy absent-stage commits-applied default that skips git am on retry. Applied worktree patches land as plain worktree changes: git apply --3way implies --index, so the apply snapshots the staged set before and unstages exactly the newly staged paths afterwards (disjoint pre-existing staged entries survive), keeping the child's dirty-state semantics and the next commit-bearing apply's clean-index preflight intact. An aliased mboxPath/worktreePatchPath pair (one file recorded as both kinds) is sanitized at read time to the kind matching the artifact's shape, so the bytes are consumed exactly once. The initial pending-marker write propagates its failure, aborting generation before capture (no unreachable files, no cleanup unblocking), and the auto-cleanup gate fails closed on absence: a patch-eligible (exec-like) reported task with no artifact on disk defers deletion until a retried generation persists a terminal artifact, so a failed marker write can no longer let cleanup destroy the only copy of the child's work. Recording a completed application is itself fail-closed now: if the completion write fails after a successful apply (direct, replay target-local, or acknowledged recovery), the tool reports failure with do-not-re-apply guidance instead of success, so a workflow cannot checkpoint past durable state that still reads am-started/partial. The parent-side roll-up index update reads strictly too: a malformed parent index or entry aborts the roll-up (preserving the child session for retry) instead of self-healing to a reduced map whose persistence would orphan previously retained sibling patches. Corrupt dirty-work evidence degrades to dirty: a present non-boolean hadUncommittedChanges sanitizes to true, so falsey corruption cannot bypass the uncaptured-changes apply gate or unblock cleanup while a skip reason still records uncaptured work. A zero appliedAtMs is rejected like other out-of-range values and degrades to the fail-closed partial/unknown marker, because consumers test the timestamp by truthiness and an accepted 0 would neither prove application nor block a replay.

Validation

Test matrix covers: zero commits + dirty tracked change (artifact no longer skipped), commits + dirty change applied in order, untracked file capture, clean tree gaining none of the dirty-capture fields, worktree-patch apply failure surfacing without corrupting the target, size-guard skip metadata, and skip-reason warnings on both failed and successful applies.

Risks

Medium. This extends the sub-agent patch pipeline, which crash-recovery also reruns; generation stays idempotent per artifact. The worktree patch can conflict after git am (mitigated by --3way + explicit failure notes directing recovery). Pre-existing artifact-lifecycle semantics are unchanged: the artifact may still read pending immediately after the report wake, and task_apply_git_patch does a fresh read at apply time.


Generated with mux • Model: anthropic:claude-fable-5 • Thinking: xhigh

@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9d174b0142

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/tools/task_apply_git_patch.ts
Comment thread src/node/services/tools/task_apply_git_patch.ts
Comment thread src/node/services/tools/task_apply_git_patch.ts
@ibetitsmike
ibetitsmike force-pushed the mike/fix-subagent-dirty-loss branch from 9d174b0 to d34a946 Compare July 30, 2026 13:39
@ibetitsmike
ibetitsmike force-pushed the mike/fix-memory-cache-invalidation branch from f7dbb66 to 935a940 Compare July 30, 2026 13:39
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d34a94608e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/gitPatchArtifactService.ts Outdated
@ibetitsmike
ibetitsmike force-pushed the mike/fix-subagent-dirty-loss branch from d34a946 to c9b8482 Compare July 30, 2026 13:56
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@ibetitsmike
ibetitsmike force-pushed the mike/fix-subagent-dirty-loss branch from d34a946 to c9b8482 Compare July 30, 2026 13:56

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c9b8482f82

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/gitPatchArtifactService.ts Outdated
@ibetitsmike
ibetitsmike force-pushed the mike/fix-memory-cache-invalidation branch from 935a940 to c42ef91 Compare July 30, 2026 14:11
@ibetitsmike
ibetitsmike force-pushed the mike/fix-subagent-dirty-loss branch from c9b8482 to 270dac2 Compare July 30, 2026 14:11
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 270dac2935

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/tools/task_apply_git_patch.ts Outdated
Comment thread src/node/services/gitPatchArtifactService.ts Outdated
@ibetitsmike
ibetitsmike force-pushed the mike/fix-subagent-dirty-loss branch from 270dac2 to dabd92a Compare July 30, 2026 14:28
@ibetitsmike
ibetitsmike force-pushed the mike/fix-memory-cache-invalidation branch 2 times, most recently from c42ef91 to a62f7d2 Compare July 30, 2026 14:28
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@ibetitsmike
ibetitsmike force-pushed the mike/fix-subagent-dirty-loss branch from 270dac2 to dabd92a Compare July 30, 2026 14:28

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dabd92a930

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/workflows/WorkflowTaskServiceAdapter.ts
@ibetitsmike
ibetitsmike force-pushed the mike/fix-subagent-dirty-loss branch from dabd92a to 795afc3 Compare July 30, 2026 14:51
@ibetitsmike
ibetitsmike force-pushed the mike/fix-memory-cache-invalidation branch from a62f7d2 to 6fb63b5 Compare July 30, 2026 14:51
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 795afc3829

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/tools/task_apply_git_patch.ts
Comment thread src/node/services/tools/task_apply_git_patch.ts Outdated
Comment thread src/node/services/gitPatchArtifactService.generate.test.ts Outdated
@ibetitsmike
ibetitsmike force-pushed the mike/fix-subagent-dirty-loss branch from 795afc3 to 588f730 Compare July 30, 2026 15:14
@ibetitsmike
ibetitsmike force-pushed the mike/fix-memory-cache-invalidation branch from 6fb63b5 to 82282f5 Compare July 30, 2026 15:14
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 588f730695

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/tools/task_apply_git_patch.ts Outdated
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

This comment has been minimized.

@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8589c5d41d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/subagentGitPatchArtifacts.ts Outdated
@ibetitsmike
ibetitsmike force-pushed the mike/fix-subagent-dirty-loss branch from 8589c5d to 9754a4a Compare July 31, 2026 04:45
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9754a4afbd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/subagentGitPatchArtifacts.ts
@ibetitsmike
ibetitsmike force-pushed the mike/fix-subagent-dirty-loss branch from 9754a4a to 4fc1fa8 Compare July 31, 2026 04:54
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4fc1fa888c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/workspaceService.ts
@ibetitsmike
ibetitsmike force-pushed the mike/fix-subagent-dirty-loss branch from 4fc1fa8 to 84f1132 Compare July 31, 2026 05:05
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 84f1132525

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/tools/task_apply_git_patch.ts Outdated
Comment thread src/node/services/workspaceService.ts
@ibetitsmike
ibetitsmike force-pushed the mike/fix-subagent-dirty-loss branch from 84f1132 to 0563983 Compare July 31, 2026 05:22
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0563983935

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/gitPatchArtifactService.ts
@ibetitsmike
ibetitsmike force-pushed the mike/fix-subagent-dirty-loss branch from 0563983 to b68babd Compare July 31, 2026 05:45
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b68babdc60

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/tools/task_apply_git_patch.ts Outdated
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aeef1dd783

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/workspaceService.ts Outdated
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 891ecd2caa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/tools/task_apply_git_patch.ts
Comment thread src/node/services/gitPatchArtifactService.ts Outdated
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f69e8da60d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/gitPatchArtifactService.ts
Comment thread src/node/services/tools/task_apply_git_patch.ts
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 96a279742d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/node/services/subagentGitPatchArtifacts.ts Outdated
Comment thread src/node/services/workspaceService.ts
Comment thread src/node/services/subagentGitPatchArtifacts.ts Outdated
Comment thread src/node/services/subagentGitPatchArtifacts.ts
Fork-isolated exec children that never commit lose their working tree when
the fork worktree is removed after the report: the patch artifact is
commit-only (zero commits = skipped, no mbox).

Generation now captures dirty state (tracked edits + untracked non-ignored
files) as a binary diff against HEAD using a temporary index, so the child's
real index and worktree are never mutated. The diff is stored next to the
mbox and recorded in artifact metadata (hadUncommittedChanges,
worktreePatchPath/Bytes, worktreePatchSkippedReason above the 10 MB cap).
Zero-commit dirty artifacts become ready instead of skipped.

task_apply_git_patch applies the mbox via git am first, then the worktree
diff via git apply --3way --binary, leaving it as uncommitted changes.
Failures are surfaced with conflict paths and the repo left recoverable.
Artifact rollup for nested subagents carries the new patch file too.
@ibetitsmike

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 00c7600ecc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +2994 to +2996
const provenApplied = isReplay
? localCompletion != null
: Boolean(projectArtifact.appliedAtMs);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Revalidate sibling completions during acknowledgment sweeps

Fresh evidence beyond the ordinary-retry completion check is that an all-project acknowledge_partial_recovery sweep treats any timestamped sibling as proven applied without calling checkAppliedWorkStillPresent. If project A was previously completed but its commits or worktree changes were later reset/discarded, acknowledging a partial project B skips A and returns success, allowing a workflow to checkpoint with A's work missing; validate these completion records before skipping them.

AGENTS.md reference: AGENTS.md:L111-L111

Useful? React with 👍 / 👎.

Comment on lines +219 to +224
if (
sanitized.worktreePatchSkippedReason !== undefined &&
typeof sanitized.worktreePatchSkippedReason !== "string"
) {
delete sanitized.worktreePatchSkippedReason;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Derive dirty state from a retained skip reason

Fresh evidence beyond sanitizing falsey non-boolean dirty flags is that a type-valid corrupted record with hadUncommittedChanges: false and a valid worktreePatchSkippedReason passes unchanged. Both the apply acknowledgment gate and automatic-cleanup blocker require the flag to be exactly true, so the captured subset can be accepted and the child workspace containing the only uncaptured changes can be deleted; a retained skip reason should conservatively force dirty/uncaptured state.

AGENTS.md reference: AGENTS.md:L109-L111

Useful? React with 👍 / 👎.

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.

1 participant