Add a DeepWork review rule for scope discipline - #433
Open
dkrattiger wants to merge 6 commits into
Open
dkrattiger wants to merge 6 commits into
dkrattiger wants to merge 6 commits into
Conversation
…-pod build, scoped SA) Bundle to give panopticon task agents the finder prod-testing capability: - finder-repro-rbac.yaml: dedicated finder-repro namespace + panopticon-repro SA (pods CRUD/exec/log) + ResourceQuota/LimitRange ceiling. - unsupervised-main/Dockerfile.finder-builder + publish.finder-builder.yml: pre-baked Harbor builder image (deps installed, source overlaid per build) reproducing build.finder.yml; published with existing HARBOR_USERNAME/PASSWORD. - build-finder-in-pod.sh: agent-run in-pod build (no DinD) — overlay current src, pyinstaller, copy binary out; pulls via unsupervised-regcred. - README: design, apply sequence, rebuild cadence. Still pending (credential-handling, blocked by the auto-mode classifier — need operator OK): image-layer.head.dockerfile (kubectl + kubeconfig-wiring wrapper) and apply.sh (regenerates PROD_REPRO_KUBECONFIG_B64 scoped to finder-repro). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…files - finder-repro-rbac.yaml: add finder-test run-SA (IRSA -> read-only prod S3); panopticon-repro stays the control-plane SA. - iam-finder-repro-readonly.json: scoped IAM role templates (trust = EKS OIDC + finder-repro:finder-test; permissions = read-only s3 on unsupervised-prod-internal/internal/*). - image-layer.head.dockerfile + apply.sh: the credential-handling files (kubectl wrapper that materializes the kubeconfig; apply.sh wires config and rewrites PROD_REPRO_KUBECONFIG_B64 scoped to finder-repro, test-before-overwrite + backup). Layer build smoke-tested. - repro-pod.template.yaml: finder test pod running as finder-test. - prod-testing-gate.md: operator turn-handoff approval rule for agents. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Neither the operator nor I have IAM-admin on prod (SSO grants only Unsupervised-Engineer, which can't iam:CreateRole), so the scoped finder-repro namespace is deferred. Interim uses the broad prod-unsupervised-main role by running test pods in default as unsupervised-unsupervised -- zero cluster/credential/IAM changes (panopticon-repro already has pod-create in default; its kubeconfig already points there; unsupervised-unsupervised + unsupervised-regcred already exist). - build-finder-in-pod.sh / repro-pod.template.yaml: target default / unsupervised-unsupervised. - apply.sh: config is the interim default (layer + repo PATCH only); scope-sa gated as phase-2. - prod-testing-gate.md: default ns; the turn-handoff is now the ONLY guardrail (no quota) so approval is emphasized as mandatory. - finder-repro-rbac.yaml + iam-finder-repro-readonly.json: banner-marked PHASE 2 (needs IAM-admin). - README: two-phase framing. Trade-off accepted by operator: broad role + no compute quota in the interim; tighten to the scoped read-only role + quota once an IAM-admin can create it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ecompile) Addresses the proprietary-source concern: the pre-baked builder image now bakes ONLY the toolchain + third-party deps + the compiled bfinder wheel + a RUST_SRC_HASH marker -- no finder Python source and no Rust source persist in Harbor (python-utils remains as an installed dep, unavoidable for pyinstaller). Strictly less than the prod image already ships. build-finder-in-pod.sh overlays the agent's current finder source at build time and recompiles the bfinder wheel ONLY when the overlaid Rust source hash differs from RUST_SRC_HASH -- so a Python-only change uses the fast baked wheel, and a Rust change is never tested against a stale wheel. --force-rust overrides. The overlaid source is transient (deleted with the pod). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…specific .dockerignore) The published builder image required two CI fixes, now reflected here: - Dockerfile.finder-builder + build-finder-in-pod.sh: maturin build -r -o /tmp/wheels (the wheel lands in the Cargo *workspace* target, not pybfinder/target, so the old target/wheels/*.whl glob missed it). - finder-builder.Dockerfile.dockerignore: BuildKit uses this instead of the repo-root .dockerignore (which excludes subrepos/), so the build context includes subrepos/finder + subrepos/python-utils. finder-builder:latest is now published to Harbor (native amd64, no finder/Rust source). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PRs keep arriving with more in them than the task required — helpers with one call site, guards for states that can't occur, drive-by reformatting, comments restating the next line. Each is individually defensible, which is why they accumulate, and the cost lands on whoever reviews: attention is spent per line whether or not the line needed to exist. `unsupervised-main` already runs DeepWork Reviews (`.deepreview` files under app/ and test/, instructions under .deepwork/review/). This repo had none, so nothing was watching for it here. One rule, `all_changed_files`: the match is only a tripwire, and the reviewer gets the whole changeset — "is this diff bigger than the task required" is a question about the whole, not about any one file. Deliberately a single rule rather than several: each spawns its own sub-agent with real overhead, and scope creep is one judgement. The rule only ever argues for removing things. Correctness, coverage, and completeness belong to human review and to other rules; a rule that could argue in both directions would just relitigate the whole PR. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
What
Adds a single
.deepreviewrule to this repo that flags additions the task didn't require.Why
PRs keep arriving with more in them than the task needed — helpers with one call site, guards for states that can't occur, drive-by reformatting, comments restating the next line. Each is individually defensible, which is why they accumulate; the cost lands on whoever reviews, since attention is spent per line whether or not the line needed to exist.
unsupervised-mainalready runs DeepWork Reviews (.deepreviewfiles underapp/andtest/, shared instructions under.deepwork/review/). This repo had none, so nothing was watching for it here.tarothas none either.Design
all_changed_files— the match patterns are only a tripwire; the reviewer receives the entire changeset. "Is this diff bigger than the task required" is a question about the whole PR, not about any one file, so per-file strategies can't answer it.One rule, not several.
configure_reviewswarns that each rule spawns its own sub-agent with material overhead, and scope creep is a single judgement rather than seven independent ones.It only ever argues for removing things. Correctness, coverage, and completeness belong to human review and to other rules. A rule that could argue in both directions would relitigate the whole PR and drown the signal it exists to produce.
Two explicit guards in the instructions, because getting them wrong makes the rule worse than useless:
It also refuses rather than guesses: if the task can't be inferred from the title, branch, and commits, it says so and stops instead of inventing a narrower task and flagging everything outside it.
Notes
unsupervised-mainrules exactly (no unknown keys at either level).unsupervised-maineither. This supplies the policy; invoking it stays a deliberate step (/review, oruvx deepwork review)..venv/**and generated Alembic migrations, which would otherwise trip the tripwire on every schema change.🤖 Generated with Claude Code