From 0532563a8f8d182191b69d53dab3429c098cabda Mon Sep 17 00:00:00 2001 From: song Date: Sat, 17 Oct 2026 16:28:53 +0800 Subject: [PATCH 1/5] docs: keep repository instructions within the default loading budget Signed-off-by: song --- AGENTS.md | 826 ++++++------------ docs/development/README.md | 108 +++ docs/development/good-smokes.md | 43 + .../core-control-plane/state-machine.md | 25 +- ...apability-extension-placement-doc-smoke.py | 8 +- tests/test_repository_instruction_delivery.py | 82 ++ 6 files changed, 547 insertions(+), 545 deletions(-) create mode 100644 tests/test_repository_instruction_delivery.py diff --git a/AGENTS.md b/AGENTS.md index db007917f0..38c7130fd3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,5 +1,10 @@ # Agent Instructions +These are standing repository constraints. Read a linked procedure when its +trigger applies to the current task; do not load every reference for every turn. +Keep current work, evidence and monitor targets in their canonical task/state, +not in this file or a frozen prompt. + ## Goal-Oriented Development Before selecting non-trivial work, resolve the current requested outcome from @@ -9,15 +14,14 @@ and relevant domain acceptance; do not make every fix wait for every RFC or invent a roadmap id. Check latest `main`, related PRs and canonical Todos so an older task description cannot override corrected direction or duplicate work. -For recurring operational or performance problems, connect the demonstrated -failure to the owning roadmap/RFC acceptance before choosing a repair. Separate -caller overhead, shared typed semantics/transport and provider-specific costs; -prefer the existing common contract where behavior is shared. Follow the -[optimization evidence guide](docs/development/testing-and-quality.md#roadmap-aligned-optimization). -Distinguish an interim mitigation from closing the owning acceptance: faster -lookup, a larger timeout or successful promotion alone does not qualify sustained -operation. Reconcile the existing checkpoint when the evidence changes it; -do not add a parallel roadmap or require unrelated RFC work for a bounded fix. +For recurring operational or performance failures, follow the +[roadmap-aligned optimization guide](docs/development/testing-and-quality.md#roadmap-aligned-optimization): +bind the demonstrated failure to its owning acceptance before repair, separate +caller, shared typed transport/semantics and provider costs, and reuse the common +contract. A faster lookup, timeout increase or successful promotion is only an +interim result until sustained acceptance is proven. Reconcile the existing +checkpoint as evidence changes; do not create a parallel roadmap or require +unrelated RFC work for a bounded fix. Carry one compact delivery brief from task to PR: goal/source, current gap, observable result, owning boundary and decisive acceptance evidence. Reuse the @@ -36,20 +40,12 @@ dependency and why the boundary improves verification or rollback. Reuse or update an existing successor; do not create ceremonial follow-up tasks for a completed request. Real authorization, cost and operational stop gates remain. -When a change introduces or extends a state classification, protocol literal, -Enum, `Literal`, named closed set, or TypeScript `as const` vocabulary, answer -whether it reuses an existing owner, extends a registered vocabulary, remains -local, or creates a new shared contract. Before the full-tree semantic check, -run the development-time advisory over the current diff: - -```bash -uv run python scripts/generate_semantic_inventory.py --changed-from HEAD -``` - -Pass each intentional untracked source with `--include-untracked loopx/path.py`; -the probe never scans untracked or ignored files automatically. Findings are -review prompts, not a gate or proof of semantic equivalence, and an empty result -does not cover dynamic construction or unsupported syntax. +When adding or extending state/protocol vocabulary (Enum, `Literal`, named +closed set or TypeScript `as const`), answer whether it reuses an owner, extends +a registered vocabulary, remains local or creates a shared contract. Follow the +[diff-scoped semantic advisory](CONTRIBUTING.md#check-new-semantic-vocabulary-while-developing) +before the full-tree check, including each intentional untracked source. Findings +are review prompts, not a gate or proof that all semantic forms were covered. For multi-Agent changes, qualify the relationship the user needs: dependency artifacts, receiver adoption, claim/lease handling, independent acceptance and @@ -70,11 +66,12 @@ this prose do not certify it or settle a Goal. ### Worktree And PR Gate For any tracked repository change beyond a trivial typo fix, create or use a -dedicated clean `git worktree` on a `codex/` branch. Use latest `origin/main` -unless the user explicitly names an integration or release branch; in that -case, fetch that branch and use its latest remote head as both the worktree -baseline and pull-request base. Before pushing, verify the merge base and PR -base so unrelated `main` history cannot leak into a stacked integration PR. +dedicated clean `git worktree` on a `codex/` branch. Verify the target repository +and remote first: a fork's `origin/main` may not be the PR target. Fetch and use +the latest target `main`, or the integration/release branch explicitly named by +the user, as both worktree baseline and PR base. Before pushing, verify the +merge base and actual PR base so unrelated `main` history cannot leak into a +stacked integration PR. Do not implement changes directly in a dirty primary worktree, even when the task starts by inspecting that dirty tree. @@ -86,7 +83,8 @@ obsolete. Leave unrelated untracked local artifacts alone. Every tracked repository change must be pushed on a branch and reviewed through a pull request before it reaches `main`. Do not push broad mixed commits or -direct commits to `main`. +direct commits to `main`. Only skip this worktree/PR gate when the user explicitly +says the change is local-only and must not be proposed for the repository. ### Author Submission And Review Entry @@ -107,555 +105,328 @@ merge authorization as their separate gates. ### DCO Sign-Off -Every commit in a pull-request branch must include a -`Signed-off-by: Your Name ` trailer, or the `DCO` -check will reject the PR. Always commit with `git commit -s`. If a commit is -already missing the trailer, amend it with `git commit --amend -s` (or an -interactive rebase for multiple commits) before pushing. See CONTRIBUTING.md -for the full DCO policy. - -Only skip this worktree/PR gate when the user explicitly says the change is -local-only and must not be proposed for the repository. +Every PR commit needs a `Signed-off-by` trailer: always use `git commit -s`. +Repair a missing trailer with `git commit --amend -s` or a multi-commit rebase +before pushing. See [DCO](CONTRIBUTING.md#license-and-dco-sign-off); otherwise the +DCO check rejects the PR. ### Bilingual RFC Diff Gate For an existing English/Chinese RFC pair under `docs/architecture/rfcs/`, including its ledger entries, both files must change in the same PR. Check the -staged files before committing: +staged files before committing. Set `PR_BASE_REF` to the verified PR target +remote-tracking ref (for example `upstream/main` for a fork PR): ```bash -python3 scripts/check_rfc_bilingual.py --base origin/main --staged +: "${PR_BASE_REF:?set the verified PR target ref}" +uv run --extra test python scripts/check_rfc_bilingual.py --base "$PR_BASE_REF" --staged ``` The staged check includes earlier branch commits. Before merge, run -`python3 scripts/check_rfc_bilingual.py --base origin/main --head HEAD` against -the PR's actual target branch and exact reviewed head. The source-checkout -`uv run --extra test loopx canary premerge --from-git-diff` runs this as a direct +`uv run --extra test python scripts/check_rfc_bilingual.py --base "$PR_BASE_REF" --head HEAD` +against the PR's actual target branch and exact reviewed head. The source-checkout +`uv run --extra test loopx canary premerge --from-git-diff --git-diff-base "$PR_BASE_REF"` +runs this as a direct blocking check, including in the quick tier. A failure holds merge even when CI waiting is disabled or admin bypass is authorized. It checks the committed diff; an unstaged translation cannot satisfy it. The check uses the merge base, covers paired deletion/renaming, and leaves legacy single-language files alone. Review the actual translation; paired diffs do not certify semantic equivalence. -For non-trivial repository changes, especially anything that touches benchmark -adapters, smoke tests, public docs, or commit/push workflows, use the -`git-split-commit-pr` workflow before staging: - -1. Establish ground truth with `git status --short --branch`, - `git diff --stat`, `git diff --name-only`, and - `git ls-files --others --exclude-standard`. -2. Classify every changed path before staging: - - core product code; - - core documentation; - - durable validation smoke; - - local/private state; - - low-value or obsolete artifact. -3. Scan candidate paths for credentials, private state, local absolute paths, - raw benchmark logs, trajectories, verifier output, and internal links. -4. Stage by explicit pathspecs only. Do not use `git add .`. -5. Split commits by reviewer logic: - - runtime/API behavior; - - public docs and protocol notes; - - focused validation or cleanup. -6. Push a branch and open a PR for reviewable batches. - -Control-plane, runtime and product surfaces require explicit maintainer -merge authorization. A change that alters the behavior of `loopx/**`, `apps/**` -or `packages/**`, a permission or authority boundary, a persisted state or -receipt contract, a quota/scheduler/todo rule, or a CLI/API contract must be -proposed and reviewed on its exact head. After the required validation passes -and exact-head merge readiness returns ready, an agent may use admin bypass -when the maintainer has explicitly authorized that merge or delivery batch. -A small diff, successful validation, published review or green CI does not grant -merge authority by itself. Admin bypass never substitutes for exact-head review, -required validation or merge readiness. Without explicit authorization, leave -these changes for the maintainer to merge; when the affected boundary is unclear, -use the same authorization requirement. - -For small, low-risk PRs, maintainers may self-merge after validation when all -of the following are true: - -Here, "自合并" means: 自己 review/refine, publish that review on the exact head, -then admin-bypass merge after the required validation and authorization. A -self-merge whose head carries no published review is a process gap, not a -smaller form of review: repair it by publishing the exact-head review for the -merged commit and correcting the rule that let it through. - -- the PR only touches public docs, contributor metadata, tests, examples, or - narrow cleanup, and none of the surfaces named above; -- the change is single-purpose and easy to review from the diff; -- required checks or focused smokes have passed; -- the exact head carries a published self-review, and - `loopx pr-review --goal-id GOAL --check-merge-readiness NUMBER@HEAD_OID` - returned ready for - that unchanged head. GitHub blocks formal self-approval, so on an - author-owned PR the record is a `COMMENTED` review on the exact head that - states the approval conclusion and an English verdict; a green CI run, a diff - read, or the merge itself is not that record; -- private state, raw benchmark evidence, credentials, local paths, and - generated logs are excluded; -- there is no runtime behavior, benchmark adapter, permission, destructive git, - or public evidence-policy change that needs separate review. - -Small benchmark seam/refactor PRs may also be self-merged when they are like -PR #145: they add or clarify a reusable adapter/control-plane contract, include -focused public smokes, do not launch benchmark jobs, do not change scoring or -runner behavior for an existing benchmark, and do not include temporary probes, -raw evidence, private state, credentials, local paths, or generated logs. - -Benchmark helper/runtime PRs may also be self-merged after owner authorization -when they are limited to public benchmark helper code, status/runtime -observation, reducer/closeout plumbing, or benchmark developer workflow support; -focused smokes or compile checks pass; public/private boundary scans are clean; -and the PR does not change benchmark scoring, task semantics, leaderboard or -submission behavior, permission boundaries, or launch new benchmark jobs. - -After self-merging, sync local `main`, leave unrelated untracked local artifacts -alone, and continue with the next safe project batch. - -Before self-merging non-trivial LoopX changes, run -`loopx canary premerge --from-git-diff` or an equivalent risk-based validation -set. The PR comment must name the changed surfaces, checks run, failures/skips, -manual holds, and why the coverage is enough. One hand-picked smoke is not -enough for runtime, quota/status, scheduler, todo, install, dashboard, -benchmark-boundary, or public/private evidence changes. - -### Personal Feishu Release Guide - -Every named public release must include an illustrated Feishu release guide -owned by the maintainer's verified personal user account. Follow the -[release guide procedure](docs/product/release-readiness.md#personal-feishu-release-guide) -before calling release communication complete. Verify account and document -ownership, use public-safe screenshots of the released UI, teach upgrade and -capability use, and read back the published text and images. Do not substitute a -corporate account or bot, claim a candidate is released, or infer authorization -for chat announcements or other social channels. Keep account identifiers, -private reference documents, drafts, and validation logs out of the repository. - -### Release Contributor Attribution - -Keep the shipped product changes as the primary release narrative. When the tag -range contains merged work from community contributors other than project -founder `@huangruiteng`, add a prominent `## Community Contributors` section -after the English product groups and a matching `### 社区贡献者` section after -the Chinese product groups. Place both before compatibility, validation, or -update material so the credit remains visible without replacing the release -summary. - -Link each eligible contributor's GitHub handle and relevant pull requests, and -describe the concrete contribution instead of publishing an unannotated name -list. Explicitly highlight external and first-time contributors when -applicable. Do not list or thank `@huangruiteng` in contributor sections; -founder stewardship is implicit in LoopX releases. Omit both contributor -sections when the tag range has no eligible community contribution. - -Derive attribution from the previous-tag-to-current-tag Git range plus merged -PR metadata. Do not guess from commit display names, omit contributors because -their work is summarized elsewhere, let contributor credit displace product -content, or invent community attribution for a founder-only release. The -release PR and final GitHub release body must preserve the same bilingual -attribution. - -### Release Capability Usage Gate - -When a release introduces or materially changes an optional capability, -workflow, managed skill, or host surface, the final GitHub release body must -teach the user how to operate it. For every affected surface, include matching -English and Chinese entries with: - -- exact activation or per-command/profile opt-in; -- a minimum runnable validation or readback command; -- exact disable, uninstall, envelope-removal, or rollback guidance; -- the authority and privacy boundary that activation does not grant; -- a canonical versioned documentation link. - -Save the complete final release body in an ignored or temporary Markdown file. -Run `examples/release/release-readiness-doc-smoke.py --release-notes ...` with -one `--surface` argument per affected surface before publication, then read the -remote body back and run the same check again. A release with no applicable -surface changes must use `--expect-no-optional-capability-changes` and include -the validator's explicit bilingual no-change declarations. Never treat the -existence of a checklist, a release PR draft, or architecture-only capability -copy as proof that the final release body is usable. +### Staging And Public Boundary + +Before staging non-trivial changes (including benchmark adapters, smokes, +public docs and commit/push workflows): + +1. Read `git status --short --branch`, `git diff --stat`, `git diff --name-only` + and `git ls-files --others --exclude-standard`. +2. Classify every changed path: product code, core docs, durable validation + smoke, local/private state, or low-value/obsolete artifact. +3. Scan candidates for credentials, private state, local absolute paths, raw + benchmark logs/trajectories/verifier output and internal links. +4. Stage explicit pathspecs only; never `git add .`. +5. Split by reviewer logic: runtime/API behavior, public docs/protocol notes, + and focused validation/cleanup. Push reviewable batches as branch PRs. + +### Merge Authority And Validation + +Before approval or merge, read the repository-kept +[merge workflow](skills/loopx-pr-merge/SKILL.md); installing it is optional. +It does not replace repository policy or the current capability-owned review. + +Control-plane, runtime and product surfaces require explicit maintainer merge +authorization. This includes behavior in `loopx/**`, `apps/**` or `packages/**`, +permissions or authority, persisted state/receipts, quota/scheduler/todo rules, +and CLI/API contracts. Propose and review +the exact head, pass required validation, and obtain ready merge-readiness for +that unchanged head. Only then may an explicitly authorized merge or delivery +batch use admin bypass. Small diffs, green CI or published reviews do not grant +merge authority. Unclear affected boundaries use the same authorization rule; +without it, leave the merge to the maintainer. Bypass never replaces review, +validation or readiness. + +Self-merge means self-review/refinement, a published exact-head review, then +merge under the required validation and authorization. For author-owned PRs, +GitHub blocks formal self-approval: publish a `COMMENTED` review carrying the +approval conclusion and an English verdict. A diff read, green CI or the merge +itself is not this record. If a self-merge lacks it, publish the review for the +merged commit and repair the rule that allowed the gap. + +Maintainers may self-merge single-purpose, low-risk public-doc, contributor +metadata, test, example or narrow-cleanup PRs when all these conditions hold: + +- none of the protected behavior surfaces above is changed; +- the diff is easy to review and required checks/focused smokes passed; +- its exact head has the published review, and + `uv run --extra test loopx pr-review --goal-id GOAL --check-merge-readiness NUMBER@HEAD_OID` + returned ready for that unchanged head; +- no private state, raw benchmark evidence, credentials, local paths or + generated logs are included; +- no runtime behavior, benchmark adapter, permission, destructive Git or public + evidence-policy change requires separate review. + +The existing benchmark exceptions retain these boundaries: + +- Small seam/refactor PRs (as in #145) may add/clarify a reusable adapter or + control-plane contract with focused public smokes; they do not launch jobs or + change existing scoring/runner behavior, and exclude temporary probes, raw + evidence, private state, credentials, local paths and generated logs. +- After owner authorization, public benchmark helper/runtime, status observation, + reducer/closeout or developer-workflow PRs may self-merge when focused smokes + or compile checks pass and public/private scans are clean. They must not alter + scoring, task semantics, leaderboard/submission behavior, permission boundaries + or launch jobs. + +Before any non-trivial self-merge, run +`uv run --extra test loopx canary premerge --from-git-diff --git-diff-base "$PR_BASE_REF"` +or equivalent risk-based validation. The PR comment names changed surfaces, +checks, failures/skips, manual holds and why coverage is sufficient. One +hand-picked smoke is insufficient for runtime, quota/status, scheduler, todo, +install, dashboard, benchmark-boundary or public/private evidence changes. +After merging, sync local `main`, preserve unrelated untracked artifacts and +continue the next safe project batch. + +### Named Public Releases + +Before completing a named public release, follow the existing release owners: + +- [Personal Feishu guide](docs/product/release-readiness.md#personal-feishu-release-guide): + verified personal user ownership, illustrated released UI, upgrade/use guidance + and ownership/text/image readback; no corporate/bot substitution or implied + chat-announcement authority. Keep account ids, drafts and raw evidence private. +- [Contributor attribution](docs/product/release-readiness.md#release-note-checklist): + derive eligible non-founder community contributions from the tag range and + merged PR metadata, preserve concrete bilingual attribution and product-first + ordering, and omit both contributor sections when none qualifies. +- [Final release body](docs/product/release-readiness.md#final-release-body-usage-gate): + teach each changed optional surface's activation, validation, disable/rollback, + authority/privacy boundary and versioned docs in both languages. Validate the + complete final body before publishing and after remote readback; no-change + releases require the validator's explicit bilingual declaration. A candidate, + checklist or release PR draft does not prove a released, usable capability. ## First-Screen Review Evidence -Treat the first visible screen of public product surfaces as reviewable product -presentation. For authorized work within the requested outcome and existing -product/design contract, do not add a separate user-confirmation round before commit, -push or PR submission merely because the first viewport, hero, primary CTA or -opening navigation changes. Reuse prior scoped approval; a new commit or a -routine refinement does not invalidate it. - -Before submitting or updating the PR, inspect the actual first viewport and put -its screenshot or browser capture directly in the PR description's visual -evidence section. Include a useful before/after comparison for an existing -surface, affected desktop/narrow views, and a working preview link when one is -available. Identify the tested revision and whether the data and surface are -synthetic, a prototype, packaged or installed. Use public-safe evidence; keep -private messages, paths and live operating context out. A screenshot only in -chat, a PR comment, a Todo note or the final report does not satisfy this rule. -Refresh the evidence when the visible result materially changes, without -requesting another approval for the same authorized direction. - -Ask only for a genuinely missing consequential decision, work outside the -accepted scope, or an explicit user instruction reserving presentation approval. -Do not infer such a reservation from a first-screen change alone. Name the -specific unresolved choice and continue independent authorized work. This -presentation evidence rule does not grant merge, installation, release, paid -execution or sensitive-data publication authority. Existing approval records -remain historical evidence; older prose that merely repeats the blanket -first-screen confirmation requirement follows this current rule. +Treat public first screens as reviewable product presentation. Within the +requested outcome, existing design contract and scoped authorization, do not +add a confirmation round before commit, push or PR submission merely because +the viewport, hero, primary CTA or opening navigation changes. Reuse prior +approval; a new commit or routine refinement does not invalidate it. + +Before submitting/updating the PR, inspect the actual first viewport and put a +screenshot/browser capture directly in the PR description's visual evidence. +Include useful before/after views for existing surfaces, affected desktop/narrow +views and a working preview link when available. State the tested revision and +synthetic/prototype/packaged/installed boundary. Evidence must be public-safe: +no private messages, paths or live operating context. Chat, PR comments, Todos +or final reports alone do not satisfy this rule. Refresh evidence on material +visual changes without asking again for the same authorized direction. + +Ask only for a missing consequential decision, work outside accepted scope, or +an explicit user reservation of presentation approval. A first-screen change +alone creates no such reservation. Name the unresolved choice and continue +independent authorized work. This rule grants no merge, install, release, paid +execution or sensitive-publication authority. Existing approvals remain historical +evidence; older blanket-confirmation prose follows this current rule. ## Product Delivery Completeness -For product changes, identify affected user entry points (frontend, Lark, CLI) -while planning. Inspect existing settings and capability editors before calling -a configuration change backend-only; include necessary companion work in the -same delivery plan, reusing the existing configuration owner and projection. - -Every new or materially extended caller-facing capability must enter the -shipped frontend through an existing appropriate surface: users can discover -its purpose, perform the authorized user operation, see unavailable/error states, -and read back the result. A CLI command, catalog row, static description or -"copy this command" button alone does not complete frontend delivery. Reuse the -same typed owner; frontend presence does not require a duplicate configuration -switch or automatic execution of privileged tools. Internal helpers and providers -belong to their owning capability's interaction rather than a separate screen. - -Before PR handoff, validate that journey in the packaged frontend alongside the -real owning rule/backend, including a relevant failure or recovery case. State -which frontend/Lark/CLI entrypoints changed and what was verified. A staged CLI -or backend prerequisite may be proposed as partial, with its remaining frontend -journey and linked owner; do not mark the capability complete or use "developer -only" as an automatic exemption. These are agent-owned completion checks, not -new approval gates. +For product changes, read [task-first frontend delivery](docs/development/frontend-delivery.md#task-first-delivery--从用户任务开始) +before implementation. Identify affected frontend/Lark/CLI entrypoints and +inspect existing settings/capability editors before declaring a change backend-only. +Every new or materially extended caller-facing capability needs a shipped +frontend journey: discover purpose, perform the authorized operation, see +unavailable/error states and read back the result. Catalog text, a CLI command +or a copy-command button alone is incomplete. Reuse the typed owner; do not add +duplicate configuration. Frontend presence does not require automatic execution +of privileged tools. Internal helpers/providers belong to their owning +capability's interaction, not an independent screen. + +Before PR handoff, validate the packaged journey with its real owning backend +and a relevant failure/recovery case. Name changed entrypoints and evidence. +A staged prerequisite may be proposed as partial with its remaining journey and +linked owner; "developer only" is not an exemption. Completion checks do not +create new approval gates. Distinguish proposed, packaged and installed behavior. ## UI Design Standard -Apply the whole-viewport attention review in `docs/development/design.md` -("Earn The User's Attention") before implementation and in PR visual evidence. -For production-bound interactive UI changes, carry one ordinary user task through -`docs/development/frontend-delivery.md`: compare the shortest authorized journey -before coding, validate the packaged interaction and owning state, and distinguish -proposed changes from installed behavior. For documentation and prototypes, name -the simulated boundary without claiming packaged or installed readback. - -Before changing or reproducing any LoopX UI, read and follow the repository-root -`docs/development/design.md`. This includes websites, dashboards, desktop applications, -documentation, prototypes, screenshots, and framework migrations. When an -approved design source is provided, match it and use `docs/development/design.md` for unspecified -details. +Before changing or reproducing any UI (including websites, dashboards, desktop, +documentation, prototypes, screenshots or framework migrations), read +[the design system](docs/development/design.md), especially +[whole-viewport attention](docs/development/design.md#earn-the-users-attention). +Apply it before implementation and in PR visual evidence. Match an approved +design source and use the system for unspecified details. For production +interactions follow task-first delivery above; label simulations and prototypes +without claiming packaged or installed readback. ## Public And Private Boundary -Do not commit internal department, team, customer, meeting, reporting, -strategy, or local operating context into the public repository. This includes -planning notes, status narratives, rollout stories, fixtures, screenshots, -examples, catalog rows, PR descriptions, and review artifacts whose value -depends on private organizational context rather than reusable public product -behavior. - -Keep private planning and incident evidence in ignored local state such as -`.local/`, or another explicitly ignored owner-approved location. Only commit -material after it has been generalized into public-safe product, maintainer, or -developer language and no longer reveals internal actors, timelines, reporting -needs, local paths, raw logs, private links, or private decision context. - -Before staging public docs, fixtures, examples, catalogs, or metadata, scan the -candidate paths for internal/private wording and ignored local artifact -references. If such context was already pushed, stop normal delivery, run -LoopX self-repair, clean the current public heads, and record any remaining -PR-ref or cached-view cleanup as an explicit user/support gate. +Never publish private department/team/customer/meeting/reporting/strategy or +local operating context in repository changes, PR descriptions or review +artifacts, including notes, fixtures, screenshots, examples and catalog rows. +Keep private planning/incident evidence in ignored `.local/` or another explicitly +ignored owner-approved location. Generalize material into public-safe product, +maintainer or developer language before publication; remove internal actors, +timelines, reporting needs, local paths, raw logs, private links and decisions. + +Before staging public docs, fixtures, examples, catalogs or metadata, scan +candidates for private wording and ignored-artifact references. If already +pushed, stop normal delivery, use self-repair, clean current public heads and +report remaining PR-ref/cached-view cleanup as an explicit user/support gate. ## PR Review Comments -When the user asks the agent to review a GitHub PR, treat PR feedback as a -public collaboration artifact by default. After validating the findings, -publish actionable review findings directly on the PR as a comment or review, -unless the user explicitly asks for a local-only review or the finding contains -private/security-sensitive material that must not be posted publicly. - -Do not leave actionable PR blockers only in chat memory. The final user report -should include the PR comment URL and a compact summary of the posted findings. - -Before publishing a review, run the capability-owned review lenses for typed -state rules, domain neutrality, behavior-change disclosure, and -guidance-vs-obligation, plus default-off isolation and authority semantics -(defined in `pull_request_review_execution_contract_v2`). -Flag substring denylists or prose-only classification rules with the concrete -misclassification risk, product- or benchmark-specific wording in generic -control-plane contracts, silent default-behavior changes, and text that calls a -machine-enforced obligation "guidance". For opt-in behavior, prove feature-off -parity across every shared changed surface. Reject protocol names that imply a -broader actor lifecycle or authority model than the implementation provides. +For a requested GitHub PR review, publish validated actionable findings on the +PR by default, unless local-only was requested or the material is private or +security-sensitive. Do not leave blockers only in chat; return the published URL +and a compact summary. Use [the review workflow](skills/loopx-pr-review/SKILL.md) +and execute the current capability-owned `pull_request_review_execution_contract_v2`. + +Before publication, apply its typed-state, domain-neutrality, behavior-change, +guidance-vs-obligation, default-off-isolation and authority lenses. Flag concrete +substring/prose-classification misclassification risks, domain-specific generic +contracts and silent default changes. Prove feature-off parity on every changed +shared surface. Do not call enforced obligations guidance or give a protocol a +name that claims broader actor lifecycle/authority than it implements. ## Engineering Quality And Right-Sized Scope ### TypeScript-First Implementation -Prefer TypeScript for new or refactored domain-neutral LoopX capabilities, -control-plane and orchestration logic, capability composition, context/memory -composition, and shared projections. Follow the accepted -[TypeScript migration RFC](docs/architecture/rfcs/typescript-control-plane-migration-v0.md) -and reuse the existing typed owner. Do not introduce a parallel Python decision -owner. Stable domain providers, specialized computation and transport adapters -may remain Python with an explicit placement rationale; this preference does -not require unrelated language-only rewrites or make a full migration a -prerequisite for a bounded user outcome. - -尽量用 TypeScript 实现新增或重构的 LoopX 通用能力、控制面、编排、能力组合、上下文/记忆组合及共享投影，遵循 TS 重构 RFC 并复用既有类型化 owner，不新增平行的 Python 决策源。稳定领域 provider、专门计算与传输适配器可保留 Python，但须说明归属理由；不因语言偏好扩大无关重写，也不把全量迁移设为有界用户结果的前置条件。 +Prefer TypeScript and the existing typed owner for new/refactored domain-neutral +capabilities, control plane, orchestration, capability/context/memory composition +and shared projections. Follow the accepted [migration RFC](docs/architecture/rfcs/typescript-control-plane-migration-v0.md); +do not create a parallel Python decision owner. Stable domain providers, +specialized computation and transport adapters may remain Python with a placement +rationale. Do not expand into unrelated language rewrites or make full migration +a prerequisite for a bounded outcome. ### Source-Checkout Python Entry Points -Run source development and validation from the intended worktree root with -`uv run --extra test python ...` or `uv run --extra test loopx ...`. Use -`uv sync --extra test` to prepare the project environment. An explicitly -activated compatible environment with the checkout installed remains valid. -Check `sys.executable` and `loopx.__file__` when interpreter or source provenance -is uncertain; a global `loopx` may point to another release snapshot. - -Keep Python subprocesses on the selected interpreter (`sys.executable`), and -keep bootstrap interpreter discovery, supported-version declarations, CI -version coverage, and version-specific fixtures intact. Do not replace those -with a nested `uv run`, rewrite historical execution receipts, or commit a -generated `uv.lock` as part of an unrelated change. See the testing and quality -guide for the validation layers and the source-checkout environment boundary. +From the intended worktree root, prepare with `uv sync --extra test` and use +`uv run --extra test python ...` / `uv run --extra test loopx ...`; an activated +compatible environment with that checkout installed is also valid. Follow the +[environment guide](docs/development/testing-and-quality.md#local-validation-environment--本地验证环境). +If provenance is uncertain, check `sys.executable` and `loopx.__file__`; a global +CLI may use another snapshot. Keep subprocesses on `sys.executable`, not nested +`uv run`. Preserve bootstrap discovery, supported versions, CI version coverage +and version-specific fixtures. Do not rewrite historical receipts or commit an +unrelated generated `uv.lock`. ### Evidence-Based Budget Decisions -When a size, structure, latency, or similar regression budget fails, follow the -[budget decision guide](docs/development/testing-and-quality.md#budget-failure-decisions). -Establish what the limit protects, measure the same base/head workload, and -inspect consumer value and true redundancy before choosing compaction, keeping -the limit, or an evidence-backed increase. Preserve decision semantics and -compatibility; neither a historical ceiling nor a green revised test is the -objective. Record the tradeoff in existing PR validation/review evidence, not a -new approval workflow. Hard limits and frozen experiment/promotion criteria -retain their owning authority and cannot be reclassified to erase a failure. - -LoopX is a semantic control plane: agent-consumed prose is part of its behavior -contract. Before shortening packet guidance, compare obligations clause by -clause, including ordering, evidence provenance, modality, scope qualifiers, -continuation and stop conditions. Stable fields, commands and decision codes -do not prove that shorter instructions preserve those obligations. A regression -ceiling is not frozen authority: prefer an evidence-backed limit increase when -compression would discard useful meaning. Use the existing review capability's -`observable_semantics.decision_text_assessment`; do not optimize for a green -size test at the expense of the caller's decision. +When a size, structure or latency budget fails, follow the +[budget decision guide](docs/development/testing-and-quality.md#budget-failure-decisions): +measure the same base/head workload, identify what the limit protects and +compare information value and real redundancy before compaction, retention or +an evidence-backed increase. Hard limits and frozen experiment/promotion criteria +retain their authority; never relabel an earlier failure or add an approval flow. + +Agent-consumed prose is behavior. Before shortening it, compare obligations +clause by clause: actor, ordering, modality, evidence provenance, scope, +continuation and stop conditions. Stable fields/commands/enums do not prove +instruction parity. Use `observable_semantics.decision_text_assessment` from the +current review capability; prefer justified regression-budget growth over losing +useful meaning. A host's hard loading limit needs complete delivery through +reachable task-specific instructions, not silent truncation. ### Refactor Real-Path Validation -Before delivering a refactor, validate the affected production entrypoint and -real backend, not only mocks, in-memory substitutes, or unit tests. Authority -store refactors that affect PostgreSQL must run the PostgreSQL integration -suite against an isolated real server; report the exact source and results. -Use a separate database/tenant and disposable runtime with synthetic fixtures -or an owner-authorized read-only snapshot. Never test by promoting, rewriting, -or corrupting an active goal, its registry, writer fence, Todo, or lease state. -If the required real environment is unavailable, report the evidence gap and -hold delivery; a skipped test does not satisfy this gate. See the testing and -quality guide for the same safety and evidence boundary. - -Treat code volume as a cost, especially during refactors. A good LoopX change -should make the next change easier to localize, test, and revert; it should not -turn a design possibility into unused production structure. +Before claiming refactor delivery, follow the [real-path gate](docs/development/testing-and-quality.md#refactor-real-path-gate--重构真实路径门): +validate the affected production entrypoint and real backend, beyond mocks or +unit tests. PostgreSQL authority-store changes require its integration suite on +an isolated real server; report exact source/results. Use a separate database or +tenant and disposable runtime with synthetic fixtures or owner-authorized +read-only snapshots. Never test by promoting, rewriting or corrupting active +Goal/registry/writer-fence/Todo/lease state. An unavailable environment is an +evidence gap that holds delivery qualification, not a passing skip. Keep code +volume a cost: refactors should localize the next change, test and rollback, +not introduce unused production structure. ### Bounded Future-Facing Refactoring -During development and again before approving or merging each PR, explicitly -ask whether the touched behavior or its adjacent owning boundary has a small, -related, behavior-preserving refactor that would make the next likely product -or control-plane change easier. Prefer removing duplicate authority, -strengthening typed contracts, narrowing module ownership, and retiring -obsolete compatibility seams over adding speculative frameworks. For -control-plane work, keep state-machine and effect authority in the typed -TypeScript boundary when that is the established owner; Python may adapt or -bridge that contract, but must not silently recreate a second source of truth. - -Apply the refactor in the current PR when it shares the same domain or change -reason, remains locally reviewable and reversible, and is covered by -characterization or parity validation; it need not be strictly required for -the immediate fix. Do not use this principle to justify broad migration or -unrelated cleanup. When the valuable related refactor is larger than a bounded -companion change, record a focused follow-up instead. PR review and merge notes -should state whether this future-facing pass was applied, deferred, or found -unnecessary, with the concrete boundary considered. +During development and before approving or merging, apply the +[bounded refactoring and scope-fit review](docs/development/README.md#engineering-scope-and-refactoring) +to the touched behavior and adjacent owner. Include a same-domain, reversible, +behavior-preserving improvement when locally reviewable and covered by parity; +it need not be strictly required for the immediate fix. Keep larger work as a +focused follow-up. Report applied/deferred/unnecessary with the concrete boundary; +no speculative framework or unrelated migration. ## Capability And Extension Placement -Before adding an ability, decide its capability owner and provider boundary; -do not choose a directory from the feature name alone: - -- name public capabilities after caller outcomes, not delivery mechanisms. A - proposed `connector`, `provider`, `adapter`, or `sink` capability needs an - independently useful caller contract; otherwise make it an extension - provider or an internal part of the outcome capability it serves; -- extend `loopx/capabilities//` when the change belongs to an - existing product contract and shares that built-in capability's lifecycle; -- create a new built-in capability only when LoopX core must ship it by - default and it has a stable caller contract, real entrypoint, and focused - validation; -- put generic manifest, registration, compatibility, and lifecycle mechanics - in `loopx/extensions/`; -- put an independently versioned or optional provider distribution in - `packages//` when it is co-located, or in its own package or - repository when it is distributed separately. Reserve `loopx/extensions/` - for extension lifecycle code and providers bundled in the LoopX wheel; -- do not create a capability merely to make an extension installable. An - extension-owned command or workflow may declare only its runtime and - lifecycle when LoopX callers do not need a provider-neutral capability - contract; -- when a provider introduces a new product contract, register the capability - contract and implement it through the extension only when that contract is - intentionally provider-neutral and belongs in LoopX's capability catalog; -- keep private helpers in the nearest owning module. A helper is not a new - capability or extension merely because several files are involved. - -Record the placement rationale before editing: capability id, provider id, -whether the provider is built-in or extension-delivered, and why the nearest -existing owner is or is not sufficient. See `docs/reference/extensions.md` for the full -decision guide. - -Before adding a new module, builder, protocol field, CLI option, fixture, smoke -section, or abstraction, pass a scope-fit review: - -- Identify the shipped behavior, active call site, or explicit compatibility - contract that needs it. If the value is only an uncommitted future runner, a - design note, or a hypothetical extension with no validation contract, keep the - design in docs or todo state until the real call site appears. -- Prefer a cohesive behavior-preserving seam. For example, let the ledger first - recognize one compact public-safe row shape before adding a dedicated - benchmark-specific builder, arm constants, or wide field-level smoke. Do not - split so narrowly that reviewers must reconstruct one logical behavior from - several dependent PRs. -- Design tests from semantics, not observed output. Independently review the - intended invariant and legal or illegal transitions before testing the - implementation. Never derive expected results from the implementation under - test or its current output; characterization fixtures are non-authoritative, - and contradictions require rule repair plus negative or mutation coverage. -- Characterize before moving code. For status, quota, review-packet, scheduler, - monitor, and handoff behavior, add or extend parity fixtures first, then - extract the proven rule or cohesive rule group. -- Reuse existing repository patterns and bounded contexts. Add code where its - change reason belongs, such as `control_plane/runtime`, `control_plane/quota`, - or `control_plane/todos`; do not create generic sink directories or helper - layers just because several files share a similar shape. -- Treat large or hot files as warning signals. When a change would grow an - already oversized module, first look for a narrow read model, domain helper, - or bounded-context home. For internal module moves, update active call sites - and delete the old entry point; leave a compatibility wrapper only when a - real external import, persisted state, CLI/API contract, or migration window - requires it. -- Distinguish duplicate knowledge from duplicate-looking code. Collapse shared - state rules, protocol semantics, serialization contracts, and lifecycle - invariants; avoid a parameter-heavy abstraction when two callers merely look - similar but will evolve for different reasons. -- Keep smokes thin and durable. They should prove shipped behavior, boundary - enforcement, and regression contracts, not every incidental field produced by - a temporary builder. Large smokes are a prompt to move reusable logic into - product modules or to narrow the assertion surface. -- Make illegal states hard to express. For status, quota, scheduler, monitor, - todo, and handoff flows, prefer explicit enums, schemas, and transition - helpers over scattered booleans and prose-only assumptions. -- State-classification and delivery-semantics rules belong in typed enums, - schemas, or transition helpers. Substring denylists and prose heuristics must - carry a documented reason and a typed follow-up; PR review must flag them - with the concrete false-positive/negative risk. -- Core control-plane obligations and error text must stay domain-neutral. Do - not put product- or benchmark-specific wording (for example "product - advancement") into generic work-lane, quota, todo, or settlement contracts; - prefer goal-agnostic phrasing. -- Default behavior changes must be disclosed: rename the smoke that encoded the - old default, update docs/release notes, and name the affected lanes. - "Guidance" versus machine-enforced obligations (such as `must_attempt_work`) - must be explicit in the contract, not inferred from prose. -- Fail fast with actionable context at input, config, permission, and state - boundaries, but do not replace clear control flow with broad exception - plumbing or silent fallback. -- Ship right-sized, reversible batches. A PR should be theme-unified, locally - validated, and reviewable as a complete stage package. A few hundred to - roughly one or two thousand lines can be appropriate when the diff is cohesive - and avoids hidden future scaffolding; a 30-line PR can still be too small if it - leaves behavior split across follow-up PRs. Separate characterization/parity - fixtures, mechanical moves, behavior changes, and cleanup when that makes - review and rollback clearer. -- Keep public PRs concise and current-purpose focused. Future extension points - are allowed when they reduce near-term churn, preserve compatibility, or - define a real contract that is documented and tested. Do not bundle - private/local experiment scaffolding, diagnostic run dumps, unused speculative - plumbing, or long background narratives with the code path needed by the - current behavior. -- Compress rather than append. For docs, fixtures, dashboards, and examples, - replace or retire stale material when adding new current truth; do not let - canonical surfaces accumulate multiple versions of the same conclusion. - -Use this checklist to delete, defer, or right-size code as actively as you add -it. A PR that removes an unused abstraction, narrows a smoke to the real -contract, or moves a rule into the right bounded context is often more valuable -than one that adds a larger framework around the same behavior. +Before adding an ability, read [placement decisions](docs/reference/extensions.md#placement-decision-for-agents). +Name public capabilities after caller outcomes; reuse the nearest capability +and typed owner. Record capability id, provider id, builtin/extension origin +and why that owner is or is not sufficient before editing. The guide owns +builtin capability, optional distribution, extension lifecycle and private-helper +placement; independent installation alone does not justify a new capability. + +Before adding a module, builder, protocol field, CLI option, fixture, smoke section +or abstraction, apply [scope-fit review](docs/development/README.md#scope-fit-review). +Require an active caller, shipped behavior or explicit compatibility contract; +characterize before moving code and derive tests from independent semantics. +Keep state and delivery classifications typed, generic obligations domain-neutral, +and default-behavior changes disclosed. Prose heuristics need a documented reason, +a typed follow-up and review of misclassification risk; distinguish guidance from +machine-enforced obligations. Prefer complete, reversible slices, replace stale +canonical material, and avoid unused structure or duplicate authority. ## Automation And Monitor Todos -Do not hard-code one-off project or PR monitor logic into a generic heartbeat -automation prompt. Recurring project-specific watches, such as "monitor PR #532 -until merge", belong in LoopX state as `continuous_monitor` todos with compact -metadata such as `claimed_by`, `unblocks_todo_id`, and evidence notes. The -heartbeat prompt should remain generic and discover monitor work through -status, quota, and todo projection. Only update an automation prompt when the -heartbeat lifecycle contract itself changes or the user explicitly asks to -change the scheduler. +Keep generic heartbeat prompts free of one-off project/PR monitor policy. +Recurring watches belong in canonical `continuous_monitor` Todos with compact +`claimed_by`, `unblocks_todo_id` and evidence metadata; discover them through +status/quota/Todo projections. Change the saved prompt only for heartbeat +lifecycle contract changes or an explicit user request to change the scheduler. +See [prompt layers](docs/heartbeat-automation-prompt.md#two-prompt-layers). ## Projection Sink Design -When adding an operator-facing display sink such as Lark Base, dashboards, -chat summaries, or reports, build it from LoopX's public-safe state and -projection surfaces instead of parsing project-specific private source files. -Valid display inputs include todo projection, quota/status contracts, -frontstage projections, compact run-history events, public-safe evidence -pointers, and redacted source warnings. - -Do not make a generic sink depend on the shape of one local document, private -planning file, non-public wiki, raw transcript, local path, or connector payload. -If a source is valuable, first convert it into a bounded LoopX projection with -stable ids, source labels, evidence, and explicit gates; then let the sink -render that projection. Public or multi-user sinks must consume redacted -public-safe evidence. An explicitly owner-only operator board may sync private -planning evidence when the user authorizes that boundary, but it should still -scope rows by `agent_id` and avoid credentials or secrets unless the user -explicitly asks for a credential-handling workflow. - -Projection sinks should preserve row lineage as data, not as ad hoc prose. When -rows supersede, migrate, or retire earlier display rows, represent that through -projection lifecycle fields such as `row_lifecycle`, `supersedes`, -`superseded_by`, `source_id`, and compact migration audit evidence. A sink may -render the lineage in existing evidence/history fields, but should not require -reading a project-private source document to understand why a row changed. +When adding a display sink (Lark Base, dashboard, chat summary or report), follow +[the projection sink contract](docs/product/core-control-plane/state-machine.md#7-projection-sink-machine). +Use bounded public-safe state projections rather than project-private raw files; +retain source/evidence/gates. Sinks should preserve row lineage as data. +Public/multi-user sinks require redacted evidence. An owner-only private planning board requires explicit +user authorization for that boundary and retains the contract's agent scope and +credential restrictions. A sink displays state; it does not become its authority. ## Smoke Retention Policy -Keep a smoke test only when it validates a durable public behavior: - -- shipped CLI/runtime behavior; -- a reusable control-plane contract; -- public/private boundary enforcement; -- a regression that previously stranded automation; -- a representative fixture that is likely to catch future bugs. - -Do not keep one-off smokes whose main purpose is to assert the exact text of a -dated research note, candidate ranking packet, temporary run review, or -transitional benchmark decision. Preserve that information in the research doc -itself, and cover shared invariants with a data-driven aggregate smoke. - -When a smoke grows beyond roughly 500 lines, re-check whether it is really one -test. Prefer splitting reusable logic into product modules and keeping the -smoke as a thin public behavior check. Large integration smokes are acceptable -only when they cover a real end-to-end adapter contract that smaller unit tests -cannot cover. - -Benchmark smokes must never require raw task text, raw trajectories, raw logs, -verifier output tails, credentials, uploads, leaderboard submissions, or local -private artifact paths. +Before adding, retaining or consolidating a smoke, read +[good smokes](docs/development/good-smokes.md), including independent semantic +oracles, durable behavior, public-safe fixtures and preservation of unique +integration coverage. Keep shipped behavior, reusable contracts, boundary checks, +automation regressions and representative fixtures likely to catch future bugs; +do not freeze dated prose or temporary builders. Roughly 500 lines triggers a +review, not automatic deletion; thin smoke assertions should exercise product logic. + +For benchmark changes, follow [benchmark smoke boundaries](docs/development/good-smokes.md#benchmark-boundaries) +and [archive placement](benchmark/README.md#archive-placement). Active smokes +protect stable toolkit contracts; experiment-specific validation stays with the +research workspace. Archives do not enter active CI benchmark execution. Never +require raw tasks, trajectories, logs, verifier tails, credentials, uploads, +leaderboard submissions or private local artifact paths. ## LoopX Self-Repair @@ -666,22 +437,3 @@ or process gaps: update the skill, interaction docs, active-state projection, or focused smoke so the lesson is durable. Do not resolve self-repair by lowering gates, guessing around contradictory payloads, or committing private logs and local state. - -## Benchmark Smoke Classification - -Use this classification when cleaning or reviewing benchmark-related changes: - -- Keep focused boundary smokes such as - `examples/benchmark-candidate-source-boundary-smoke.py`; they guard a reusable - public/private source contract. -- Keep toolkit permission and integrity smokes while they validate the shipped - provider-neutral capability contract. -- Keep benchmark-native runners, adapters, ledgers, scoring reducers, and dated - experiment packets outside the active product surface. Historical versions - follow the canonical archive placement rules in `benchmark/README.md`: - retired implementations and dated packets belong under - `deprecate/benchmark-legacy/`; explicitly identified immutable experiment - snapshots may remain under `benchmark/` only under that document's conditions. - Neither category is part of active CI benchmark execution. -- Add a new active benchmark smoke only when it protects a stable toolkit - behavior; experiment-specific validation belongs with the research workspace. diff --git a/docs/development/README.md b/docs/development/README.md index fd75498dc9..4d7cf5ecf2 100644 --- a/docs/development/README.md +++ b/docs/development/README.md @@ -94,3 +94,111 @@ issue or regression 通常闭环为：问题或回归 -> 确定性刻画 -> 聚焦实现 -> 单测与 durable smoke -> catalog 选择的 canary -> 敏感变更 owner review -> 必要时观察发布结果。 + +## Engineering Scope And Refactoring + +Before adding a module, protocol field, CLI option, fixture or abstraction, +apply the scope-fit review below. During development and again before approval +or merge, apply the bounded refactoring pass. These are engineering decisions +within the accepted task, not additional user-approval gates. + +### Bounded Future-Facing Refactoring + +During development and again before approving or merging each PR, explicitly +ask whether the touched behavior or its adjacent owning boundary has a small, +related, behavior-preserving refactor that would make the next likely product +or control-plane change easier. Prefer removing duplicate authority, +strengthening typed contracts, narrowing module ownership, and retiring +obsolete compatibility seams over adding speculative frameworks. For +control-plane work, keep state-machine and effect authority in the typed +TypeScript boundary when that is the established owner; Python may adapt or +bridge that contract, but must not silently recreate a second source of truth. + +Apply the refactor in the current PR when it shares the same domain or change +reason, remains locally reviewable and reversible, and is covered by +characterization or parity validation; it need not be strictly required for +the immediate fix. Do not use this principle to justify broad migration or +unrelated cleanup. When the valuable related refactor is larger than a bounded +companion change, record a focused follow-up instead. PR review and merge notes +should state whether this future-facing pass was applied, deferred, or found +unnecessary, with the concrete boundary considered. + +### Scope-Fit Review + +Before adding a new module, builder, protocol field, CLI option, fixture, smoke +section, or abstraction, pass a scope-fit review: + +- Identify the shipped behavior, active call site, or explicit compatibility + contract that needs it. If the value is only an uncommitted future runner, a + design note, or a hypothetical extension with no validation contract, keep the + design in docs or todo state until the real call site appears. +- Prefer a cohesive behavior-preserving seam. For example, let the ledger first + recognize one compact public-safe row shape before adding a dedicated + benchmark-specific builder, arm constants, or wide field-level smoke. Do not + split so narrowly that reviewers must reconstruct one logical behavior from + several dependent PRs. +- Design tests from semantics, not observed output. Independently review the + intended invariant and legal or illegal transitions before testing the + implementation. Never derive expected results from the implementation under + test or its current output; characterization fixtures are non-authoritative, + and contradictions require rule repair plus negative or mutation coverage. +- Characterize before moving code. For status, quota, review-packet, scheduler, + monitor, and handoff behavior, add or extend parity fixtures first, then + extract the proven rule or cohesive rule group. +- Reuse existing repository patterns and bounded contexts. Add code where its + change reason belongs, such as `control_plane/runtime`, `control_plane/quota`, + or `control_plane/todos`; do not create generic sink directories or helper + layers just because several files share a similar shape. +- Treat large or hot files as warning signals. When a change would grow an + already oversized module, first look for a narrow read model, domain helper, + or bounded-context home. For internal module moves, update active call sites + and delete the old entry point; leave a compatibility wrapper only when a + real external import, persisted state, CLI/API contract, or migration window + requires it. +- Distinguish duplicate knowledge from duplicate-looking code. Collapse shared + state rules, protocol semantics, serialization contracts, and lifecycle + invariants; avoid a parameter-heavy abstraction when two callers merely look + similar but will evolve for different reasons. +- Keep smokes thin and durable. They should prove shipped behavior, boundary + enforcement, and regression contracts, not every incidental field produced by + a temporary builder. Large smokes are a prompt to move reusable logic into + product modules or to narrow the assertion surface. +- Make illegal states hard to express. For status, quota, scheduler, monitor, + todo, and handoff flows, prefer explicit enums, schemas, and transition + helpers over scattered booleans and prose-only assumptions. +- State-classification and delivery-semantics rules belong in typed enums, + schemas, or transition helpers. Substring denylists and prose heuristics must + carry a documented reason and a typed follow-up; PR review must flag them + with the concrete false-positive/negative risk. +- Core control-plane obligations and error text must stay domain-neutral. Do + not put product- or benchmark-specific wording (for example "product + advancement") into generic work-lane, quota, todo, or settlement contracts; + prefer goal-agnostic phrasing. +- Default behavior changes must be disclosed: rename the smoke that encoded the + old default, update docs/release notes, and name the affected lanes. + "Guidance" versus machine-enforced obligations (such as `must_attempt_work`) + must be explicit in the contract, not inferred from prose. +- Fail fast with actionable context at input, config, permission, and state + boundaries, but do not replace clear control flow with broad exception + plumbing or silent fallback. +- Ship right-sized, reversible batches. A PR should be theme-unified, locally + validated, and reviewable as a complete stage package. A few hundred to + roughly one or two thousand lines can be appropriate when the diff is cohesive + and avoids hidden future scaffolding; a 30-line PR can still be too small if it + leaves behavior split across follow-up PRs. Separate characterization/parity + fixtures, mechanical moves, behavior changes, and cleanup when that makes + review and rollback clearer. +- Keep public PRs concise and current-purpose focused. Future extension points + are allowed when they reduce near-term churn, preserve compatibility, or + define a real contract that is documented and tested. Do not bundle + private/local experiment scaffolding, diagnostic run dumps, unused speculative + plumbing, or long background narratives with the code path needed by the + current behavior. +- Compress rather than append. For docs, fixtures, dashboards, and examples, + replace or retire stale material when adding new current truth; do not let + canonical surfaces accumulate multiple versions of the same conclusion. + +Use this checklist to delete, defer, or right-size code as actively as you add +it. A PR that removes an unused abstraction, narrows a smoke to the real +contract, or moves a rule into the right bounded context is often more valuable +than one that adds a larger framework around the same behavior. diff --git a/docs/development/good-smokes.md b/docs/development/good-smokes.md index c2823fffe5..2929fde0c2 100644 --- a/docs/development/good-smokes.md +++ b/docs/development/good-smokes.md @@ -113,6 +113,42 @@ evidence. 不要仅因 integrated smoke 很慢就删除它。应先测量成本来自哪些阶段，并证明较小测试 覆盖同一端到端边界；只要它仍提供独有证据，就应保留。 +When a smoke grows beyond roughly 500 lines, re-check whether it is really one +test. This is a review prompt, not an automatic deletion or failure threshold. +Move reusable behavior into its product owner and keep the smoke thin. Large +integration smokes remain appropriate when smaller tests cannot cover their +real end-to-end adapter contract. + +Do not retain one-off smokes whose main purpose is to assert the exact text of +a dated research note, candidate ranking packet, temporary run review, or +transitional benchmark decision. Keep that information in the research document +and cover shared invariants with a data-driven aggregate smoke. + +Smoke 超过约 500 行时，应重新检查它是否混合了多个测试；这不是自动删除或失败阈值。 +将可复用行为放回产品 owner，使 smoke 保持轻量；较小测试无法覆盖真实端到端 adapter +合同时，仍可保留大型 integration smoke。不要用一次性 smoke 固化过期研究笔记、候选 +排名、临时运行评估或阶段性 benchmark 决策的精确文案；这些信息留在研究文档，共有 +不变量交给数据驱动的聚合 smoke。 + +### Benchmark Boundaries + +Keep focused public/private boundary smokes such as +`examples/benchmark-candidate-source-boundary-smoke.py`, and toolkit permission +and integrity smokes while they validate the shipped provider-neutral contract. +Add active benchmark smokes only for stable toolkit behavior; experiment-specific +validation belongs with the research workspace. Benchmark-native runners, +adapters, ledgers, scoring reducers and dated packets stay outside the active +product surface. Follow [archive placement](../../benchmark/README.md#archive-placement): +retired implementations and dated packets belong under `deprecate/benchmark-legacy/`; +only the explicitly qualified immutable snapshots may remain under `benchmark/`. +Neither archive category enters active CI benchmark execution. + +保留公开/私有边界，以及已交付 provider-neutral toolkit 权限和完整性 smoke。 +新增 active benchmark smoke 只保护稳定 toolkit 行为，实验专属验证归研究工作区。 +Benchmark-native runner、adapter、ledger、scoring reducer 和过期 packet 不进入活跃 +产品面；退役实现和过期 packet 归 `deprecate/benchmark-legacy/`，满足规范条件的不可变 +快照才可留在 `benchmark/`。两类归档都不参与 active CI benchmark 执行。 + ## Public-Safe Fixtures / 公开安全 Fixture Use temporary repositories, fake transports, synthetic records, and compact @@ -131,6 +167,13 @@ smoke touches benchmark, host, agent, or external-system data. 当 smoke 接触 benchmark、host、agent 或外部系统数据时，遵循 [公开/私有边界](../public-private-boundary.md)。 +Benchmark smokes must never require raw task text, trajectories, logs, verifier +output tails, credentials, uploads, leaderboard submissions, or local private +artifact paths. + +Benchmark smoke 不得依赖原始任务文本、trajectory、日志、verifier 输出 tail、凭证、 +上传、leaderboard 提交或本地私有 artifact 路径。 + ## Validate The Change / 验证变更 Run the narrowest meaningful checks first, then add risk-based coverage: diff --git a/docs/product/core-control-plane/state-machine.md b/docs/product/core-control-plane/state-machine.md index 23fd67bb9f..01eb54c25f 100644 --- a/docs/product/core-control-plane/state-machine.md +++ b/docs/product/core-control-plane/state-machine.md @@ -396,10 +396,27 @@ flowchart LR | `Projection gap` | Missing concrete todo, stale route, conflicting source, or collapsed user/agent channel. | Repair the source or projection builder before relying on it. | | `Write API` | Todo update, gate decision, refresh-state, monitor poll, spend, scheduler ack, or event append. | Append durable facts; do not mutate the sink as truth. | -This machine protects the public/private boundary: a projection may render -public-safe summaries and evidence refs, but it must not become a dependency on -private raw docs, transcripts, credentials, local paths, benchmark logs, or -unredacted connector payloads. +Build operator-facing sinks from LoopX's public-safe state and projection +surfaces: todo projections, quota/status contracts, frontstage projections, +compact run-history events, public-safe evidence pointers and redacted source +warnings. A generic sink must not depend on the shape of a project-private +document, planning file, non-public wiki, raw transcript, local path, benchmark +logs or connector payload. First convert a valuable source into a bounded +projection with stable ids, source labels, evidence and explicit gates; then +render that projection. + +Public or multi-user sinks consume redacted public-safe evidence. An explicitly +owner-only operator board may synchronize private planning evidence when the +user authorizes that boundary, but should still scope rows by `agent_id` and avoid +credentials or secrets unless the user explicitly requests a credential-handling +workflow. A display sink does not gain write or publication authority from the +data it can read. + +Projection sinks should preserve row lineage as data rather than ad hoc prose. +When rows supersede, migrate or retire earlier display rows, use fields such as +`row_lifecycle`, `supersedes`, `superseded_by`, `source_id` and compact migration +audit evidence. The sink may render lineage in existing evidence/history fields; +understanding a row's change must not require reading a project-private document. ## 8. Agent Onboarding / Automation Enablement Machine diff --git a/examples/capability-extension-placement-doc-smoke.py b/examples/capability-extension-placement-doc-smoke.py index dfb7ad490e..35e2661da5 100644 --- a/examples/capability-extension-placement-doc-smoke.py +++ b/examples/capability-extension-placement-doc-smoke.py @@ -10,17 +10,17 @@ for anchor in ( "## Capability And Extension Placement", - "loopx/capabilities//", - "loopx/extensions/", - "packages//", + "docs/reference/extensions.md#placement-decision-for-agents", "capability id, provider id", - "name public capabilities after caller outcomes", + "Name public capabilities after caller outcomes", ): assert anchor in AGENTS, anchor for anchor in ( "## Placement Decision For Agents", "## Repository Layout", + "loopx/capabilities//", + "loopx/extensions/", "packages//", "There is intentionally no", "What user outcome and caller-visible contract is being added or changed?", diff --git a/tests/test_repository_instruction_delivery.py b/tests/test_repository_instruction_delivery.py new file mode 100644 index 0000000000..42dc8602a7 --- /dev/null +++ b/tests/test_repository_instruction_delivery.py @@ -0,0 +1,82 @@ +"""Check instruction delivery, not whether an agent obeys the delivered prose.""" + +from __future__ import annotations + +import re +from pathlib import Path +from urllib.parse import unquote, urlsplit + +import pytest + + +ROOT = Path(__file__).resolve().parents[1] +INSTRUCTIONS = ROOT / "AGENTS.md" +# The default Codex project-doc budget is bytes, shared by discovered files. +# This root must fit by itself; host overrides and ancestor files need their +# own delivery checks. Passing this test does not certify model comprehension. +DEFAULT_PROJECT_DOC_MAX_BYTES = 32 * 1024 + + +def _heading_anchors(path: Path) -> set[str]: + anchors: set[str] = set() + counts: dict[str, int] = {} + in_fence = False + for line in path.read_text(encoding="utf-8").splitlines(): + if line.startswith(("```", "~~~")): + in_fence = not in_fence + if in_fence: + continue + match = re.match(r"^#{1,6}\s+(.+?)\s*#*\s*$", line) + if not match: + continue + heading = re.sub(r"[^\w\- ]", "", match[1].lower()).replace(" ", "-") + suffix = counts.get(heading, 0) + counts[heading] = suffix + 1 + anchors.add(f"{heading}-{suffix}" if suffix else heading) + return anchors + + +def _local_links(path: Path) -> list[tuple[Path, str]]: + links = [] + for target in re.findall(r"\]\(([^\s)]+)\)", path.read_text(encoding="utf-8")): + parts = urlsplit(target) + if parts.scheme or parts.netloc or parts.path.startswith("/"): + continue + resolved = (path.parent / unquote(parts.path)).resolve() if parts.path else path + links.append((resolved, unquote(parts.fragment))) + return links + + +def test_root_instructions_fit_default_project_doc_loading() -> None: + contents = INSTRUCTIONS.read_bytes() + loaded = contents[:DEFAULT_PROJECT_DOC_MAX_BYTES] + assert loaded == contents, ( + f"Root instructions lose {len(contents) - len(loaded)} bytes at the " + "default loader boundary; route applicable detail to its existing owner." + ) + + +def test_root_instruction_routes_resolve_to_existing_files_and_sections() -> None: + links = _local_links(INSTRUCTIONS) + assert links, "Root instructions need reachable task-specific procedures." + for target, anchor in links: + assert target.is_file(), f"Missing instruction route: {target.relative_to(ROOT)}" + if anchor: + assert anchor in _heading_anchors(target), ( + f"Missing instruction section: {target.relative_to(ROOT)}#{anchor}" + ) + + +@pytest.mark.parametrize( + "consumer", + [ + "loopx/capabilities/pr_review_queue/README.md", + "docs/development/design.md", + "docs/development/frontend-delivery.md", + ], +) +def test_existing_consumers_can_follow_root_instruction_anchors(consumer: str) -> None: + anchors = [anchor for target, anchor in _local_links(ROOT / consumer) if target == INSTRUCTIONS] + assert anchors, f"{consumer} must retain its repository-policy route." + for anchor in anchors: + assert anchor in _heading_anchors(INSTRUCTIONS), f"Broken root-policy anchor: {anchor}" From b4681c31caa44c40617f30598089fbda2e05180a Mon Sep 17 00:00:00 2001 From: song Date: Sat, 17 Oct 2026 16:30:48 +0800 Subject: [PATCH 2/5] refactor(skills): route project procedures through packaged references Signed-off-by: song --- examples/bootstrap-command-pack-smoke.py | 13 +- ...-lifecycle-command-modularization-smoke.py | 6 +- ...ore-executive-projection-contract-smoke.py | 5 +- examples/pr-review-command-smoke.py | 10 +- pyproject.toml | 8 + skills/loopx-pr-review/SKILL.md | 79 +- skills/loopx-pr-review/agents/openai.yaml | 2 +- skills/loopx-project/SKILL.md | 1168 ++--------------- .../references/autonomous-execution.md | 307 +++++ .../references/handoff-and-feedback.md | 152 +++ .../references/heartbeat-hosts.md | 199 +++ .../references/installation-and-diagnosis.md | 149 +++ .../references/materials-explore-quality.md | 94 ++ .../references/start-and-connect.md | 217 +++ tests/test_packaged_skill_metadata.py | 24 + tests/test_workflow_skill_install.py | 32 + 16 files changed, 1357 insertions(+), 1108 deletions(-) create mode 100644 skills/loopx-project/references/autonomous-execution.md create mode 100644 skills/loopx-project/references/handoff-and-feedback.md create mode 100644 skills/loopx-project/references/heartbeat-hosts.md create mode 100644 skills/loopx-project/references/installation-and-diagnosis.md create mode 100644 skills/loopx-project/references/materials-explore-quality.md create mode 100644 skills/loopx-project/references/start-and-connect.md diff --git a/examples/bootstrap-command-pack-smoke.py b/examples/bootstrap-command-pack-smoke.py index 8b407d93d4..520fe53f08 100644 --- a/examples/bootstrap-command-pack-smoke.py +++ b/examples/bootstrap-command-pack-smoke.py @@ -904,26 +904,31 @@ def test_skill_slash_fallback_contract() -> None: pr_review_skill_text = (REPO_ROOT / "skills" / "loopx-pr-review" / "SKILL.md").read_text( encoding="utf-8" ) + start_path = REPO_ROOT / "skills/loopx-project/references/start-and-connect.md" + assert "before its operation" in " ".join(skill_text.split()) + assert "references/start-and-connect.md" in skill_text + start_text = start_path.read_text(encoding="utf-8") + start_normalized = " ".join(start_text.split()) normalized = " ".join(skill_text.split()) pr_review_normalized = " ".join(pr_review_skill_text.split()) assert "## Slash Command Fallback" in skill_text assert "`/loopx`" in skill_text assert "`/loopx `" in skill_text - assert "loopx bootstrap-command-pack --project ." in skill_text + assert "loopx bootstrap-command-pack --project ." in start_text assert "loopx start-goal --guided --project ." in skill_text - assert "canonical_project_alias" in skill_text + assert "canonical_project_alias" in start_text assert ( "`start-goal --project` keeps the requested project route" in skill_text ) - assert "Lower-level diagnostic command packs" in normalized + assert "Lower-level diagnostic command packs" in start_normalized assert '--goal-text ""' in skill_text assert "bare `/loopx` read-only command" in skill_text assert "explicit goal-start intent" in normalized assert "planner order plus `todo add` write order" in normalized assert "do not silently downgrade `/loopx `" in normalized assert "`/loopx-global-summary`" in skill_text - assert "Legacy `/loop-global-*` forms" in normalized + assert "Legacy `/loop-global-*` forms" in start_normalized assert "loopx slash-commands" in skill_text assert "not project bootstrap commands" in normalized assert "`/loopx-pr-review`" in skill_text diff --git a/examples/cli-project-lifecycle-command-modularization-smoke.py b/examples/cli-project-lifecycle-command-modularization-smoke.py index 1a005765e0..54ecbfab35 100644 --- a/examples/cli-project-lifecycle-command-modularization-smoke.py +++ b/examples/cli-project-lifecycle-command-modularization-smoke.py @@ -168,8 +168,10 @@ def main() -> None: module_source = MODULE.read_text(encoding="utf-8") init_source = INIT.read_text(encoding="utf-8") project_skill = PROJECT_SKILL.read_text(encoding="utf-8") - reward_skill = project_skill.split("## Record Human Reward", 1)[1].split( - "## Multi-Project Status", 1 + require("references/handoff-and-feedback.md" in project_skill, "reward procedure must be routed from skill") + handoff = PROJECT_SKILL.parent / "references/handoff-and-feedback.md" + reward_skill = handoff.read_text(encoding="utf-8").split("## Record Human Reward", 1)[1].split( + "## Capability Context And Child Models", 1 )[0] for marker in ( diff --git a/examples/explore-executive-projection-contract-smoke.py b/examples/explore-executive-projection-contract-smoke.py index e1f9d983e7..be6849268b 100644 --- a/examples/explore-executive-projection-contract-smoke.py +++ b/examples/explore-executive-projection-contract-smoke.py @@ -8,7 +8,7 @@ ROOT = Path(__file__).resolve().parents[1] CONTRACT_PATHS = ( - ROOT / "skills" / "loopx-project" / "SKILL.md", + ROOT / "skills" / "loopx-project" / "references" / "materials-explore-quality.md", ROOT / "loopx" / "capabilities" / "explore" / "README.md", ) @@ -28,6 +28,9 @@ def main() -> int: + skill = (ROOT / "skills/loopx-project/SKILL.md").read_text(encoding="utf-8") + assert "references/materials-explore-quality.md" in skill + assert "Read the matching procedure" in skill for path in CONTRACT_PATHS: text = path.read_text(encoding="utf-8") normalized = " ".join(text.lower().split()) diff --git a/examples/pr-review-command-smoke.py b/examples/pr-review-command-smoke.py index 7fc1318c05..3054b781b1 100644 --- a/examples/pr-review-command-smoke.py +++ b/examples/pr-review-command-smoke.py @@ -110,6 +110,13 @@ def main() -> int: "详细中文评审", "英文简短结论", "complete Chinese five-block review plus one concise English verdict", + "review_execution_contract.verdict_policy", + "validation_matrix", + "validation_source", + "failure_attribution", + "durable_smoke_value", + "null action does not imply an existing conclusion", + "Reserve Draft for unfinished author work", "review_execution_contract.policy_revision", "review_policy_revision", "Do not retain expired temporary worktree overrides", @@ -1320,7 +1327,8 @@ def approved_open_head( assert "## Combined Review Sequence" in markdown, markdown assert markdown.index("## Unmerged PRs") < markdown.index("## Merged PRs"), markdown assert "risk_hint=`medium`" in markdown, markdown - assert "template below is intentionally blank" in markdown, markdown + assert "establish the goal and specification first" in markdown, markdown + assert "then execute this exact-head review_plan and its evidence commands" in markdown, markdown assert "- 推荐阅读顺序:" in markdown, markdown assert "- 五块模板（留空给 agentloop 填写）:" in markdown, markdown assert "动机（至少 " in markdown, markdown diff --git a/pyproject.toml b/pyproject.toml index 7642a85eec..dbe072b74b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -122,6 +122,14 @@ include = ["loopx*"] "skills/loopx-project/SKILL.md", "skills/loopx-project/.loopx-skill-scope", ] +"share/loopx/skills/loopx-project/references" = [ + "skills/loopx-project/references/start-and-connect.md", + "skills/loopx-project/references/installation-and-diagnosis.md", + "skills/loopx-project/references/autonomous-execution.md", + "skills/loopx-project/references/heartbeat-hosts.md", + "skills/loopx-project/references/handoff-and-feedback.md", + "skills/loopx-project/references/materials-explore-quality.md", +] "share/loopx/skills/loopx-project/agents" = [ "skills/loopx-project/agents/openai.yaml", ] diff --git a/skills/loopx-pr-review/SKILL.md b/skills/loopx-pr-review/SKILL.md index 0fb2679540..75f6508891 100644 --- a/skills/loopx-pr-review/SKILL.md +++ b/skills/loopx-pr-review/SKILL.md @@ -64,7 +64,7 @@ names the contract the change is judged against. ## Execute One Review Plan Receive pending current-session requests through `review_execution_contract.decision_procedure.establish_goal` before generic queue selection, including other agents sharing a GitHub account; task intake and claimed action authority are separate judgments. Resolve the selected requests with repeatable `--target-exact-head NUMBER@HEAD_OID`. Follow `scheduling_policy` and its ranked actionable `review_sequence`; explicit current-request PR selection may override ordering only, never `pull_requests[].review_action_kind` or exact-head idempotency. Generic `re-review`, `重新review`, and `复审` wording selects the named PR; it is not a force-refresh token. Todo/monitor prose may not select work. -When `review_action_kind` is null, the row stays in `pull_requests` inventory but must not appear in `review_sequence`; its `review_plan` and `review_template` are null and `evidence_commands` is empty. Read back the row's lifecycle/draft state and exact-head conclusion once; report the actual reason rather than treating every null action as an already-concluded review. If the caller is also the authorized author and its proposal is complete, follow [author submission](../../AGENTS.md#author-submission-and-review-entry): mark Draft ready and read back the unchanged head, then regenerate the packet. Missing review or held CI/merge qualification does not require Draft. A reviewer alone has no authority to change another author's submission state or invent a review action. Run a fresh audit only when the user explicitly requests fresh evidence despite an existing conclusion, or supplies a concrete new concern; regenerate with `--fresh-audit-exact-head NUMBER@HEAD_OID`, then execute the complete current plan and never inherit the earlier approval. Fresh audit is not a Draft bypass. For every actionable PR: +When `review_action_kind` is null, the row stays in `pull_requests` inventory but must not appear in `review_sequence`; its `review_plan` and `review_template` are null and `evidence_commands` is empty. Read the packet's actual no-action reason and read back the relevant lifecycle/draft state or conclusion once; null action does not imply an existing conclusion. If the caller is also the authorized author and its proposal is complete, mark Draft ready and read back its state and unchanged head, then regenerate the packet. Reserve Draft for unfinished author work with a concrete remaining implementation or integration step. Missing review or held CI/merge qualification does not require Draft. A reviewer alone has no authority to change another author's submission state or invent a review action. Run a fresh audit only when the user explicitly requests fresh evidence despite an existing conclusion, or supplies a concrete new concern; regenerate with `--fresh-audit-exact-head NUMBER@HEAD_OID`, then execute the complete current plan and never inherit the earlier approval. Fresh audit is not a Draft bypass. For every actionable PR: 1. Record the packet's exact head, then follow `review_execution_contract.decision_procedure`: the current goal and `problem_context` delivery judgment first, including on re-review, @@ -90,20 +90,24 @@ When `review_action_kind` is null, the row stays in `pull_requests` inventory bu Each PR needs independent evidence and a standalone card; a queue table is a preface only. -For managed review, pass `--goal-id GOAL` and follow the packet’s resolved `wait_for_ci`: false means never fetch, poll, or wait for CI; true retains available CI observation and the merge gate's CI policy. It does not require every CI job to finish or succeed before `APPROVE` when independent current evidence covers the changed invariants. Follow `validation_matrix.validation_source`: `required` marks decisive review evidence, not branch protection; record merely pending remote jobs as separate diagnostic rows, and keep an invariant unverified when CI is its only decisive coverage. Pending CI alone is never a `REQUEST_CHANGES` reason. Apply `validation_matrix.failure_attribution`, including its `evidence_scope`, to current required checks. An independently attributed unchanged baseline failure or external outage can hold merge readiness without forcing `REQUEST_CHANGES` on an unrelated PR. Preserve earlier failures and the current evidence that supersedes them in existing evidence fields; historical root-cause completeness alone is not an approval gate. Current unattributed failures, material instability and missing affected-invariant coverage still block; selecting one successful rerun does not resolve them. Configure one Goal with `configure-goal --goal-id GOAL --no-pr-review-wait-for-ci --execute`; clear with `--clear-pr-review-configuration --execute`. +For managed review, pass `--goal-id GOAL` and execute the packet's current +`validation_matrix`: its `wait_for_ci`, `validation_source` and +`failure_attribution` (including `evidence_scope`) own CI observation and failure +judgment. With `wait_for_ci=false`, never fetch, poll or wait for CI. Keep review +verdict and merge readiness separate; do not substitute CI metadata for decisive +changed-invariant evidence. Configure one Goal with `configure-goal --goal-id GOAL +--no-pr-review-wait-for-ci --execute`; clear with +`--clear-pr-review-configuration --execute`. ## Publish And Read Back For an open PR, publish validated actionable findings by default unless the user requested local-only/dry-run output or the finding is private or security-sensitive. -- Remaining blocker: formal `REQUEST_CHANGES`; for an author-owned PR, use a - `COMMENTED` review titled `Request changes conclusion (author-owned PR; GitHub blocks formal self-review)`. -- Non-blocking finding with no blockers: formal `APPROVE`, not a bare comment; - when the account is the author and self-approval is rejected, record the same - conclusion as a `COMMENTED` review titled `Approval conclusion (author-owned PR; GitHub blocks formal self-approval)`. -- Non-blocking P2 suggestions: still `APPROVE`; keep them in the body. -- Merged PR: publish a post-merge audit comment only for a new actionable finding; - avoid duplicating an equivalent exact-head result. +Use `review_execution_contract.verdict_policy` for formal `REQUEST_CHANGES`, +`APPROVE`, author-owned `COMMENTED` fallbacks and post-merge audit publication. +Publish the complete checked body using the policy's action and fallback title; +non-blocking suggestions and merge holds are not automatically review blockers. +Avoid duplicating an equivalent exact-head result. Build public text from the exact reviewed head; remove local paths, private context, raw logs, credentials and internal-only links. Read the published review back, verify state/body @@ -119,46 +123,21 @@ bypass never overrides this gate, and author fallback needs user authority. ## Full PR Review And Bilingual Format -Every review must cover the whole PR, not only the top finding: read the full diff/local -validation, then explain motivation, architecture, changed symbols, both paths, whole-diff -risk, validation, and judgment. A findings-only or blocker-only body is incomplete. - -Publish two artifacts: - -1. **详细中文评审** - a standalone Chinese full-PR review with the exact head and five - sections: `动机`, `改动思路`, `具体改动`, `对主干的风险`, `我的整体评价`. Cover every - changed surface and key symbols, not only the main finding. -2. **审查者来源** - one visible `body_marker` line naming the actor kind and, for an - agent, the model and provider. It must agree with `result.reviewer`; a reviewer - from another operator reads this body, not your local result. -3. **英文简短结论** - a line starting with exactly `English verdict:` followed by the bare - token `APPROVE` or `REQUEST_CHANGES`, then the head, key finding, and validation. The - parser matches that token as a keyword, so `English verdict: Approved at ...` is invalid - and leaves the head without a merge-ready approval; write `English verdict: APPROVE - ...`. - -Do not publish before the Chinese section covers the entire PR. Follow the packet's `problem_context` publication rule for an understandable opening. Read both artifacts back. - -## Example / Walkthrough / Smoke-Only PRs - -When the review plan marks `smoke_or_example_only`, the `durable_smoke_value` -evidence is mandatory before approval. The essence is real, durable value to -the repository and product: running, deterministic, and public-safe are -necessary but not enough. - -1. Name the shipped behavior, boundary, or maintenance cost this artifact - guards. "Demonstrates something that already works" is not durable value. -2. Scan existing coverage (`rg -l '' examples tests`) and the - same-author batch (`gh pr list ... --author ` / `gh search prs`); - flag same-shape batches opened within minutes as PR farming. -3. Apply the repo smoke policy: thin + durable, guard shipped behavior or a - real boundary, compress rather than append, consolidate same-shape - walkthroughs into one PR or focused tests. -4. Verdict: `REQUEST_CHANGES` for duplicative, oversized, or value-less - scaffolding; name the consolidation or thinning repair in the body. -5. Repeat offenders: after a REQUEST_CHANGES warning, further low-value - same-shape PRs from the same author escalate to a contribution-restriction - recommendation (owner blocks the account from further PR submissions); the - warning must name this consequence. +Use each PR's current `review_template`, `reviewer_declaration` and +`completion_gate.required_final_sections` to render a standalone full-PR review. +The **详细中文评审** covers the entire PR in five sections (`动机`, `改动思路`, +`具体改动`, `对主干的风险`, `我的整体评价`); a findings-only or blocker-only body is +incomplete. Cover every changed surface and key symbols according to the current +plan, not just the main finding. Open with the visible `body_marker` matching +`result.reviewer`. The **英文简短结论** starts with `English verdict:` and the bare +token `APPROVE` or `REQUEST_CHANGES`, followed by the head, finding and validation. +Follow the packet's `problem_context` publication rule and body floors; verify the +published body through the same check-result/readback loop above. + +For example, walkthrough or smoke-only PRs, execute the current plan's required +`durable_smoke_value` evidence: its fields and rule own the shipped-value, +existing-coverage, same-author batch, consolidation and repeat-offender judgment. +Do not approve from a passing smoke or a public-safe artifact alone. ## Autonomous Queue diff --git a/skills/loopx-pr-review/agents/openai.yaml b/skills/loopx-pr-review/agents/openai.yaml index 285fd559c0..8dabb96d17 100644 --- a/skills/loopx-pr-review/agents/openai.yaml +++ b/skills/loopx-pr-review/agents/openai.yaml @@ -1,4 +1,4 @@ interface: display_name: "LoopX PR Review" short_description: "Review LoopX PRs with deep key-code explanations" - default_prompt: "Use $loopx-pr-review to interpret the entire PR: read every changed file, explain key symbols and control/data flow, walk positive and negative paths, cover all surfaces in the Chinese five-block review, and append one concise English verdict." + default_prompt: "Use $loopx-pr-review to obtain and execute the current capability-owned exact-head review packet, publish the checked complete Chinese five-block review plus English verdict, and read back the published result." diff --git a/skills/loopx-project/SKILL.md b/skills/loopx-project/SKILL.md index c5099c9e91..8ff2b7b097 100644 --- a/skills/loopx-project/SKILL.md +++ b/skills/loopx-project/SKILL.md @@ -23,27 +23,36 @@ load `loopx-doc-registry` and use that narrower workflow first. LoopX has two layers: -- **Project-local state**: each repo owns `.loopx/registry.json` and - `.codex/goals//ACTIVE_GOAL_STATE.md`. -- **Shared local control plane**: `~/.codex/loopx` stores run history and +- **Project-local state**: each repo owns `.loopx/registry.json`; new Goal state + defaults to `.loopx/goals//ACTIVE_GOAL_STATE.md`. +- **Shared local control plane**: the default `~/.loopx` stores run history and `registry.global.json` for multi-project status. +Use the registry-declared `state_file` and the resolved runtime/global-registry +route for existing installations. `.codex/goals` and `~/.codex/loopx` are legacy +compatibility paths, not defaults to recreate or migrate implicitly. If routes +conflict, diagnose the existing state before selecting or migrating one. + Do not manually copy one project's registry entry into another project. Local `connect` and `refresh-state` should sync into the shared global registry automatically. -## Slash Command Fallback +## Read The Relevant Procedure Before Acting + +The following resources ship with this skill. Resolve their links relative to +this file, including after host installation. Read the matching procedure +before its operation; ordinary admitted work does not load every procedure. -When asked to upgrade LoopX on a machine with the desktop App, inspect the App -bundle and its runtime as well as the CLI. `loopx doctor` exposes -`desktop_installation` for standard macOS install locations; a paired bundle -still does not prove which App process is running. Upgrade the App and its -bundled runtime together, then restart and verify the App, CLI and service -source revisions. Never claim a desktop upgrade from CLI/HTTP checks alone: -current App builds ask before replacing a different CLI runtime (update the -App, or align the CLI to the App's bundled runtime), while an older build can -still replace a separately upgraded CLI with its bundle. -Keep SSH host verification separate; a host without an App needs no App install. +| Operation | Required procedure | +| --- | --- | +| `/loopx` goal-start, connect, identity binding or scope changes | [Start and connect](references/start-and-connect.md) | +| Missing CLI, desktop upgrade, runtime drift, diagnosis or registry recovery | [Installation and diagnosis](references/installation-and-diagnosis.md) | +| Set up or repair autonomous execution, missing/contradictory contract, state-only delivery writeback | [Autonomous execution and settlement](references/autonomous-execution.md) | +| Create, recover or change a recurring heartbeat or host loop | [Heartbeat hosts](references/heartbeat-hosts.md) | +| Relay a review packet, record a human reward or run separately authorized child work | [Handoff and feedback](references/handoff-and-feedback.md) | +| Register material, change an owner-facing Explore view or qualify an enabled change-quality diff | [Materials, Explore and quality](references/materials-explore-quality.md) | + +## Slash Command Fallback When the visible user message is exactly a LoopX slash command or starts with a LoopX slash command plus arguments, do not treat it as ordinary chat. @@ -53,11 +62,6 @@ Recognized project-local goal-start command: - `/loopx ` - `/loopx --capability-route issue-fix ` -Recognized repo-review commands: - -- `/loopx-pr-review` -- `/loopx-pr-review