fix: repair authored skill safety and integration contracts - #54
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d0350d8b4
ℹ️ 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".
| These calls may write candidate artifacts but do not acquire a canon lease. | ||
| Do not add unsupported expected_state_hash / command_schema_version arguments; | ||
| the server constructs a versioned MutationEnvelope from the supported fields. |
There was a problem hiding this comment.
When generation or repair targets an existing project's shot, this recipe explicitly forbids expected_state_hash, while queue_generation and run_shot_repair_loop construct envelopes without one and may persist a candidate. A concurrent re-ingest can therefore change the contract after before is read; the final assertion detects the change only after generation and persistence have occurred. Accept or internally construct and validate the freshly read project state_hash before performing either write.
AGENTS.md reference: AGENTS.md:L64-L72
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 3771e2c8a060fb7331f0d2419b19d4f01b6bbb98.
Candidate generation and repair now accept/bind expected_state_hash, validate before provider execution, and retain one process-local store lock through persistence, including a re-entrant state recheck. This is not a distributed-lock guarantee. Race/repair regressions passed.
Independent scoped review passed; parent full M0 gate passed with declared CI dependencies (269 tests passed, 2 skipped). Current hosted M0, Postgres/MinIO, and wheel/sdist checks are successful. No merge performed.
| for path, value in changes.items(): | ||
| with tempfile.NamedTemporaryFile(dir=path.parent, delete=False) as stream: | ||
| staged[path] = Path(stream.name) |
There was a problem hiding this comment.
On Unix, NamedTemporaryFile creates these staged files with mode 0600, and os.replace transfers that mode to every evolved sidecar and index. Applying evolution to a group-readable or service-owned corpus therefore makes the files inaccessible to other readers; the rollback path creates the same mode change while claiming to restore prior state. Copy each original file's mode onto its staged and rollback replacement before installing it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 3771e2c8a060fb7331f0d2419b19d4f01b6bbb98.
Both evolution engines now preserve original POSIX permission bits on staged and rollback replacements. Regressions inspect replacement sources and final modes, including failure after both corpus files are replaced.
Independent scoped review passed; parent full M0 gate passed with declared CI dependencies (269 tests passed, 2 skipped). Current hosted M0, Postgres/MinIO, and wheel/sdist checks are successful. No merge performed.
1 participant
Summary
Make embedded/package Kubrick evolution plan-first, idempotent, evidence-bound and recovery-aware; preserve index entries; repair profile installation and MCP/writing recipes.
Verification
python -m pytest -q -ra --tb=short -o addopts= tests -m 'not integration'in an isolated Python 3.12 environment.Scope and limits
Original working checkouts and installed profiles remain untouched; tests use disposable fixtures. No production deployment, live approval/enrollment, paid provider use, merge or release was performed. This is not a full production-service or crash-atomicity certification. Existing license grants are unchanged.
Live Postgres/MinIO integrations were deselected. Installed-wheel dry-run/apply/replay smoke also passed.