[codex] Clean Knowledge Loop workspace after failures#34
Conversation
![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: 9292db1ece
ℹ️ 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".
| runner: CommandRunner, | ||
| report: KnowledgeLoopReport, | ||
| ) -> None: | ||
| _run_checked(["git", "clean", "-fd", "--", *_MANAGED_DIRTY_PREFIXES], repo, runner, report) |
There was a problem hiding this comment.
When --create-pr is used with a repo-relative --learning-event path under one of the managed prefixes (for example a freshly dropped ledger/fixtures/*.json file, which the CLI accepts as a file/dir/glob input), the loop counts the event before _prepare_base_checkout, then this git clean -fd -- okf exports reports evals ledger schema deletes the untracked input before _run_phase2_imports invokes ledger import on the same path. That makes the import fail with a missing/no-match input and also loses the sanitized artifact; exclude configured learning-event paths from the preflight clean or require/clean only output locations after import.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca803ddac9
ℹ️ 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".
| ) -> None: | ||
| excludes = _managed_clean_excludes(repo, exclude_paths) | ||
| exclude_args = [arg for path in excludes for arg in ("-e", path)] | ||
| _run_checked(["git", "clean", "-fd", *exclude_args, "--", *_MANAGED_DIRTY_PREFIXES], repo, runner, report) |
There was a problem hiding this comment.
When --learning-event points at a repo-local file under a managed prefix (for example the new test’s ledger/incoming/one.json), this git clean call preserves it via -e; checked git clean -h, where -e, --exclude <pattern> adds an ignore rule for cleaning only. Nothing later excludes that same path from _git_dirty or _publish_pr's git add ledger, so a successful publishing run can publish the input artifact and a failed validation cleanup still leaves ?? ledger/incoming/ dirty, reproducing the blocked-checkout condition this change is meant to fix.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65de161da6
ℹ️ 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".
| report.detail = phase2_gate | ||
| report.ledger = ledger | ||
| return report | ||
| learning_event_import_paths = _snapshot_repo_local_managed_learning_events( |
There was a problem hiding this comment.
This snapshot happens after _learning_event_count() has already loaded the operator-supplied files. In a publishing run with a repo-local managed input such as ledger/incoming/bad.json, malformed JSON or an unreadable file raises before any snapshot is written, and the new exception cleanup then runs git clean -fd -- ... ledger ..., deleting the only untracked input the operator needs to fix. Move the backup ahead of the load/count step, or skip destructive managed cleanup until inputs have been preserved.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a619e97ee
ℹ️ 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".
| runner: CommandRunner, | ||
| report: KnowledgeLoopReport, | ||
| ) -> None: | ||
| _run_checked(["git", "clean", "-fd", "--", *_MANAGED_DIRTY_PREFIXES], repo, runner, report) |
There was a problem hiding this comment.
In a publishing-capable run (--create-pr) on a checkout that has a new untracked curated concept such as okf/curated/..., this git clean -fd -- okf ... removes it before the loop does any work; git clean -h documents -d as removing whole directories for supplied pathspecs. Since this repo’s AGENTS.md says okf/curated/ is human-edited OKF, cleaning the whole okf prefix can discard user-owned knowledge, so the clean should be restricted to loop-generated subtrees or explicitly exclude curated OKF.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4285b5dc62
ℹ️ 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".
| continue | ||
| target = snapshot_root / rel | ||
| target.parent.mkdir(parents=True, exist_ok=True) | ||
| shutil.copy2(source, target) |
There was a problem hiding this comment.
When --learning-event ledger/incoming/* matches a subdirectory (the loop CLI accepts a file, directory, or glob, and the loader handles directories), expand_event_paths returns that directory and this copy2 raises IsADirectoryError before the budget count/import runs. This makes publishing-capable runs fail for batched incoming events organized in subdirectories under ledger/incoming; copy directories recursively or expand them to JSON files before copying.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4285b5dc62
ℹ️ 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".
|
|
||
| # Generated/exported paths the loop is allowed to refresh and republish. | ||
| _MANAGED_DIRTY_PREFIXES = ("okf", "exports", "reports", "evals", "ledger", "schema") | ||
| _LOOP_OUTPUT_PREFIXES = ("okf/generated", "okf/observed", "exports", "reports", "ledger/proposed", "ledger/reviews") |
There was a problem hiding this comment.
When ingest runs, it always calls ensure_curated_indexes() (src/hyrule_knowledge/cli.py:358), which rewrites okf/curated/index.md and each curated section index.md. This new output list excludes those generated index files, so _git_dirty can still report the checkout dirty while _publish_pr stages only _LOOP_OUTPUT_PREFIXES; if a curated index update is the only change, no PR is opened, and if there are other output changes, the PR omits the index edits and leaves the persistent workspace dirty. Include the generated curated index paths in the staged outputs or explicitly ignore/revert them.
Useful? React with 👍 / 👎.
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d890ec931
ℹ️ 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".
| _LOOP_OUTPUT_PREFIXES = ( | ||
| "okf/generated", | ||
| "okf/observed", | ||
| *_CURATED_INDEX_OUTPUTS, |
There was a problem hiding this comment.
For publishing-capable loop runs, cmd_ingest() still appends curated_seed_concepts() and write_concepts() rewrites files such as okf/curated/architecture/as215932-system-map.md on every ingest (including last_verified_at churn), but this managed set now includes only curated index files. Those curated seed page modifications are therefore treated as unmanaged by _drop_volatile_only_changes() and are not staged by _publish_pr(), so a normal refresh can still finish with dirty tracked curated pages after a PR/no-PR path—the cleanup regression this change is trying to prevent.
Useful? React with 👍 / 👎.
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ba1e6bd6d
ℹ️ 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".
| continue | ||
| target = snapshot_root / rel | ||
| if source.is_dir(): | ||
| shutil.copytree(source, target, dirs_exist_ok=True) |
There was a problem hiding this comment.
When a --learning-event glob matches a directory, this copytree(..., dirs_exist_ok=True) merges into any snapshot left from the same run_id instead of replacing it. In a persistent state dir, GitHub reruns can hit the same run id (GitHub docs say run_id “does not change if you re-run” the workflow: https://docs.github.com/en/actions/reference/workflows-and-actions/contexts), so a file removed from ledger/incoming/batch remains in the snapshot and is still imported because import_paths points at the snapshot directory. Remove the target directory before copying, or include a rerun-attempt/unique suffix in the snapshot path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 7306bd4: directory snapshots now replace any existing snapshot target before copytree, so reruns with the same run_id cannot import stale files left from a previous directory snapshot. Added regression coverage in tests/test_knowledge_loop.py for stale snapshot removal.
| snapshot_root = state_dir / "learning-event-inputs" / safe_run_id | ||
| import_paths: list[Path] = [] | ||
| for path in paths: | ||
| if not _is_repo_local_managed_path(repo, path): |
There was a problem hiding this comment.
This prefix check runs on the unexpanded input path, so supported globs that do not literally start with a cleaned prefix, such as ledger/*/*.json, can match files under ledger/incoming but skip the snapshot path entirely. A publishing run then executes git clean ... ledger/incoming before ledger import, leaving the original glob empty even though the budget preflight counted the event; expand globs before deciding whether each matched source needs to be copied into state.
Useful? React with 👍 / 👎.
| runner: CommandRunner, | ||
| report: KnowledgeLoopReport, | ||
| ) -> None: | ||
| _run_checked(["git", "clean", "-fd", "--", *_LOOP_CLEAN_PREFIXES], repo, runner, report) |
There was a problem hiding this comment.
If operators configure a repo-local state dir under a cleaned output prefix, for example --state-dir reports/loop-state, this git clean -fd -- reports ... runs after the lock is acquired and removes reports/loop-state/knowledge-loop.lock (local git clean -h documents -d as “remove whole directories”). That drops the singleton lock while the cycle is still running, so a second invocation can acquire it and concurrently reset/publish; exclude the resolved state dir from the clean pathspecs or reject state dirs under managed clean prefixes.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7306bd4624
ℹ️ 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".
| try: | ||
| _run_checked(["git", "reset", "--hard"], repo, runner, report) | ||
| _clean_managed_untracked(repo, state_dir, runner, report) | ||
| _run_checked(["git", "checkout", config.base_branch], repo, runner, report) |
There was a problem hiding this comment.
When a --create-pr --learning-event ledger/incoming/foo.json run imports successfully but a later validation command fails, this cleanup removes the untracked ledger/proposed/*.json created by ledger import; the original ledger/incoming input was already cleaned after being snapshotted, and the learning-event budget is charged before this error path returns. The event is therefore neither included in a PR nor available at the original CLI path for the next retry unless an operator manually recovers the state snapshot, so post-import failures can effectively drop learning artifacts.
Useful? React with 👍 / 👎.
| _run_checked(["git", "fetch", "origin", config.base_branch], repo, runner, report) | ||
| _run_checked(["git", "checkout", "-B", branch, f"origin/{config.base_branch}"], repo, runner, report) | ||
| _run_checked(["git", "add", *_MANAGED_DIRTY_PREFIXES], repo, runner, report) | ||
| _run_checked(["git", "add", *_LOOP_OUTPUT_PREFIXES], repo, runner, report) |
There was a problem hiding this comment.
If the persistent checkout contains an untracked curated page such as okf/curated/architecture/new-page.md, ensure_curated_indexes() will include it in the regenerated section index, but this git add stages only _LOOP_OUTPUT_PREFIXES (indexes and seed pages) and omits that page. A publishing run can then open a PR that adds an index link to a file not present in the PR, while the untracked page remains in the workspace and keeps influencing later runs.
Useful? React with 👍 / 👎.
| if not config.create_pr or config.dry_run: | ||
| return | ||
| _run_checked(["git", "fetch", "origin", config.base_branch], repo, runner, report) | ||
| _run_checked(["git", "reset", "--hard"], repo, runner, report) |
There was a problem hiding this comment.
For publishing-capable runs, this unconditional reset happens before any path-scoped cleanup; checked git reset -h, where --hard is documented as resetting HEAD, index and working tree. If the persistent checkout has local tracked edits outside the loop output set, such as knowledge.config.yml or a repo-local tracked learning-event input, they are silently discarded instead of being preserved or rejected, even though the managed cleanup below is otherwise path-limited.
Useful? React with 👍 / 👎.
| if state_prefix and _matches_prefix(state_prefix, _LOOP_OUTPUT_PREFIXES): | ||
| _run_checked(["git", "reset", "--", state_prefix], repo, runner, report) |
There was a problem hiding this comment.
When the loop state directory is configured to a managed output root itself, e.g. HYRULE_KNOWLEDGE_LOOP_STATE_DIR=reports, state_prefix is reports, so this reset unstages every report just added rather than only private state files. Because the same prefix is also excluded from cleanup, normal quality, eval, and ledger report updates can be left dirty and omitted from the PR whenever state is placed at that output root.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec0116455f
ℹ️ 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".
| _run_checked(["git", "fetch", "origin", config.base_branch], repo, runner, report) | ||
| _run_checked(["git", "checkout", "-B", branch, f"origin/{config.base_branch}"], repo, runner, report) | ||
| _run_checked(["git", "add", *_MANAGED_DIRTY_PREFIXES], repo, runner, report) | ||
| _run_checked(["git", "add", *_dedupe_strings((*_LOOP_OUTPUT_PREFIXES, *_curated_index_referenced_pages(repo)))], repo, runner, report) |
There was a problem hiding this comment.
When a curated page that was previously generated and committed is removed or renamed, this only stages the current index files plus pages that still exist and are linked from the new indexes. Because _curated_index_referenced_pages() skips missing targets, the deletion of the old okf/curated/.../*.md file is never staged, so the PR can update the index while leaving stale curated content in the repository.
Useful? React with 👍 / 👎.
| runner: CommandRunner, | ||
| report: KnowledgeLoopReport, | ||
| ) -> None: | ||
| _run_checked(["git", "clean", "-fd", *_state_dir_clean_excludes(repo, state_dir), "--", *_LOOP_CLEAN_PREFIXES], repo, runner, report) |
There was a problem hiding this comment.
If a publishing run creates a new curated page referenced by a generated index and then fails validation, the error cleanup resets the index back to origin/main before running this git clean, but the clean pathspecs do not include arbitrary referenced okf/curated/.../*.md pages. Those untracked pages are left in the persistent workspace and can keep later deploy checkouts dirty, which is the failure mode this cleanup is meant to prevent.
Useful? React with 👍 / 👎.
| update_ledger(state_dir, learning_events_imported=phase2_learning_events_to_charge) | ||
| if report.openrouter_calls_run and not phase2_openrouter_charged: | ||
| update_ledger(state_dir, openrouter_calls=report.openrouter_calls_run) | ||
| cleanup_error = _restore_base_checkout_after_error(repo, config, state_dir, active_runner, report) |
There was a problem hiding this comment.
When a create_pr run produces real changes but the PR budget is already exhausted (for example max_cycles_per_day > max_prs_per_day or max_prs_per_day=0), control returns from the PR-budget gate instead of raising, so the cleanup added here in the exception path is skipped. That leaves the generated okf/exports/reports changes dirty in the persistent checkout even though the run cannot publish them, which can still block the next deploy checkout.
Useful? React with 👍 / 👎.
| if report.openrouter_calls_run and not phase2_openrouter_charged: | ||
| update_ledger(state_dir, openrouter_calls=report.openrouter_calls_run) | ||
| cleanup_error = _restore_base_checkout_after_error(repo, config, state_dir, active_runner, report) | ||
| restore_error = _restore_learning_import_outputs(repo, learning_import_output_snapshot) |
There was a problem hiding this comment.
When a publishing-capable run imports learning events successfully and then fails validation, cleanup first resets/cleans the checkout but this call copies the imported ledger/proposed or ledger/reviews files back afterward. That leaves the persistent workspace dirty on an error outcome, so the same validation-failure scenario can still block later deploy checkouts whenever the cycle included a learning-event import.
Useful? React with 👍 / 👎.
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cfb1ae483b
ℹ️ 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".
| ) | ||
| # Additional transient inputs that may be removed only after they have been | ||
| # copied into state for the current run. | ||
| _LOOP_CLEAN_PREFIXES = (*_LOOP_OUTPUT_PREFIXES, "ledger/incoming") |
There was a problem hiding this comment.
When a publishing run has files in ledger/incoming that are not included in this run's learning_event_paths (including the case where no --learning-event is configured), this new clean prefix makes _clean_managed_untracked run git clean -fd -- ... ledger/incoming and delete those untracked inbox files without snapshotting them. I checked git clean -h: -f forces cleaning and -d removes directories, so the whole inbox pathspec is destructive; only the configured paths are copied by _snapshot_repo_local_managed_learning_events, which can lose sanitized events waiting for a later import.
Useful? React with 👍 / 👎.
| if prefix in _LOOP_CLEAN_PREFIXES: | ||
| excludes: list[str] = [] | ||
| for name in _STATE_PRIVATE_FILES: | ||
| excludes.extend(("-e", f"{prefix}/{name}")) | ||
| for glob in _STATE_PRIVATE_GLOBS: | ||
| excludes.extend(("-e", f"{prefix}/{glob}")) | ||
| for name in _STATE_PRIVATE_DIRS: | ||
| excludes.extend(("-e", f"{prefix}/{name}", "-e", f"{prefix}/{name}/**")) | ||
| return excludes |
There was a problem hiding this comment.
When state_dir is set to an output root that this branch now handles specially, such as reports, this branch only excludes private state files from cleaning/staging, but _git_dirty still ignores the entire state prefix. In that configuration, a cycle that only refreshes report outputs like reports/coverage.md or eval reports is treated as clean and no PR is opened, so generated report updates are silently dropped; the dirty-state filter needs the same private-file granularity as the clean/stage paths.
Useful? React with 👍 / 👎.
| else: | ||
| target.parent.mkdir(parents=True, exist_ok=True) | ||
| shutil.copy2(source, target) | ||
| import_paths.append(target) |
There was a problem hiding this comment.
For a repo-local input such as ledger/incoming/one.json, this now imports the copied cache path instead of the operator-supplied path, and import_learning_events records that argument in the proposed event's source.path. The committed ledger/proposed/*.json will therefore point reviewers at .cache/.../learning-event-inputs/... (or another private state directory) rather than the original inbox artifact, so the PR loses reviewable provenance for the imported learning event even when the import succeeds.
Useful? React with 👍 / 👎.
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7320671f80
ℹ️ 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".
| return None | ||
| try: | ||
| extra_clean_paths = _curated_index_referenced_pages(repo) | ||
| _reject_unmanaged_tracked_changes(repo, state_dir, runner, report) |
There was a problem hiding this comment.
When a publishing run fails after modifying an already tracked curated page that is linked from an index but not listed in _LOOP_CLEAN_PREFIXES (for example okf/curated/policies/loop-knowledge-gap-routing.md), this pre-reset reject runs before the reset/clean path. The publish path explicitly stages _curated_index_referenced_pages(...), so these are loop-managed changes; rejecting them here makes cleanup abort and leaves the dirty worktree for the next cycle.
Useful? React with 👍 / 👎.
| snapshot = state_dir / "learning-event-inbox" / safe_run_id / "ledger" / "incoming" | ||
| if snapshot.exists(): | ||
| shutil.rmtree(snapshot) | ||
| shutil.copytree(inbox, snapshot) |
There was a problem hiding this comment.
When a publishing run is configured with --state-dir inside ledger/incoming (for example ledger/incoming/state), acquire_lock creates that directory, so the inbox exists even with no events. This copytree then copies ledger/incoming into one of its own descendants and recurses until it errors, making every create-PR cycle fail before cleanup; skip the state subtree or place this snapshot outside the inbox.
Useful? React with 👍 / 👎.
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56b92c44fc
ℹ️ 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".
| shutil.rmtree(target) | ||
| else: | ||
| target.unlink() | ||
| shutil.copytree(source, target) |
There was a problem hiding this comment.
When --state-dir is placed under ledger/incoming and --learning-event is a glob such as ledger/incoming/*, the expanded inputs include the state directory itself. This copytree then tries to copy that directory into a child under its own learning-event-inputs/... snapshot, recursively nesting until the loop raises an OSError before importing the real event. Filter the configured state directory out of learning-event input snapshots, as the inbox snapshot path already does.
Useful? React with 👍 / 👎.
| child = path.removeprefix(f"{prefix}/") | ||
| for glob in _STATE_PRIVATE_GLOBS: | ||
| if "/" not in child and fnmatchcase(child, glob): | ||
| return True |
There was a problem hiding this comment.
When the state directory is exactly a managed output root such as --state-dir reports, this branch evaluates the private glob against any repo path because it never first checks that path is under reports/. For example, a real top-level ?? ledger-2026-07-06.json is treated as private state and _git_dirty can report an idle cycle even though the worktree has an untracked file outside the configured state directory.
Useful? React with 👍 / 👎.
| extra_clean_paths = _curated_index_referenced_pages(repo) | ||
| _reject_unmanaged_tracked_changes(repo, state_dir, runner, report, extra_managed_paths=extra_clean_paths) |
There was a problem hiding this comment.
The publish path explicitly treats deleted tracked curated pages as stageable output via _deleted_curated_pages, but the failure cleanup only whitelists currently referenced pages, so a validation or PR-budget failure after a run deletes okf/curated/.../old-page.md will make _reject_unmanaged_tracked_changes see D old-page.md as unmanaged and abort the restore. Include deleted curated pages in this cleanup whitelist as well, otherwise the failure path can leave the same dirty workspace this change is meant to clear.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
tests/test_knowledge_loop.pyRoot Cause
The production Knowledge Loop generated a refresh, failed validation, and returned without restoring the persistent workspace. That left
okf,exports, andreportsdirty, which later blocked the network-operations deploy checkout.Validation
uv run pytest tests/test_knowledge_loop.pyuv run pytestuv run ruff check src tests