Repository navigation
Harness² ideas (1/2): prompt-rules — Facts head, deliverable guard, plain-text tool-call warning - #46
Merged
Merged
Conversation
…tool-call warning (Harness² prompt-rules) - Stable ## Facts block states the verifier command and any deliverables. - Optional loop.json deliverables + deliverableNudges (default 2): the harness re-prompts the worker before verify while files are missing/empty. - Safety rule 7: never write a tool call as plain text; the next iteration gets a warning section when the previous reply contained one.
Review (prompt-rules, should-fix): missingDeliverables joined repoRoot with the configured string, so an absolute path or a .. segment was stat'd as given and the nudge told the worker to create it outside the repo. - loop.json parse rejects absolute (POSIX/Windows) and escaping paths. - missingDeliverables never reports a path that is absolute or resolves outside repoRoot (defence in depth for callers that bypass the schema). - Tests for both.
The run-scope doctor test asserts the 'telegramNotify is enabled' warning, which only appears when Telegram env is unset. Under `doppler run` (how loops run verify.sh) AGENT_LOOP_TELEGRAM_* are set, so the test failed. The loop workers had worked around it by deleting the vars in a file-wide afterEach, leaving process env cleared for later tests (review should-fix, out of scope for the loops). Clear the three vars inside that test only and restore the caller's values in finally.
… CANCEL' as a detached HTTP/2 transport error stall-probes run 1 died with exit 1 in the JUDGE phase: @cursor/sdk 1.0.23's detached team-repo RPC (getTeamReposOrEmptyIfNotInTeam → checkRepoBlocked → shouldBlockRead) rejected with `[canceled] http/2 stream closed with error code CANCEL (0x8)`. The HTTP/2 unhandled-rejection guard exists for exactly this RPC but only matched NGHTTP2_* / ERR_HTTP2_STREAM_ERROR text, so it rethrew and Node exited before the judge-error classification (#45) could run. Match Connect-node's RST_STREAM wording in both the HTTP/2 guard pattern and the general transport pattern; tests use the observed error shape.
…mmand Review should-fix: the later Facts join left a single newline before ## Workspace (git) when no Facts block was present, and glued the block onto safety rule 6 (renderLoopPromptRulesSection has no trailing newline). - Join the optional stable-head block (## Facts) after the rules with exactly one blank line; without it the head is byte-identical to main. - Tests assert every heading starts at line start after a blank line. - Nits: ## Facts named in the stable-head comment; the verifier command is shown exactly as configured (not trimmed).
Owner
Author
Reviewey Ray sensor: #46 (Harness² 1/2, prompt-rules)RiskMEDIUM What could go wrong?
Review depth
VerdictADVISORY Advisory
Nits
|
dancingteeth
marked this pull request as ready for review
October 21, 2026 14:53
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dancingteeth
added a commit
that referenced
this pull request
Oct 21, 2026
…in, #46 entries - stallProbes: restore behaviour (branch/HEAD, ignore sources, parking on a failed restore) and its limits: ignored files are not snapshotted, and it is not safe with concurrent writers (one loop per worktree). - lessons: opt-in, quoted as data, informative lines. - Missing #46 entries: ## Facts block, plain-text tool-call warning + safety rule 7, HTTP/2 CANCEL crash fix.
dancingteeth
added a commit
that referenced
this pull request
Oct 21, 2026
* loops: freeze lessons-file, prompt-rules, stall-probes bundles (Harness² ideas) * loops: judge follows worker runtime (opencode default judge model) * loops: run harness2-ideas loops on the Cursor runtime * feat(loop): prompt rules — Facts head, deliverable guard, plain-text tool-call warning (Harness² prompt-rules) - Stable ## Facts block states the verifier command and any deliverables. - Optional loop.json deliverables + deliverableNudges (default 2): the harness re-prompts the worker before verify while files are missing/empty. - Safety rule 7: never write a tool call as plain text; the next iteration gets a warning section when the previous reply contained one. * fix(loop): route worker faults in deliverable nudges through one fault path * fix(loop): confine deliverable paths to the repo Review (prompt-rules, should-fix): missingDeliverables joined repoRoot with the configured string, so an absolute path or a .. segment was stat'd as given and the nudge told the worker to create it outside the repo. - loop.json parse rejects absolute (POSIX/Windows) and escaping paths. - missingDeliverables never reports a path that is absolute or resolves outside repoRoot (defence in depth for callers that bypass the schema). - Tests for both. * test(doctor): scope the Telegram env reset to the one test that needs it The run-scope doctor test asserts the 'telegramNotify is enabled' warning, which only appears when Telegram env is unset. Under `doppler run` (how loops run verify.sh) AGENT_LOOP_TELEGRAM_* are set, so the test failed. The loop workers had worked around it by deleting the vars in a file-wide afterEach, leaving process env cleared for later tests (review should-fix, out of scope for the loops). Clear the three vars inside that test only and restore the caller's values in finally. * fix(cursor): treat Connect-node 'http/2 stream closed with error code CANCEL' as a detached HTTP/2 transport error stall-probes run 1 died with exit 1 in the JUDGE phase: @cursor/sdk 1.0.23's detached team-repo RPC (getTeamReposOrEmptyIfNotInTeam → checkRepoBlocked → shouldBlockRead) rejected with `[canceled] http/2 stream closed with error code CANCEL (0x8)`. The HTTP/2 unhandled-rejection guard exists for exactly this RPC but only matched NGHTTP2_* / ERR_HTTP2_STREAM_ERROR text, so it rethrew and Node exited before the judge-error classification (#45) could run. Match Connect-node's RST_STREAM wording in both the HTTP/2 guard pattern and the general transport pattern; tests use the observed error shape. * fix(loop): own-line Facts heading in the stable head; exact verify command Review should-fix: the later Facts join left a single newline before ## Workspace (git) when no Facts block was present, and glued the block onto safety rule 6 (renderLoopPromptRulesSection has no trailing newline). - Join the optional stable-head block (## Facts) after the rules with exactly one blank line; without it the head is byte-identical to main. - Tests assert every heading starts at line start after a blank line. - Nits: ## Facts named in the stable-head comment; the verifier command is shown exactly as configured (not trimmed). * feat(loop): lessons.md from green runs (Harness² lessons-file) After a green verifier run the harness writes deduped verify gotchas to <loop>/lessons.md (capped at 6000 chars); the next run injects them into the stable prompt head before ## Workspace (git). lessons.md is frozen (workers cannot edit it). Opt out with "lessons": false. Includes the lessons review fixes (own-line lessons heading in the stable head; trim to the cap using the configured verify command rather than re-parsing a backtick span). * feat(loop): stall probes — two guided workers on the first hard stall (Harness² stall-probes) Opt-in stallProbes (default false). On the first hard stall at escalateAfterStagnation the harness runs two sequential probe workers (procedure hints vs repo facts), snapshots/restores the git workspace between them, keeps the probe whose verify goes green or moves the failure fingerprint, logs probe: { variant, kept }, then allows normal model escalation on later stalls (at most once per run). * fix(loop): stall probes — git-object workspace snapshots and probe-round review fixes Stall-probes loop review (grok-4.7, BLOCKERS) found data-loss bugs in the default WorkspacePort (reset --hard + untracked delete wiped staged files under the loop bundle; porcelain-quoted paths reverted/deleted; symlinks followed on restore; staged renames half-restored; 0755 → 0644; UTF-8 re-encoding of binary bytes; HEAD tripwire after the writes). createGitWorkspacePort is rebuilt on git plumbing instead of a file copy: - snapshot is read-only: HEAD + `git write-tree` of the real index + a worktree tree built with `git add -A` in a throwaway GIT_INDEX_FILE (respects .gitignore, stores symlinks as links, keeps modes and raw bytes, -z plumbing so no path quoting). No stash, no ref, no file writes. - restore diffs the current worktree tree against the snapshot (diff-tree -r -z --no-renames), deletes then writes only the changed paths, never writes through a symlink (lstat/unlink the link itself, parents must be real dirs), restores 0755/0644 and symlinks, never deletes under .cursor/loops/, then `git read-tree` restores the index exactly (staged adds/renames/deletions). - HEAD check runs before any write: a probe commit on top of the snapshot is undone with reset --soft (reflog keeps it); an unrelated HEAD throws. agentLoop probe round (review should-fix / must-fix): - finalVerify runs after a green probe (from the loop's round-2 fix). - default stagnationThreshold no longer stops the run in the probe round itself, so escalation can resume (from the loop's round-2 fix). - a throwing procedure probe restores s0 before the shared fault path; a throwing facts probe restores the procedure attempt and logs it. - a snapshot failure (e.g. unmerged index) skips probes for the run instead of ending it. - budget stop after the probe round; probes get the deliverable nudges; a kept green probe is logged once (with its review), not twice. Guard tests: quoted/unicode paths, symlink to a file outside the repo, staged rename, 0755 file, staged new file under .cursor/loops, probe commit / unrelated HEAD, read-only snapshot, procedure probe that throws, single log line for a green probe. stallProbes stays off by default. * fix(loop): stall-probe restore keeps ignored files and the branch; park with both errors on a failed restore Review of #47 (BLOCKERS): - Blocker (data-loss): restore decided what to delete under the probe's ignore rules, so a probe that edited or deleted .gitignore made .env / node_modules look probe-added and they were deleted. Restore now puts the snapshot's ignore sources back first (every .gitignore in the change set, plus $GIT_DIR/info/exclude, captured in the snapshot), recomputes the diff under those rules, and never deletes a path that git check-ignore --no-index matches (also covers a probe that force-added an ignored file to the index). - HEAD tripwire also checks the branch: the snapshot records the symbolic ref; a probe's git checkout -b / --orphan / --detach is switched back (HEAD repointed, worktree untouched; a deleted branch is recreated at the snapshot commit) before the commit check runs. - A restore that throws no longer escapes the probe round or hides the worker error: it is wrapped in an AggregateError (restore error + original probe/worker error, cause = original) and the run parks (status waiting, failure domain stall_probe_restore_failed). - Nits: a restored regular file keeps its own permission bits (0600 stays 0600; only the exec bit follows git); a file/symlink parent under a protected dir is never unlinked. - Guard tests from the repro (edit/delete .gitignore, force-add, info/exclude, nested .gitignore, checkout -b with and without commits, orphan, detached, deleted branch, modes, protected parent; all fail on the previous code) and agentLoop tests for parking on a failed restore. * fix(loop): lessons.md opt-in, quoted as data, informative failure lines Review of #47: - lessons defaults to false (opt-in) — no new file in consumers' loop bundles on every green run in a patch release. - Verifier output is stored, and injected into the prompt, as a fenced block under a notice that it is data, not instructions. The fence is longer than any backtick run in the output so it cannot be closed early; older unfenced files are re-quoted when read into the prompt. - Gotchas are the most informative lines of each failure overcome instead of the first output line (always '[verify] 0 — toolchain' for this repo's verifiers): FAIL / error / assertion lines, stderr first, tagged with the failing [verify] step; banner, progress and summary lines are skipped; ANSI stripped; at most 3 lines of 240 chars per failure; the 6000-char file cap still drops the oldest. Frozen lessons-file acceptance updated for the default change: its wiring tests now opt in with lessons: true, and it asserts the default is false; the pinned sha256 in verify.sh is updated to match. No other verify.sh change. * docs: CHANGELOG/README for #47 review — snapshot limits, lessons opt-in, #46 entries - stallProbes: restore behaviour (branch/HEAD, ignore sources, parking on a failed restore) and its limits: ignored files are not snapshotted, and it is not safe with concurrent writers (one loop per worktree). - lessons: opt-in, quoted as data, informative lines. - Missing #46 entries: ## Facts block, plain-text tool-call warning + safety rule 7, HTTP/2 CANCEL crash fix. * loops(lessons-file): GOAL.md — lessons defaults to false (opt-in) Per Paul in chat 2026-10-21 19:15 (review of #47). verify.sh does not pin GOAL.md, so no sha change. --------- Co-authored-by: dancingteeth <dancingteeth@users.noreply.github.com>
Merged
dancingteeth
added a commit
that referenced
this pull request
Oct 21, 2026
- package.json 0.8.3 (lockfile has no root version; unchanged). - Version labels kept in sync: README current line, CONSUMERS.md, docs/embed-api.md, llms.txt, site/ pages + site/llms.txt, and the site version test. - CHANGELOG: Unreleased -> 0.8.3 — 2026-10-21 with PR links (#44 #45 #46 #47); adds the parked-runs-exit-2 and deliverables-confined entries. Co-authored-by: dancingteeth <dancingteeth@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Harness² ideas, part 1 of 2: prompt-rules. This is split out so it can land on its own. Part 2 (lessons-file + stall-probes) is stacked on this branch.
What's in it
.cursor/loops/{lessons-file,prompt-rules,stall-probes}). The judge follows the worker runtime, and the loops run on the Cursor runtime.feat(loop)):## Factsblock in the stable prompt head, with the verifier command exactly as configured and the deliverables;deliverables/deliverableNudges(0–5, default 2). If deliverables are missing or empty, the harness re-prompts the worker before verify;handleWorkerFault): transient classification, escalation and parking, with no livelock.deliverablesmust be repo-relative. Absolute and..paths are rejected.## Facts/## Workspace (git)headings always start on their own line after a blank line. With no Facts, the head is byte-identical to main.doctortest: the Telegram env reset is scoped to the one test that needs it (the suite was failing under Doppler).http/2 stream closed with error code CANCELrejection is now treated as a detached HTTP/2 transport error, so the process no longer crashes on an unhandled rejection. This crashed a stall-probes loop run.Gates (clean committed tree, 4209773)
bash .cursor/loops/prompt-rules/verify.sh: exit 0, all steps 0–9, including step 8 (loopStagnationPolicy.ts/loopAgentEscalation.tsunchanged).pnpm build,pnpm typecheck: pass.pnpm test: 134 files, 1211 tests passed, plus the node test scripts.No version bump.
stallProbesandlessonsaren't in this PR.