Fix all 17 verified defects from the systematic bug scan - #34
Merged
Merged
Conversation
Medium: - Classify GitHub's startup_failure conclusion as failing so a workflow that fails to start alerts instead of pending forever (github_eval) - Allow workflow_call events as eligible required PR checks so reusable-workflow CI can complete a watch (github_client) - Map http.client.HTTPException to ProviderError so truncated model responses degrade instead of crashing watch add --intent (inference_api) - Treat a concurrent atomic token rotation as safe (stable regular file already opened) and keep the daemon alive through transient store-iteration failures (secrets_file, daemon) - Make config list survive invalid stored values and truncated JSON while keeping legacy plaintext secrets masked; guard config unset/keys against non-object config.json (cli, config) - Tokenize query and content symmetrically in relevant_to so the recorded PR identity matches remembered context (memory) Low: - Reject empty memory content; report hidden older items in memory list - Decline the watch-add confirmation on EOF stdin; report unusable secret stores cleanly across config and watch commands - Serialize config/secret read-modify-write cycles with sidecar flocks and write config.json atomically - Start the cooperative-stop window after startup-lock acquisition so queued time is not deducted from the stop budget (runner_control) - Classify truncated bodies as retryable network errors, local secret-store failures as blockers, and permanent 301/308 redirects as PR-not-found blockers instead of endless retries (github_client) - Default next_check_at at creation so an ACTIVE task is never persisted unschedulable (tasks) 535 tests pass (20 new regression tests); the first-use e2e demo passes all 8 scenarios.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #33 — all 17 verified defects from the systematic bug scan of post-#28
main.Full methodology and per-finding evidence are on the issue; this PR implements the fixes plus 20 new regression tests.
Medium fixes
startup_failureclassified as failing (core/github_eval.py) — a workflow that fails to start (e.g. invalid YAML in a PR) now emitsCHECKS_FAILEDinstead of presenting a real failure as "required checks pending" forever.workflow_calleligible (native/github_client.py) — reusable-workflow required checks can now complete a watch instead of staying PENDING on fully green CI.IncompleteRead→ProviderError(native/inference_api.py) — truncated model responses degrade per the documented contract instead of crashingwatch add --intent.native/secrets_file.py,native/daemon.py) — a concurrent atomic rotation is distinguished from tampering by re-inspecting the directory entry (stable regular file already opened → read it; non-regular entry →OSError, unchanged).daemon.ticksurvives transient store-iteration failures (warn + retry next pass) instead of terminating.config listsurvives damaged state (cli.py) — invalid stored values render askey=<invalid: reason>, truncated JSON fails cleanly, and legacy plaintext secret keys stay masked (an existing safety test caught a masking regression here during development).config unset/keysguarded (core/config.py) — the JSON-object contract is enforced likeget/set; corrupt files produce clean errors, not tracebacks.relevant_tosymmetric tokenization (core/memory.py) — one shared tokenizer for query and content, so a recorded observation forowner/repo#12is surfaced when watching the same PR.Low fixes
Empty memory content rejected on all write paths;
memory listreports hidden older items; EOF on thewatch addconfirmation declines cleanly; unusable secret stores report clean errors acrossconfig/watch; config and secret read-modify-write cycles serialize through sidecar flocks with atomic config writes; the cooperative-stop window starts after startup-lock acquisition; truncated HTTP bodies classify as retryable network errors; local secret-store failures classify as accurate blockers; permanent 301/308 redirects becomePRNotFoundErrorblockers instead of endless retries;next_check_atdefaults at creation so an ACTIVE task is never persisted unschedulable.Verification
examples/first_pr_watch.pye2e demo: all 8 scenarios pass.Review notes
Two deliberate contract changes beyond bug-for-bug parity:
secrets.json.lock/config.json.locksidecar files appear in the data home (never unlinked, matching the runner-control convention); two directory-content assertions were updated accordingly.