fix: move-codex-session should refuse sessions whose goals cite source attachments #147

Merged
jercik merged 4 commits from fix/move-codex-session-goal-attachments into main 2026-10-10 08:28:51 +00:00
Owner

Codex 0.160.1's TUI saves long goal text (over 4,000 characters), pasted text and images under <codex_home>/attachments/<uuid>/ and cites them by absolute path in thread_goals.objective (codex-rs/tui/src/goal_files.rs at rust-v0.160.1). The script moves goal rows but not those files, so a moved goal cites files that are lost once the source home is deleted.

Preflight now refuses, before any lock or write, when a moved thread's objective contains /attachments/<entry>/ for an entry of the real source home's attachments/. It checks again once the script holds the writer locks, because a source writer can change a goal until then. The error names the thread and file. Matching the random UUID directory names doesn't depend on how the home is spelled. Codex doesn't resolve symlinks in the default ~/.codex, so an earlier path match missed aliases.

SKILL.md lists the refusal with its remedy: /goal clear, or replace the goal with one of at most 4,000 characters with no pasted text or images, then rerun. It also separates thread_attachments rows (client JSON, which is moved) from attachment files (not moved), and unifies the placeholder names.

Rejected: copying the files and rewriting the objective. It needs a new verification exception, and none of the 170 Codex homes checked has an attachments/ directory.

Not checked: old citations in rollout ThreadGoalUpdated events and in threads.preview. Codex restores goals only from thread_goals.

The new tests fail on main. They cover each Codex reference form, four alias spellings, a goal that changes after the preflight, no modification on refusal, and unrelated citations still moving. Six mutants of the first version were all caught.

🤖 Generated with Claude Code

Codex 0.160.1's TUI saves long goal text (over 4,000 characters), pasted text and images under `<codex_home>/attachments/<uuid>/` and cites them by absolute path in `thread_goals.objective` (`codex-rs/tui/src/goal_files.rs` at `rust-v0.160.1`). The script moves goal rows but not those files, so a moved goal cites files that are lost once the source home is deleted. Preflight now refuses, before any lock or write, when a moved thread's objective contains `/attachments/<entry>/` for an entry of the real source home's `attachments/`. It checks again once the script holds the writer locks, because a source writer can change a goal until then. The error names the thread and file. Matching the random UUID directory names doesn't depend on how the home is spelled. Codex doesn't resolve symlinks in the default `~/.codex`, so an earlier path match missed aliases. `SKILL.md` lists the refusal with its remedy: `/goal clear`, or replace the goal with one of at most 4,000 characters with no pasted text or images, then rerun. It also separates `thread_attachments` rows (client JSON, which is moved) from attachment files (not moved), and unifies the placeholder names. Rejected: copying the files and rewriting the objective. It needs a new verification exception, and none of the 170 Codex homes checked has an `attachments/` directory. Not checked: old citations in rollout `ThreadGoalUpdated` events and in `threads.preview`. Codex restores goals only from `thread_goals`. The new tests fail on `main`. They cover each Codex reference form, four alias spellings, a goal that changes after the preflight, no modification on refusal, and unrelated citations still moving. Six mutants of the first version were all caught. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Codex's TUI writes long goal text, pasted text and images to
`<home>/attachments/<uuid>/` and cites them by absolute path in
`thread_goals.objective`. The script moved the goal rows but not the files, so
a moved goal pointed into the source home and lost its files once that home
was deleted.

Preflight now refuses when a moved thread's goal cites a path under the given
source home's `attachments/` directory or under its real path, naming the
thread and the path. SKILL.md lists the refusal and no longer blurs
`thread_attachments` rows (client JSON, which do move) with attachment files.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
fix: move-codex-session should detect goal attachment citations however the source home is spelled
All checks were successful
commit-msg / commitlint (pull_request) Successful in 22s
Node tests / node:test (pull_request) Successful in 4m14s
Review / Review (pull_request_target) Successful in 3s
ccd911fd06
Without `CODEX_HOME`, Codex cites `$HOME/.codex/attachments/<uuid>/...`
normalized lexically but not symlink-resolved. When `~/.codex` is a symlink and
`--source-home` names its target or another alias, the citation matched neither
the given path nor its real path, and the move went through.

The guard now lists the UUID directories under the real source home's
`attachments/` and refuses when a moved thread's goal contains
`/attachments/<uuid>/` for one of them, whatever the home's spelling. A citation
whose directory is already gone loses nothing more by moving, so the given-path
and real-path matching is gone. The error names the file under the real source
home.

SKILL.md now says how to clear the refusal: `/goal clear`, or replace the goal
with one of at most 4,000 characters that has no pasted text or images, then
close Codex and rerun. Its home placeholders are consistent.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Review 01M4JEJM2EZVMYD0793EKH734D — head 7943249911174577f22012e7da296e2cfc12bdb9

Review — j4k-oss/agent-skills @ 5b1aa6fb3b

Scope: diff against base tree aaa8e581a81e
Status: dispatched — coverage complete (5/5 slots terminal)
Facts: current review-wide projection

Computed under:

{
  "abandonment": "abandonment-v1",
  "anchor_recipe": 1,
  "batch_policy": "batch-v1",
  "coverage": "coverage-v3",
  "dispatch_policy": "dispatch-v2",
  "grounder_version": 1,
  "grounding_read_rule": "grounding-read-v1",
  "promotion_policy": "promotion-v1",
  "report": "report-v4",
  "tally": "tally-v1",
  "triage_settle": "triage-settle-v2"
}

Findings (1)

low — Goals citing another home’s attachment are refused when its directory ID collides

  • claim: 01M4JEMG7GYE1MAM4ZAAQE0DAF
  • anchor: skills/move-codex-session/scripts/move-codex-session.ts (snippet)
        const marker = `/attachments/${directory}/`;
        const start = goal.objective.indexOf(marker);
  • lens: general-bug · arm: default
  • verdicts: 1 valid / 0 invalid / 0 uncertain
    • pass 01M4JEQ3BYBD6WD1BF7F0C2MWM · valid: The exact-grounded excerpt constructs /attachments/${directory}/ and searches goal.objective with indexOf, so the match itself does not distinguish source-home paths from destination or other-home paths. The reviewer supplies a coherent trace that requireNoGoalAttachmentReferences aborts on that match at both preflight and locked checks, and reports that the documented blocker is a reference under the source attachments directory. With the same directory name in both homes, the cited destination path satisfies the source-entry marker despite not referring to a source attachment. No supplied guard refutes this conditional false refusal; an executed reproduction is unnecessary to establish the string-match mechanism. Low severity is appropriate. Resolve the referenced path against the source attachment location while preserving supported lexical and symlink aliases.
  • disposition: none

A move can be refused even though the moved goal's referenced file remains in the destination or another home and will not be lost with the source. The check searches only for /attachments/<entry>/ in the objective; it never verifies that the text before that marker names the source home. For example, if both homes contain attachments/<same-id>/ and a goal cites <destination-home>/attachments/<same-id>/goal.md, this condition matches the source entry and aborts the move.

The guard should establish that the cited path resolves to the source home’s attachment directory, while retaining support for source-home aliases that Codex may write into objectives. I traced requireNoGoalAttachmentReferences and its two calls from main; the check runs during preflight and again after writer locks, but both use this same marker-only match. The skill documentation says the blocker is a goal citing a file under the source attachments directory. I did not run a reproduction; the collision behavior follows directly from indexOf and is conditional on the same attachment directory name existing in both locations. A reproduction with that duplicate directory name would establish the false refusal; path-aware matching with symlink and lexical aliases covered would refute it.

Other claims

  • grounding-pending (0)
  • ungrounded (0)
  • rejected (0)
  • duplicate-of (0)
  • unadjudicated (0)

Coverage

Coverage pass: 01M4JEK05QA8K75E0GDQYQPE2D
Accounting: complete
Slot health: healthy

lens part arm unit status runs loss
general-bug whole default claims-emitted 1 no
writing-quality whole default no-claims 1 no
test-trimming whole default no-claims 1 no
restated-sets whole default no-claims 1 no
project-docs whole default no-claims 1 no
<!-- review:summary --> **Review** `01M4JEJM2EZVMYD0793EKH734D` — head `7943249911174577f22012e7da296e2cfc12bdb9` # Review — j4k-oss/agent-skills @ 5b1aa6fb3b4c Scope: diff against base tree `aaa8e581a81e` Status: dispatched — coverage complete (5/5 slots terminal) Facts: current review-wide projection Computed under: ```json { "abandonment": "abandonment-v1", "anchor_recipe": 1, "batch_policy": "batch-v1", "coverage": "coverage-v3", "dispatch_policy": "dispatch-v2", "grounder_version": 1, "grounding_read_rule": "grounding-read-v1", "promotion_policy": "promotion-v1", "report": "report-v4", "tally": "tally-v1", "triage_settle": "triage-settle-v2" } ``` ## Findings (1) ### low — Goals citing another home’s attachment are refused when its directory ID collides - claim: `01M4JEMG7GYE1MAM4ZAAQE0DAF` - anchor: `skills/move-codex-session/scripts/move-codex-session.ts` (snippet) ``` const marker = `/attachments/${directory}/`; const start = goal.objective.indexOf(marker); ``` - lens: general-bug · arm: default - verdicts: 1 valid / 0 invalid / 0 uncertain - pass `01M4JEQ3BYBD6WD1BF7F0C2MWM` · valid: The exact-grounded excerpt constructs /attachments/${directory}/ and searches goal.objective with indexOf, so the match itself does not distinguish source-home paths from destination or other-home paths. The reviewer supplies a coherent trace that requireNoGoalAttachmentReferences aborts on that match at both preflight and locked checks, and reports that the documented blocker is a reference under the source attachments directory. With the same directory name in both homes, the cited destination path satisfies the source-entry marker despite not referring to a source attachment. No supplied guard refutes this conditional false refusal; an executed reproduction is unnecessary to establish the string-match mechanism. Low severity is appropriate. Resolve the referenced path against the source attachment location while preserving supported lexical and symlink aliases. - disposition: none > A move can be refused even though the moved goal's referenced file remains in the destination or another home and will not be lost with the source. The check searches only for <code>/attachments/&lt;entry&gt;/</code> in the objective; it never verifies that the text before that marker names the source home. For example, if both homes contain <code>attachments/&lt;same-id&gt;/</code> and a goal cites <code>&lt;destination-home&gt;/attachments/&lt;same-id&gt;/goal.md</code>, this condition matches the source entry and aborts the move. > > The guard should establish that the cited path resolves to the source home’s attachment directory, while retaining support for source-home aliases that Codex may write into objectives. I traced `requireNoGoalAttachmentReferences` and its two calls from `main`; the check runs during preflight and again after writer locks, but both use this same marker-only match. The skill documentation says the blocker is a goal citing a file under the source attachments directory. I did not run a reproduction; the collision behavior follows directly from `indexOf` and is conditional on the same attachment directory name existing in both locations. A reproduction with that duplicate directory name would establish the false refusal; path-aware matching with symlink and lexical aliases covered would refute it. ## Other claims - grounding-pending (0) - ungrounded (0) - rejected (0) - duplicate-of (0) - unadjudicated (0) ## Coverage Coverage pass: 01M4JEK05QA8K75E0GDQYQPE2D Accounting: complete Slot health: healthy | lens | part | arm | unit status | runs | loss | | --- | --- | --- | --- | --- | --- | | general-bug | whole | default | claims-emitted | 1 | no | | writing-quality | whole | default | no-claims | 1 | no | | test-trimming | whole | default | no-claims | 1 | no | | restated-sets | whole | default | no-claims | 1 | no | | project-docs | whole | default | no-claims | 1 | no |
@ -2138,2 +2169,4 @@
);
assertThreadDatabaseEmpty(threadDatabase.destination, threadDatabase, threadIds);
if (threadDatabase.prefix === "goals") {
requireNoGoalAttachmentReferences(threadDatabase.source, threadIds, args.sourceHome);

medium — Recheck goal attachment references after acquiring writer locks

A goal can acquire a source-attachment reference after this check and still be copied, leaving the moved thread with an objective that points to a file omitted from the destination. The goal check runs in preflight; is not called until later, and the locked phase rechecks rollout lineage but does not re-read or validate before transfers it. A source writer that updates the goal between preflight and lock acquisition can therefore pass the new guard. I traced from this check through lock acquisition, the under-lock rollout recheck, and the goals database copy; this is static reasoning, not a reproduced race. Re-run the goal-reference check while holding the writer locks (or otherwise bind validation to the exact goal rows copied). A test that changes a goal from safe to source-attached during the preflight/lock interval and confirms refusal would establish the consequence; if the locking protocol makes that update impossible before the under-lock copy, documenting and verifying that guarantee would refute it.

lens general-bug · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M4JDMG4F7WYW36NH45HCBMHQ of review 01M4JDJ45QDW5VDSD17DTE9MR8

<!-- review:claim:01M4JDMG4F7WYW36NH45HCBMHQ --> **medium** — Recheck goal attachment references after acquiring writer locks > A goal can acquire a source-attachment reference after this check and still be copied, leaving the moved thread with an objective that points to a file omitted from the destination. The goal check runs in preflight; is not called until later, and the locked phase rechecks rollout lineage but does not re-read or validate before transfers it. A source writer that updates the goal between preflight and lock acquisition can therefore pass the new guard. I traced from this check through lock acquisition, the under-lock rollout recheck, and the goals database copy; this is static reasoning, not a reproduced race. Re-run the goal-reference check while holding the writer locks (or otherwise bind validation to the exact goal rows copied). A test that changes a goal from safe to source-attached during the preflight/lock interval and confirms refusal would establish the consequence; if the locking protocol makes that update impossible before the under-lock copy, documenting and verifying that guarantee would refute it. lens `general-bug` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M4JDMG4F7WYW36NH45HCBMHQ` of review `01M4JDJ45QDW5VDSD17DTE9MR8`
Author
Owner

Fixed in 2aa306d. The goal check now runs again after the script takes the writer locks, next to the rollout lineage recheck. A new test edits the source goal after the preflight and expects the refusal; without the fix the move exited 0 and copied the citation.

<!-- gh-feedback:reply-to:151682 --> Fixed in 2aa306d. The goal check now runs again after the script takes the writer locks, next to the rollout lineage recheck. A new test edits the source goal after the preflight and expects the refusal; without the fix the move exited 0 and copied the citation.
jercik marked this conversation as resolved
fix: move-codex-session should recheck goal attachment citations under the writer locks
All checks were successful
commit-msg / commitlint (pull_request) Successful in 18s
Node tests / node:test (pull_request) Successful in 3m52s
Review / Review (pull_request_target) Successful in 4m37s
2aa306d932
A source writer can change a goal between the preflight and the moment the
script holds the source writer locks, and the copy then carried an
attachment citation the preflight never saw. The check now runs again under
the locks, as the rollout lineage check does.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@ -1613,0 +1635,4 @@
return `Read the Codex goal objective file at ${home}/attachments/${GOAL_FILE_ID}/goal-objective.md before continuing.`;
}
// Every file's content hash and every directory below `home`, apart from SQLite's transient WAL companions.

medium — The snapshot helper comment duplicates its traversal inventory

This comment creates a second place to maintain the exact coverage of the home snapshot, so a future change to what the helper records can leave the description silently stale. The adjacent snapshotHome implementation recursively visits entries below home, skips names matching /(wal|shm)$/, and stores a content hash for files and not a file for other entries. Its covered set therefore is every visited file and non-file entry except entries with those suffixes; the comment describes that same set, and none appears only in one or the other. This is in a test helper, not a user-facing explanation whose reader needs a separate inventory; the code is the authority and directly shows the coverage. Delete the comment; the helper itself preserves the test's purpose without a competing description. I inspected the changed test and its snapshotHome implementation in skills/move-codex-session/scripts/move-codex-session.test.ts; this is a static comparison, not a separately run test. The deciding evidence is whether the comment serves a distinct reader task beyond describing the immediately following helper; the code and comment show no such task.

lens restated-sets · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M4JE356G7D2GZHZ41MTWAQ0H of review 01M4JE0RED3FNQY1WDMN81VMQF

<!-- review:claim:01M4JE356G7D2GZHZ41MTWAQ0H --> **medium** — The snapshot helper comment duplicates its traversal inventory > This comment creates a second place to maintain the exact coverage of the home snapshot, so a future change to what the helper records can leave the description silently stale. The adjacent `snapshotHome` implementation recursively visits entries below `home`, skips names matching `/(wal|shm)$/`, and stores a content hash for files and `not a file` for other entries. Its covered set therefore is every visited file and non-file entry except entries with those suffixes; the comment describes that same set, and none appears only in one or the other. This is in a test helper, not a user-facing explanation whose reader needs a separate inventory; the code is the authority and directly shows the coverage. Delete the comment; the helper itself preserves the test's purpose without a competing description. I inspected the changed test and its `snapshotHome` implementation in `skills/move-codex-session/scripts/move-codex-session.test.ts`; this is a static comparison, not a separately run test. The deciding evidence is whether the comment serves a distinct reader task beyond describing the immediately following helper; the code and comment show no such task. lens `restated-sets` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M4JE356G7D2GZHZ41MTWAQ0H` of review `01M4JE0RED3FNQY1WDMN81VMQF`
Author
Owner

Fixed in 7943249. The helper comment is gone; the reason for skipping the WAL companions now sits beside the skip.

<!-- gh-feedback:reply-to:151686 --> Fixed in 7943249. The helper comment is gone; the reason for skipping the WAL companions now sits beside the skip.
jercik marked this conversation as resolved
Lines 1720-1721
@ -1613,0 +1717,5 @@
assert.equal(refused.stdout, "", name);
assert.equal(
refused.stderr,
`thread ${ROOT_ID} has a goal that references ${fixture.sourceHome}/attachments/${GOAL_FILE_ID}/goal-objective.md; goal attachment files are not moved\n`,
name,

low — The alias cases expect the physical path instead of the path passed to the script

The passed: "alias-a" cases make this test expect the real source path even though the command receives the symlink path, so the test rejects the intended refusal diagnostic. parseArguments applies resolve(sourceHome), which cleans the alias lexically without resolving it, and requireNoGoalAttachmentReferences builds the reported path from join(sourceHome, "attachments"); therefore for those cases the diagnostic includes alias-a/attachments, not fixture.sourceHome/attachments. The test still checks the refusal status, but its exact-stderr assertion makes the added alias coverage fail on the valid behavior. Build the expected diagnostic from spellings[passed] (or assert the refusal and referenced UUID/file without fixing the source-home spelling). I traced the four cases in this test to parseArguments and the diagnostic construction in requireNoGoalAttachmentReferences; running node --test skills/move-codex-session/scripts/move-codex-session.test.ts did not reach those code paths here because this environment lacks lsof (spawnSync lsof ENOENT), so the failure is established by static path tracing rather than an execution of the alias case. The decisive check is running this test on its supported environment and observing the error path for a passed symlink; if the command canonicalizes that path before the shown code, this claim is refuted, but the inspected source does not do so.

lens test-trimming · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M4JE5MY5836NR93CHEAM55RW of review 01M4JE0RED3FNQY1WDMN81VMQF

<!-- review:claim:01M4JE5MY5836NR93CHEAM55RW --> **low** — The alias cases expect the physical path instead of the path passed to the script > The `passed: "alias-a"` cases make this test expect the real source path even though the command receives the symlink path, so the test rejects the intended refusal diagnostic. `parseArguments` applies `resolve(sourceHome)`, which cleans the alias lexically without resolving it, and `requireNoGoalAttachmentReferences` builds the reported path from `join(sourceHome, "attachments")`; therefore for those cases the diagnostic includes `alias-a/attachments`, not `fixture.sourceHome/attachments`. The test still checks the refusal status, but its exact-stderr assertion makes the added alias coverage fail on the valid behavior. Build the expected diagnostic from `spellings[passed]` (or assert the refusal and referenced UUID/file without fixing the source-home spelling). I traced the four cases in this test to `parseArguments` and the diagnostic construction in `requireNoGoalAttachmentReferences`; running `node --test skills/move-codex-session/scripts/move-codex-session.test.ts` did not reach those code paths here because this environment lacks `lsof` (`spawnSync lsof ENOENT`), so the failure is established by static path tracing rather than an execution of the alias case. The decisive check is running this test on its supported environment and observing the error path for a passed symlink; if the command canonicalizes that path before the shown code, this claim is refuted, but the inspected source does not do so. lens `test-trimming` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M4JE5MY5836NR93CHEAM55RW` of review `01M4JE0RED3FNQY1WDMN81VMQF`
Author
Owner

Not a defect: main canonicalizes both homes with realpathSync(args.sourceHome) before the check (move-codex-session.ts, line 2115), so for passed: "alias-a" the diagnostic names the physical path, and the test's fixture.sourceHome is right. All four alias cases pass in the PR's Node tests check and locally. The claim assumed resolve() in parseArguments is the only normalization; the lsof failure kept the reviewer from running the case.

<!-- gh-feedback:reply-to:151688 --> Not a defect: `main` canonicalizes both homes with `realpathSync(args.sourceHome)` before the check (`move-codex-session.ts`, line 2115), so for `passed: "alias-a"` the diagnostic names the physical path, and the test's `fixture.sourceHome` is right. All four alias cases pass in the PR's `Node tests` check and locally. The claim assumed `resolve()` in `parseArguments` is the only normalization; the lsof failure kept the reviewer from running the case.
jercik marked this conversation as resolved
Lines 961-963
@ -941,0 +958,6 @@
const goal = select.get(id) as { objective: string } | undefined;
if (goal === undefined) continue;
for (const directory of directories) {
const marker = `/attachments/${directory}/`;
const start = goal.objective.indexOf(marker);
if (start === -1) continue;

medium — Unrelated attachment paths can block valid session moves

A moving goal that refers to an attachment in another home can make the move refuse even though the referenced file will remain available. This blocks a valid migration and reports the wrong source attachment as the blocker.

The check searches only for /attachments/<directory>/ and ignores the path prefix. For example, if the source contains attachments/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/, a goal pointing to /other-home/attachments/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/goal-objective.md is classified as a source reference. This can occur when another home is a copy with the same attachment UUIDs.

The check should establish that the cited path resolves to the source attachment directory while still handling source-home symlink aliases; a match on a UUID directory name alone is insufficient.

I traced requireNoGoalAttachmentReferences through both calls in main: the preflight check runs before locks, then the same predicate runs under writer locks before copying. The added tests cover source aliases and a destination directory with a different UUID, but not a matching UUID in another path. I attempted the script test suite with Node 26; it could not exercise the move logic because this environment lacks lsof (spawnSync lsof ENOENT), so this conclusion is from the call-path and condition in the source rather than a successful reproduction.

A fixture with the same UUID directory in another home and a goal that cites that home would establish the false refusal; resolving the full citation to a different home would refute it.

lens general-bug · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M4JE4GCC5WRJ3AMK0HGXN22Z of review 01M4JE0RED3FNQY1WDMN81VMQF

<!-- review:claim:01M4JE4GCC5WRJ3AMK0HGXN22Z --> **medium** — Unrelated attachment paths can block valid session moves > A moving goal that refers to an attachment in another home can make the move refuse even though the referenced file will remain available. This blocks a valid migration and reports the wrong source attachment as the blocker. > > The check searches only for <code>/attachments/&lt;directory&gt;/</code> and ignores the path prefix. For example, if the source contains `attachments/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/`, a goal pointing to `/other-home/attachments/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/goal-objective.md` is classified as a source reference. This can occur when another home is a copy with the same attachment UUIDs. > > The check should establish that the cited path resolves to the source attachment directory while still handling source-home symlink aliases; a match on a UUID directory name alone is insufficient. > > I traced `requireNoGoalAttachmentReferences` through both calls in `main`: the preflight check runs before locks, then the same predicate runs under writer locks before copying. The added tests cover source aliases and a destination directory with a different UUID, but not a matching UUID in another path. I attempted the script test suite with Node 26; it could not exercise the move logic because this environment lacks `lsof` (`spawnSync lsof ENOENT`), so this conclusion is from the call-path and condition in the source rather than a successful reproduction. > > A fixture with the same UUID directory in another home and a goal that cites that home would establish the false refusal; resolving the full citation to a different home would refute it. lens `general-bug` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M4JE4GCC5WRJ3AMK0HGXN22Z` of review `01M4JE0RED3FNQY1WDMN81VMQF`
Author
Owner

Deliberate. The first version matched on the source home's path, and review reproduced a miss: Codex does not resolve symlinks in the default ~/.codex, so a goal can cite the source home by an alias that matches neither --source-home nor its real path, and the files were then lost silently. Matching on the attachment directory names avoids that. Those names are random UUIDs, so one appears under two homes only when a home is a copy of another, and the refusal is loud: the error names the thread and file, and SKILL.md gives the remedy (/goal clear, or a shorter goal). A false refusal costs one rerun; a false pass loses files. Resolving the cited prefix would mean extracting a path from free text, where spaces make its start ambiguous.

<!-- gh-feedback:reply-to:151687 --> Deliberate. The first version matched on the source home's path, and review reproduced a miss: Codex does not resolve symlinks in the default `~/.codex`, so a goal can cite the source home by an alias that matches neither `--source-home` nor its real path, and the files were then lost silently. Matching on the attachment directory names avoids that. Those names are random UUIDs, so one appears under two homes only when a home is a copy of another, and the refusal is loud: the error names the thread and file, and `SKILL.md` gives the remedy (`/goal clear`, or a shorter goal). A false refusal costs one rerun; a false pass loses files. Resolving the cited prefix would mean extracting a path from free text, where spaces make its start ambiguous.
jercik marked this conversation as resolved
test: move-codex-session snapshot helper should explain only its WAL exclusion
All checks were successful
commit-msg / commitlint (pull_request) Successful in 17s
Review / Review (pull_request_target) Successful in 3m19s
Node tests / node:test (pull_request) Successful in 4m17s
7943249911
The helper's comment restated what the code below it records. The reason the
WAL companions are skipped now sits beside the skip.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Lines 961-962
@ -941,0 +958,5 @@
const goal = select.get(id) as { objective: string } | undefined;
if (goal === undefined) continue;
for (const directory of directories) {
const marker = `/attachments/${directory}/`;
const start = goal.objective.indexOf(marker);

low — Goals citing another home’s attachment are refused when its directory ID collides

A move can be refused even though the moved goal's referenced file remains in the destination or another home and will not be lost with the source. The check searches only for /attachments/<entry>/ in the objective; it never verifies that the text before that marker names the source home. For example, if both homes contain attachments/<same-id>/ and a goal cites <destination-home>/attachments/<same-id>/goal.md, this condition matches the source entry and aborts the move.

The guard should establish that the cited path resolves to the source home’s attachment directory, while retaining support for source-home aliases that Codex may write into objectives. I traced requireNoGoalAttachmentReferences and its two calls from main; the check runs during preflight and again after writer locks, but both use this same marker-only match. The skill documentation says the blocker is a goal citing a file under the source attachments directory. I did not run a reproduction; the collision behavior follows directly from indexOf and is conditional on the same attachment directory name existing in both locations. A reproduction with that duplicate directory name would establish the false refusal; path-aware matching with symlink and lexical aliases covered would refute it.

lens general-bug · arm default · tally 1 valid / 0 invalid / 0 uncertain
claim 01M4JEMG7GYE1MAM4ZAAQE0DAF of review 01M4JEJM2EZVMYD0793EKH734D

<!-- review:claim:01M4JEMG7GYE1MAM4ZAAQE0DAF --> **low** — Goals citing another home’s attachment are refused when its directory ID collides > A move can be refused even though the moved goal's referenced file remains in the destination or another home and will not be lost with the source. The check searches only for <code>/attachments/&lt;entry&gt;/</code> in the objective; it never verifies that the text before that marker names the source home. For example, if both homes contain <code>attachments/&lt;same-id&gt;/</code> and a goal cites <code>&lt;destination-home&gt;/attachments/&lt;same-id&gt;/goal.md</code>, this condition matches the source entry and aborts the move. > > The guard should establish that the cited path resolves to the source home’s attachment directory, while retaining support for source-home aliases that Codex may write into objectives. I traced `requireNoGoalAttachmentReferences` and its two calls from `main`; the check runs during preflight and again after writer locks, but both use this same marker-only match. The skill documentation says the blocker is a goal citing a file under the source attachments directory. I did not run a reproduction; the collision behavior follows directly from `indexOf` and is conditional on the same attachment directory name existing in both locations. A reproduction with that duplicate directory name would establish the false refusal; path-aware matching with symlink and lexical aliases covered would refute it. lens `general-bug` · arm `default` · tally 1 valid / 0 invalid / 0 uncertain claim `01M4JEMG7GYE1MAM4ZAAQE0DAF` of review `01M4JEJM2EZVMYD0793EKH734D`
Author
Owner

Deliberate. The first version matched on the source home's path, and review reproduced a miss: Codex does not resolve symlinks in the default ~/.codex, so a goal can cite the source home by an alias that matches neither --source-home nor its real path, and the files were then lost silently. Matching on the attachment directory names avoids that. Those names are random UUIDs, so one appears under two homes only when a home is a copy of another, and the refusal is loud: the error names the thread and file, and SKILL.md gives the remedy (/goal clear, or a shorter goal). A false refusal costs one rerun; a false pass loses files. Resolving the cited prefix would mean extracting a path from free text, where spaces make its start ambiguous.

<!-- gh-feedback:reply-to:151696 --> Deliberate. The first version matched on the source home's path, and review reproduced a miss: Codex does not resolve symlinks in the default `~/.codex`, so a goal can cite the source home by an alias that matches neither `--source-home` nor its real path, and the files were then lost silently. Matching on the attachment directory names avoids that. Those names are random UUIDs, so one appears under two homes only when a home is a copy of another, and the refusal is loud: the error names the thread and file, and `SKILL.md` gives the remedy (`/goal clear`, or a shorter goal). A false refusal costs one rerun; a false pass loses files. Resolving the cited prefix would mean extracting a path from free text, where spaces make its start ambiguous.
jercik marked this conversation as resolved
jercik merged commit db07efd6b3 into main 2026-10-10 08:28:51 +00:00
jercik deleted branch fix/move-codex-session-goal-attachments 2026-10-10 08:28:52 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
j4k-oss/agent-skills!147
No description provided.