Integrate reviewed MVP and add the first PR-watch workflow - #28
Conversation
0600 JSON secret store behind the SecretStore port; Redactor applied at persistence boundaries; config routing (secret-named keys go to the store, never config.json); config list masks secrets. Raw secret values exist in no file under the data home except the store (tested).
Scope recorded verbatim at creation (target/purpose/cadence/allowed actions/ notification + stop conditions); explicit update_scope bumps scope_version for grant invalidation; pause/cancel/complete persisted; validation rejects any write action; redaction wired at persist; inspectable via sqlite3.
SnapshotFetcher port (typed FetchError hierarchy: retryable / auth-lost / not-found), read-only REST client (GET only, no partial snapshots, rate limit vs auth distinguished), pure evaluate_checks keyed to head SHA — old-commit results can never satisfy a watch. Scriptable FakeGitHub lands in tests/fakes.py for all later substitution.
step(task, snapshot, now) -> watch_state + events. Commit-pinned via github_eval; new head SHA resets evaluation and emits new-commit; unchanged snapshots emit nothing; merged/closed/passing each terminal exactly once; pending transitions recorded non-notable, failures and terminals notable.
TaskLoop.run_once (single-flight per task; fetch blockers typed: auth-lost blocks pending user action, retryables back off x2 to 60min with prolonged failure visibly flagged while still retrying), append-only activity log with redaction + tombstones, RunnerDaemon whose restart is reconciliation (state-machine + sink dedup prevent replay floods).
NativeNotifier: persisted inbox (unique dedup key per event content, surviving restarts) + best-effort macOS notification; notable/terminal only; evidence links stored per entry; redaction on notification text; OS delivery failure never loses the inbox entry.
watch add displays the full saved scope (six fields) and requires explicit confirmation; list shows status/latest/next-check/blockers; pause/resume/ cancel; activity + inbox views; runner --once / start / stop / status with pidfile lifecycle; clear errors for missing token.
InferenceProvider port (summarize + parse_intent); EgressGuard builds outbound payloads from a fixed whitelist (PR metadata + evidence only, structurally nothing else can attach, secrets scrubbed); OpenAI-compatible API adapter behind the port; degraded mode guaranteed (ProviderError -> raw message, watch unaffected); optional summaries decorate notifications without touching dedup keys; --intent at watch add with manual fallback; docs/design/egress.md documents what leaves the host.
…d deletion Three write paths only (user->confirmed, evidenced terminal outcomes-> observation w/ evidence provenance, proposals->proposed until CLI confirm); AST boundary test proves no model-output-to-confirmed path exists; deletion removes content everywhere and tombstones activity contentlessly; proposals expire (14d default, sweepable); CLI memory list/add/propose/confirm/edit/ rm; remembered context surfaced locally at watch add; loop proven to run on an empty store; secrets scrubbed at write. Engine: SQLite (per issue defaults; sub-decisions flagged for review).
ZCode-style: readonly enforced default with WriteForbidden gate; approvals create grants scoped to exact action+target+scope+expiry (exact-match permit checks); silence never approves (requests expire, approve-after- expiry refused); denials recorded and final; on_scope_change revokes the task's grants (wired to TaskStore.update_scope version bump); no-write invariant tests (GitHub client exposes only fetch; non-GET HTTP verbs only in the inference adapter; no third-party HTTP clients); approvals CLI.
The seven scenarios from issue #1 automated against fakes (fake GitHub, clock, OS notifier, provider) on real core + SQLite stores: scope inspection, pending->failing->passing, new-commit invalidation, restart recovery without duplicate notifications, rate-limit/lost-auth/recovery, pause/resume/cancel/terminal-stop, and no-external-write + no-secret-leak. CI runs the entire suite with networking dead (dead proxy) — the executable proof of the adapter seam.
Use atomic private writes with symlink defenses, safer hidden/stdin input, common secret-name routing, and recursive redaction. Validated offline: 46 stage tests passed. Preserve existing stacked PR history.
Integrate the corrected issue-4 dependency without rewriting either branch. Guard terminal state, schedule resumed tasks, reject unsupported scopes, and translate duplicate IDs. Offline validation: 79 stage tests passed.
Integrate corrected issue-5 without rewriting history. Paginate all suites, runs, statuses and rules; pin SHA and provenance, fail closed on unknown requirements, handle secondary throttling and reject credential redirects. Offline validation: 198 stage tests passed.
Integrate corrected issue-6 without rewriting history. Reset failure state on new commits, assign stable occurrence IDs, enforce saved scope, and explain unknown required-check evidence. Offline: 218 tests passed.
Integrate corrected issue-7 without rewriting history. Respect pause, cancel and scope changes during fetch; isolate unexpected task errors, retry safely, and remove manual resume-scheduling test repairs. Offline validation: 253 stage tests passed.
Integrate corrected issue-8 without rewriting history. Pass untrusted notification text as AppleScript arguments, preserve delivery dedup per occurrence across restarts, and cover repeated same-SHA failures. Offline validation: 265 stage tests passed.
Integrate corrected issue-9 without rewriting history. Use lifetime locks and token-scoped cooperative shutdown, wait for readiness and exit, keep read-only views lightweight, and reject unsafe target/scope inputs. Offline validation: 299 stage tests passed.
Integrate corrected issue-10 without rewriting history. Wire production provider redaction and summaries, scrub structured values before encoding, reject credential redirects, bound optional work, and accept fenced JSON. Offline validation: 329 stage tests passed.
Integrate corrected issue-11 without rewriting history. Wire redaction and contentless tombstones into real CLI commands, enforce proposal expiry, securely delete SQLite cells, and isolate optional observation failures. Offline validation: 349 stage tests passed.
#26) Integrate corrected issue-12 without rewriting history. Reject dormant write modes, enforce request/grant expiry at use, reject double approval, and revoke grants plus pending requests atomically with task scope edits. Offline validation: 374 stage tests passed.
Integrate corrected issue-13 without rewriting history. Scope dead proxies to the test step, add socket/DNS tripwires, remove hidden resume repairs, verify real CLI boundaries, and document operation and review coverage. Final offline validation: 388 tests passed; compile and diff checks pass.
Implement #14: end-to-end PR-watch validation suite, all fakes
…live configuration
hsliuustc0106
left a comment
hsliuustc0106
left a comment
There was a problem hiding this comment.
Reviewed commit c1e03dedd42e78b86229f8aeb1daf6ebc9020e05 against base 82eb79a241a024005416ee8053099e6a0ddab5cb, covering the full MVP implementation and first-use workflow.
I found three reproduced correctness issues in the inline comments: CI failure alerts disappear when required rules are empty/hidden; crash recovery can duplicate failure and terminal-success notifications when an unrelated check changes; and shutdown starts additional due watches after the stop Event is set. These should be fixed before merging.
Validation: all 433 existing tests passed on Python 3.12.13 with dead proxies and the suite's socket/DNS guard; git diff --check passed. Additional offline probes used fresh production CLI processes, native GitHub response parsing, real SQLite/inbox persistence, and actual process exits at the inbox/checkpoint boundary. The shutdown probe used the real task loop and daemon with a fake fetcher. No live model calls or native macOS delivery were exercised.
hsliuustc0106
left a comment
hsliuustc0106
left a comment
There was a problem hiding this comment.
Re-reviewed a8c704e2b6390e14c7ff5eae810ec8d210faa2d9, including the changes since c1e03dedd42e78b86229f8aeb1daf6ebc9020e05 and their interaction with the existing runner/state machine.
All three original reproductions now pass: empty/hidden required rules preserve CI failure alerts; changing optional evidence after an actual crash does not duplicate failure or terminal-success notifications; and cooperative shutdown stops before the next due watch starts.
One additional P2 remains in the inline comment: observed optional CI failures are still silent when required rules are known and their checks remain pending. Failure alerts should follow the documented check-failure policy while confirmed required-check success retains its terminal priority.
Validation: 489 tests passed on Python 3.12.13 with the offline proxy/socket guards, and git diff --check passed. I independently reran the original fresh-process CLI/crash/shutdown probes and added native CLI cases for optional failures with known versus empty rules. The control with passing required checks and a failed optional check correctly completes the watch. Both GitHub CI runs passed on this head. No live inference or native macOS delivery was exercised.
hsliuustc0106
left a comment
hsliuustc0106
left a comment
There was a problem hiding this comment.
Re-reviewed 661f4bae106e9f9718137812a803020b8954acc8, including the four-file update since a8c704e2b6390e14c7ff5eae810ec8d210faa2d9 and its interaction with the existing state machine and notification flow.
No new actionable findings. All four previously reported issues are addressed. Optional current-head failures now alert while required checks remain unconfirmed; unchanged failures stay quiet, recovery permits a new failure alert, and confirmed required success still completes the watch despite optional failures. The earlier empty/hidden-rule, crash-replay, and cooperative-shutdown reproductions also pass.
Validation: 515 tests passed on Python 3.12.13 with the offline proxy/socket guards; git diff --check passed. I independently reran the fresh-process CLI probes, actual crash/restart cases with changing optional evidence, and shutdown probe, plus the optional-failure/recovery/recurrence/completion sequence. Both GitHub CI runs passed on this head. Native macOS delivery and live inference were not exercised.
main absorbed the MVP merge (#28) and rewrote README.md around it, while this branch still described the MVP as living on the integration branch. Keep main's README as the body, add a Documentation section linking the new guides, and update installation.md / user-guide.md so they install from the default branch instead of integration/mvp-first-pr-watch (deep links become relative links to the docs now on main).
1 participant
Summary
82eb79a241a024005416ee8053099e6a0ddab5cb(main) and0492040e7b7daa9366b2a0ff3512d3c215dc18ed(the completed stack from issue-13).os-notifications falseand prevent live authentication/notification policy changes until the runner stops. Initialize runner policy under its lifetime lock before publishing readiness.python examples/first_pr_watch.pyand docs/first-pr-watch.md.Validation
433 tests passed locally on Python 3.12.14, with offline HTTP/socket guards.
git diff --checkpassed.8 CLI-process demo scenarios passed: pending; failure; hard process crash after durable inbox write and restart deduplication; new commit; stale-SHA success rejection; required CI success on the current SHA; terminal/pause/cancel no-fetch behavior; real background start/status/stop.
Demo notification sequence is exactly
checks-failed,new-commit,checks-passed. GitHub changes in that demonstration are explicitly simulated; native HTTP parsing, CLI, runner, SQLite, and inbox are production code.Real public GitHub smoke passed, without a token or model: PR Implement #14: end-to-end PR-watch validation suite, all fakes #27's existing merged state produced one local inbox notification and completed its watch. A second run attempted zero tasks. This observes an already-merged PR, not a newly occurring merge or a CI certification.
Existing CI uses only the standard
ubuntu-latestrunner and Python/pytest. No paid model calls, hosted deployment, or extra paid runner has been added. Both PR CI and push CI passed on exact headc1e03dedd42e78b86229f8aeb1daf6ebc9020e05. The first attempted run exposed duplicate case-insensitive env keys in the inherited workflow; that was fixed, covered by a regression test, and both final runs completed successfully.A real anonymous read of this open PR also succeeded. Since main has no required checks configured, nanodot correctly remained pending instead of claiming required CI success. The temporary test watch was cancelled and no runner was left active.
Try it
For a real public PR, use an isolated NANODOT_HOME, set
github-auth-mode anonymousandos-notifications false, add a watch with--cadence 1800, then runnanodot runner --once. See the walkthrough for ongoing use and cancellation.Conservative behavior
Unknown/hidden required-check rules cannot prove CI success. If no required checks are configured, the watch remains active until the PR closes or merges; optional green checks alone do not complete it. The tested crash boundary proves inbox deduplication, not arbitrary filesystem-corruption recovery or exactly-once OS popups.
Draft for review; no merge or auto-merge requested.