Skip to content

fix: preserve resync absence and profile admission safety - #364

Merged
schickling merged 4 commits into
mainfrom
fix/resync-late-review-safety
Aug 28, 2026
Merged

fix: preserve resync absence and profile admission safety#364
schickling merged 4 commits into
mainfrom
fix/resync-late-review-safety

Conversation

@schickling-assistant

@schickling-assistant schickling-assistant commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Problem

Two late reviews found safety gaps after #345 and #351 merged. Resync treated deletion and stable non-regular replacement like transient unreadability, so it emitted no tombstone and suppressed same-byte recreation. WASM refresh caching keyed only by module path, so an external admission could be reused by a catalog-relative declaration of the same spelling without descriptor-relative confinement.

Goal

Make carrier absence an observable, retry-safe state and make every WASM snapshot/compile cache respect its exact admission policy.

Decisions

  • Model a carrier as present(<sha256>) or missing; only ENOENT/stable non-regular observations establish missing. Permission and transient I/O failures retain prior state and retry, including failures during initial baseline seeding.
  • Render deletion canonically as old: <digest> / new: missing; missing-to-present is a creation even for the same prior bytes.
  • Key refresh and bounded compile/failure caches by normalized module identity plus external-or-exact-confinement-root policy, then file identity. Normalization is cache-only; admission opens the original declared spelling.
  • Preserve immutable pending bodies, occurrence identity, subscriber-local sequencing, and binding-key supersession.

Verification

  • nix develop -c cargo test --lib resync::tests:: — 31 passed.
  • nix develop -c cargo test -p agent-spec — 78 passed across unit/integration suites.
  • nix develop -c cargo test -p agent-spec --features wasm-resolver — 110 passed, including external-then-contained bypass, original-spelling admission, clone policy isolation, and replacement invalidation.
  • nix develop -c cargo test --test resync — 3 passed.
  • nix develop -c cargo test --features wasm-resolver --test resync — 5 passed.
  • nix develop -c cargo test --test catalog_apply profile_module — 7 passed.
  • nix develop -c cargo test --test catalog_config catalog_profile — 1 passed.
  • git diff --check — passed.

Pre-flip deviation: repository-wide cargo fmt --all -- --check reports pre-existing formatting drift across files untouched by this PR as well as surrounding lines in the two modified Rust files; no broad formatting churn is included here.

Complexity

No new dependency or module boundary. One explicit carrier-state enum and one compound cache-key type replace overloaded optional digest/path-only identity.

Concerns

Transient read failures emit a diagnostic at each retry deadline; this is intentional operational visibility but can be noisy during prolonged permission outages.

Friction & bottlenecks

Local Nix commands report an unavailable trace sink and repeatedly wait for automatic garbage collection. This did not affect test results.

Follow-ups

None.

References

Posted on behalf of @schickling
field value
agent_identity unknown
agent_persona unknown
agent_supervisor unavailable
agent_tool unknown
agent_tool_version unknown
agent_runtime unknown
tooling_profile dotfiles@e4789b0

@schickling-assistant

Copy link
Copy Markdown
Contributor Author

@codex review

Posted on behalf of @schickling
field value
agent_identity unknown
agent_persona unknown
agent_supervisor unavailable
agent_tool unknown
agent_tool_version unknown
agent_runtime unknown
tooling_profile dotfiles@e4789b0

@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: c282bc8e08

ℹ️ 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 crates/agent-spec/src/profile.rs Outdated
Comment thread src/resync.rs Outdated

@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: fc8bc0e0cc

ℹ️ 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 crates/agent-spec/src/profile.rs

@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: a7617d4dd0

ℹ️ 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 crates/agent-spec/src/profile.rs Outdated
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.

2 participants