Repository navigation
Pin the knowledge snapshot for Strict runs at reservation - #213
Conversation
Strict and quality runs read knowledge through settings.knowledge_dir, which
is always the packaged tree, so they ignored an activated knowledge snapshot.
Their results also never said which knowledge they used. A run that sits in
the queue (or is relaunched after a restart) must also not pick up whatever
became active later.
- RunService records the current KnowledgeView in <run>/knowledge.json when
it creates a Strict or quality reservation (idempotent retries do not
rewrite it). The record holds the snapshot id, manifest tree hash and
resolved real paths, never the `active` symlink.
- _launch points the child's KNOWLEDGE_DIR and KNOWLEDGE_ROOT at the pinned
snapshot. If that snapshot was pruned, the run is marked failed and never
launched, instead of silently reviewing with different knowledge. Without a
snapshot root, the run is pinned to the server's effective knowledge_dir
("packaged", or "unverified" for a custom one) with KNOWLEDGE_ROOT removed,
so a relaunch under a newly configured root cannot report "packaged"
while reviewing with a snapshot. Runs
reserved before this change carry no record and keep the process default.
- build_review_result reports knowledge_snapshot and knowledge_tree_sha256 in
diagnostics, so the review bot can record the knowledge behind every Strict
review, as it already does for Direct plans.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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: 68400a8fb9
ℹ️ 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".
| # the snapshot pinned at reservation, even if another is active now; | ||
| # a snapshot pruned since then fails the run instead of silently | ||
| # reviewing with different knowledge | ||
| if not (Path(pin["knowledge_dir"]) / "AGENTS.md").is_file(): |
There was a problem hiding this comment.
When a verified snapshot's file contents are modified without updating its manifest, this check still succeeds because it tests only for AGENTS.md, and the child then consumes KNOWLEDGE_DIR through raw Path/KnowledgeDocs reads rather than KnowledgeView.path(). The run can therefore review with tampered knowledge while reporting the original manifest hash; validate the pinned manifest and file hashes before launch, or route child reads through the verified view.
Useful? React with 👍 / 👎.
| run_id, created = self.reservations.reserve( | ||
| spec, owner_server_id=self.server_id, owner_server_pid=self.pid, | ||
| idempotency_key=str(spec_dict.get("idempotency_key") or "")) | ||
| if created: | ||
| if created and self._pin_or_fail(run_id): |
There was a problem hiding this comment.
If the server exits after reservations.reserve() persists the queued run but before _pin_or_fail() writes knowledge.json, a later idempotent retry can reclaim the run with created=True; _pin_knowledge() then sees no existing pin and records whichever snapshot is active at retry time. This violates the reservation-time binding specifically in the restart scenario the durable queue supports, so the pin must be part of reservation persistence or a reclaimed pinless post-change run must fail rather than be repinned.
Useful? React with 👍 / 👎.
PR 10 of the knowledge-service redesign: Strict (and quality) runs now review with the knowledge snapshot that was active when they were reserved, and report it.
Before this change, Strict read knowledge through
settings.knowledge_dir, which is always the packaged tree or a configured directory. It ignored activated snapshots, and its results didn't say which knowledge was used.KnowledgeViewin<run>/knowledge.json: the snapshot id, manifest tree hash, and resolved real paths (never theactivesymlink). Idempotent retries and reclaimed never-started reservations keep the original pin. If pinning fails, the run fails with the reason instead of staying queued forever.KNOWLEDGE_DIRandKNOWLEDGE_ROOTpoint at the pin. If the pinned snapshot has been pruned, the run is failed and never launched. Without a snapshot root, the run is pinned to the server's effectiveknowledge_dir(packaged, orunverifiedfor a custom directory), withKNOWLEDGE_ROOTremoved.build_review_resultreportsknowledge_snapshotandknowledge_tree_sha256indiagnostics. The review bot records these for Strict reviews in its upcoming trace PR.Commit hook approved; full suite and SPEC freshness pass locally.
🤖 Generated with Claude Code