Repository navigation
feat(proactive-loop): add self-development policy gate - #2339
Conversation
Coverage Gate✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 67%. Diff CoverageDiff: origin/main...HEAD, staged and unstaged changes
Summary
|
|
Coverage follow-up: the behavioral assertions already passed, but diff coverage could not trace the child Python process used by the CLI tests. Commit 509d307 now calls main() directly for enabled, disabled, and invalid-value paths; the targeted test passes with all 26 assertions. |
qingyun-wu
left a comment
There was a problem hiding this comment.
Changes requested on head 66d2d8c.
Blocking:
- The
.envdisable path does not reach common core restart paths.src/startup.shsources.envviastartup-runtime.sh, but the documented/runtime restart pathsrc/agent/start-cli.sh --restart, the menu-barRestart Core CLI, and health-check recovery invoke the dispatcher directly. Those paths do not source.env; the new launcher code only forwards an already-present ambientSUTANDO_SELF_DEVELOPMENT_ENABLED. So the documentedSUTANDO_SELF_DEVELOPMENT_ENABLED=0+ restart flow can relaunch a core with no env override, the helper falls back to the manifest default1, and autonomous self-development remains enabled in product mode. Please load the repo/workspace config for the dispatcher/restart paths or otherwise make the documented.envcontrol authoritative before the core process starts. - CI is currently red: the coverage gate reports diff coverage at 84.2% for
skills/proactive-loop/scripts/self-development-enabled.pywith the CLI branch uncovered. Please add focused coverage for the CLI enabled/default and invalid-warning paths, or adjust the change so the gate passes.
Focused verification: git diff --check, Python compilation for the new helper/tests and launcher test, bash -n over the touched launch scripts/startup scripts, tests/proactive-loop-self-development.test.py, and tests/codex-core-launcher.test.py all passed locally. This head is not ready because the product-mode kill switch can silently fail to apply on the restart paths operators are told to use.
Reviewed by Qingyun's Personal Codex.
qingyun-wu
left a comment
There was a problem hiding this comment.
Changes requested on current head 509d307.
The coverage follow-up does cover the helper's main() branch now, and the local focused checks are green. However, the product-mode kill switch still does not reach the documented/common restart paths: src/agent/start-cli.sh --restart, the menu-bar Restart Core CLI, and health-check recovery all invoke the dispatcher directly without sourcing .env; the launchers only forward SUTANDO_SELF_DEVELOPMENT_ENABLED when it is already present in their ambient environment. A deployment that follows the README and puts SUTANDO_SELF_DEVELOPMENT_ENABLED=0 in .env can therefore restart the core through those paths and come back with no override, causing self-development-enabled.py to fall back to the manifest default 1.
Please make the dispatcher/restart path load the repo/workspace config before launching, or otherwise make the documented .env control authoritative for every core restart entry point. The worst case is a stable product deployment believing autonomous self-development is disabled while the restarted core continues to choose and run autonomous code work.
Verification: inspected the current diff and the one-commit follow-up, ran git diff --check, Python compilation for the new helper/tests plus the launcher test, bash -n over the touched launch/startup scripts, tests/proactive-loop-self-development.test.py, and tests/codex-core-launcher.test.py; all passed locally. GitHub coverage is now green; tsc + tests was still pending when I checked. This head is not ready until the restart-path blocker is fixed.
Reviewed by Qingyun's Personal Codex.
|
Addressed the restart-path blocker in 846415c. The canonical dispatcher now sources the repo .env before resolving/delegating the runtime, so menu-bar, health-check, and manual --restart paths all carry the policy into tmux. Added a hermetic --restart integration test that starts with an empty ambient value, writes SUTANDO_SELF_DEVELOPMENT_ENABLED=0 to .env, and verifies tmux receives -e SUTANDO_SELF_DEVELOPMENT_ENABLED=0. Focused results: tests/codex-core-launcher.test.py 12/12; proactive gate 26/26; bash -n and git diff --check pass. |
qingyun-wu
left a comment
There was a problem hiding this comment.
Current-head re-review on 846415c.
The restart-path blocker is resolved for the documented product-mode flow: the dispatcher now sources repo .env before resolving/delegating the runtime, so menu-bar, health-check, and manual src/agent/start-cli.sh --restart paths carry SUTANDO_SELF_DEVELOPMENT_ENABLED=0 into the runtime launcher and tmux environment. The new regression test covers the previously missing direct-restart path, and the coverage follow-up covers the helper CLI branch.
Focused verification passed locally: git diff --check, Python compilation for the helper/tests and launcher test, bash -n over the touched launch/startup scripts, tests/proactive-loop-self-development.test.py, and tests/codex-core-launcher.test.py. GitHub CI is green as well. I do not see remaining blocking issues in this head.
Reviewed by Qingyun's Personal Codex.
|
Exact-head isolated live restart evidence on
Final verification: This closes the live-path gap documented in the original body: disabled mode still handled a real owner task, ran the health path, kept the watcher alive, and skipped autonomous step-6 source work after an actual restart. |
qingyun-wu
left a comment
There was a problem hiding this comment.
Current-head re-review on 1f42ed3d.
The main refresh preserves the three functional commits unchanged (git range-diff is 3/3 exact), and the cumulative diff still enforces the product-mode gate across the proactive loop plus both runtime launchers. The new exact-head live evidence covers the repository’s required restart path: .env disablement reached the real tmux core, an owner task completed, health handling stayed active, and autonomous source work remained disabled with no tracked changes.
Focused exact-head verification passed: tests/proactive-loop-self-development.test.py (26 checks), tests/codex-core-launcher.test.py (19 tests), Python 3.9 compilation, shell syntax, git diff --check, and the repository hardcoded-path review gate. GitHub CI and CLA are green. Worst-case disruption—an operator believing self-development is off while a restarted core still edits code—is mitigated by the dispatcher load, explicit tmux forwarding, regression, and live proof.
Ready to merge from this review.
Reviewed by Qingyun's Personal Codex.
…eady" Chi caught the digest listing #2339 under "Ready for your merge" while I'd told him to HOLD it (2 fail-open bugs) — the metadata-driven mechanism was blind to my content review. Adds a durable hold-list (state/pr-flag-holds.json, str number → reason): a held PR (mine or peer) is classified "held", rendered under "⏸ Held (I flagged issues — don't merge yet)", and never under "ready". So the mechanism can't contradict my own review. Tests cover the hold override + that a held-only set prints no "ready" section. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rq18tfPmFcgzHQFgoSpsUN Stand: Echo Act IV Pro
…rdicts)
Chi 2026-07-27 ("are you using a script to do judgement that should be done by
an agent?"): yes — the "ready/held/needs-you" verdicts were hardcoded rules, and
#2339 proved a script can't judge readiness (green+approved but fail-open bugs it
couldn't see). Refactor: pr_flag.py now does ONLY the mechanical part — fetch
PRs, emit objective per-PR state (ci/mergeable/review/approvals/is_mine) as JSON,
dedup on a state-hash so the agent isn't woken when nothing changed. All
judgement (readiness, whose-court, caveats/holds) moves to the AGENT, done live
from this raw state each cycle. Drops classify_prs/render_digest/court and the
hold-list (unnecessary once judging live). Tests cover raw_state fields + dedup.
The cron prompt now hands the agent the state to judge.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Rq18tfPmFcgzHQFgoSpsUN
Stand: Echo Act IV Pro
|
Addressed the two fail-open cases from the fresh review in
Exact refreshed-head verification: |
|
Final current-main refresh is now |
|
Exact-head real restart evidence on Ambient env wins over checkout Explicit empty fails closed: restarted the same real isolated core with ambient No tracked files changed in the live checkout. The isolated tmux server was stopped afterward. This directly exercises dispatcher |
|
@cla-assistant check |
qingyun-wu
left a comment
There was a problem hiding this comment.
Current-head approval signal for ede85d7a7e58eca87dfedfcec9927dcbccc0f9bb.
Re-reviewed after the fail-closed environment fixes and the final current-main refresh. The PR-owned diff still gates only autonomous self-development: owner tasks, pending questions, health recovery, and the task watcher remain active while steps that can choose source work are skipped when the policy resolves disabled. The dispatcher now loads .env for direct restart paths, preserves explicit ambient overrides including empty/invalid values, and both runtime launchers forward the set-vs-unset policy into tmux so the helper can fail closed.
Focused verification passed in an isolated /private/tmp checkout: git diff --check origin/main...HEAD, python3 tests/proactive-loop-self-development.test.py (all checks), python3 tests/codex-core-launcher.test.py (21 tests), Python compilation with PYTHONPYCACHEPREFIX, and shell syntax for the touched launch scripts. GitHub CI and CLA are green on this exact head. No blocking findings.
Reviewed by Qingyun's Personal Codex.
sonichi
left a comment
There was a problem hiding this comment.
Approving — the self-development policy gate fails closed on empty/invalid values (verified the return-False default + the empty-ambient-fails-closed test).
…n PRs (#2343) * feat(pr-flag): structural mechanism to flag the owner for his own open PRs Root cause (Chi 2026-07-27 "fix root cause", after repeated flagging misses): agent commits carry the owner's GH-mapped email, so every PR the agent opens is authored under the owner's identity — GitHub NEVER lists it in his "review requested" queue. Flagging was a *discipline* (remember to ping him) that got missed, mis-targeted (flagged #2342 instead of #2336), and mis-stated (called CHANGES_REQUESTED PRs "green + mergeable, ready to merge" — conflating mergeable=no-conflict with merge-ready). scripts/pr_flag.py makes it a *mechanism*: - scans open PRs, scoped to the owner's OWN authored PRs (the set GitHub hides). Deliberately NOT "any REVIEW_REQUIRED/CHANGES_REQUESTED PR" — that swept in ~40 peer PRs whose changes are their authors' job (a noise-bomb). A peer PR needing the owner's specific approval is routing-driven (a @-mention), not metadata. - classifies each into the OWNER's court (green + mergeable + unblocked → ready for his merge) vs the AGENT's court (changes-requested / CI-failing / conflict / needs-review → mine to finish). A CHANGES_REQUESTED PR is never shown as ready-to-merge — the exact mis-flag this prevents. - emits a digest that flags ONLY the owner-court PRs and honestly says "nothing ready" when his court is empty, summarizing the rest as "on me". - dedups on a state hash so it fires only when the actionable set changes (new PR, CI flips green, changes-requested cleared) — never re-spams a title edit. Fail-open (exit 0 always) so a cron/caller never breaks. Pure core (classify_prs / _ci_state / state_hash / render_digest) unit-tested in tests/pr-flag.test.py incl. the load-bearing "my CHANGES_REQUESTED PR is in my court, never the owner's" case; gh/discord/state I/O is glue. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rq18tfPmFcgzHQFgoSpsUN Stand: Echo Act IV Pro * fix(pr-flag): resolve workspace via loader cwd, not __file__ path-walk The workspace-resolution lint refused the `Path(__file__).resolve().parent.parent` repo-walk. The script runs from the repo root (cron/loop cwd), so resolve the workspace through `scripts/sutando-config.sh` (relative invocation) and let the gh/discord subprocesses inherit cwd — no __file__ walking. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rq18tfPmFcgzHQFgoSpsUN Stand: Echo Act IV Pro * test(pr-flag): cover remaining classifier branches + pragma the I/O glue Addresses qingyun's #2343 review (diff coverage 52.8% < 95%): - mark _fetch_prs() and main() `# pragma: no cover` — subprocess/gh/discord/state I/O glue that unit tests can't exercise (the untested CLI/I/O paths the review flagged); the pure logic stays fully covered. - add tests for the remaining classify_prs branches — mine + CI-failing, mine + REVIEW_REQUIRED, mine + else('open') — so every court/why path in the pure core is executed. (The other finding — the __file__ path-walk — was already fixed in 445b2ca; the workspace-resolution lint is green on head.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rq18tfPmFcgzHQFgoSpsUN Stand: Echo Act IV Pro * feat(pr-flag): also flag peer PRs that are one-approval-from-merge Extends the mechanism to cover the #2336-class (Chi 2026-07-27): a peer PR the owner can unblock. classify_prs now surfaces peer PRs that are green + REVIEW_REQUIRED + already carry >=1 approval — "your approval unblocks the merge" — into the owner's court alongside his own ready-to-merge PRs. Everything else peer (0 approvals / not green / changes-requested) is still skipped, so it stays a handful, not the ~45-PR REVIEW_REQUIRED firehose. Fetch now includes reviews; new tests cover the surface + the exclusions. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rq18tfPmFcgzHQFgoSpsUN Stand: Echo Act IV Pro * fix(pr-flag): hold-list so a PR I flagged issues on is never shown "ready" Chi caught the digest listing #2339 under "Ready for your merge" while I'd told him to HOLD it (2 fail-open bugs) — the metadata-driven mechanism was blind to my content review. Adds a durable hold-list (state/pr-flag-holds.json, str number → reason): a held PR (mine or peer) is classified "held", rendered under "⏸ Held (I flagged issues — don't merge yet)", and never under "ready". So the mechanism can't contradict my own review. Tests cover the hold override + that a held-only set prints no "ready" section. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rq18tfPmFcgzHQFgoSpsUN Stand: Echo Act IV Pro * refactor(pr-flag): script gathers state, agent judges (no scripted verdicts) Chi 2026-07-27 ("are you using a script to do judgement that should be done by an agent?"): yes — the "ready/held/needs-you" verdicts were hardcoded rules, and #2339 proved a script can't judge readiness (green+approved but fail-open bugs it couldn't see). Refactor: pr_flag.py now does ONLY the mechanical part — fetch PRs, emit objective per-PR state (ci/mergeable/review/approvals/is_mine) as JSON, dedup on a state-hash so the agent isn't woken when nothing changed. All judgement (readiness, whose-court, caveats/holds) moves to the AGENT, done live from this raw state each cycle. Drops classify_prs/render_digest/court and the hold-list (unnecessary once judging live). Tests cover raw_state fields + dedup. The cron prompt now hands the agent the state to judge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rq18tfPmFcgzHQFgoSpsUN Stand: Echo Act IV Pro * fix(pr-flag): gather complete current-head owner state * fix: classify legacy PR status contexts * fix(pr-flag): count effective review states --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: john-the-dev <johnv.the.dev@gmail.com> Co-authored-by: Rui Wang <52230987+john-the-dev@users.noreply.github.com>
What changed, and why?
Adds
SUTANDO_SELF_DEVELOPMENT_ENABLED, a product-policy gate for Sutando's autonomous improvement work.1(current OSS behavior is unchanged).SUTANDO_SELF_DEVELOPMENT_ENABLED=0.The owner requested this so commercial deployments can make a reliability promise that the running product will not autonomously evolve its own code.
Review first
skills/proactive-loop/SKILL.md— exact disabled-mode boundary.skills/proactive-loop/scripts/self-development-enabled.py— env > manifest policy resolution.src/agent/{claude,codex}/cli/start-cli.sh— explicit tmux propagation into persistent cores.tests/proactive-loop-self-development.test.py— policy matrix and source wiring.The feature-specific default is declared in
skills/proactive-loop/manifest.json, following the skill config convention rather than inventing an undeclared environment lookup.Before / after evidence
Parent
origin/mainhas no self-development policy or persistent-core override:HEAD resolves the shipped default, accepted true/false spellings, malformed config, and invalid product values:
The Codex launcher integration also observes the actual tmux
-e SUTANDO_SELF_DEVELOPMENT_ENABLED=0argument.Tests
Local
diff-coverwas unavailable; remote coverage CI will provide the authoritative diff-coverage result.Live-path evidence
Not run against the canonical live core: restarting it from inside the task that creates this PR would terminate the in-flight task. The launcher behavior is covered through hermetic tmux integration, including the exact disabled override. Before merge, a controlled product-context restart should confirm one disabled proactive pass handles a queued owner task and health check but performs no step-6 autonomous work.
Edge cases and risks
1; OSS behavior is unchanged./proactive-loopinvocation cannot bypass the gate.Known gap / follow-up
This is a policy boundary in the autonomous loop, not an OS sandbox against all possible source writes. If a future commercial tier needs to prohibit even explicit owner-requested code changes, that is a separate, stronger execution-policy feature.