Conversation
Terminal captures proved the feature ran but did not explain it: a new
reader had to already understand SDD and RDD to parse the output. Swap
both for hand-authored SVG diagrams that carry the concept instead.
Each diagram is a self-contained dark card with its own background, so a
single file reads correctly under both GitHub themes without the
`<picture>` + prefers-color-scheme dance, which follows the OS setting
rather than the GitHub theme toggle. No CSS or <style> blocks, only
presentation attributes, so GitHub's SVG sanitizer cannot break them.
12K each, versionable and diffable, against 532K of PNG.
Shape follows the concept: SDD is a sequence, drawn as three bands
(Understand, Plan, Build) showing the file each phase writes; RDD is a
branch, drawn as freeze, assess, then one of three depths by risk tier.
Both paragraphs were rewritten to complement the diagram instead of
repeating it. The SDD phase list and the RDD tier list are now shown, so
the prose carries the reasoning the picture cannot: why TDD lives in
Apply, why Verify is a separate step, and why freezing the candidate is
what keeps a review honest.
docs/assets/features/{sdd,rdd}.png are now unreferenced and can be
removed in a follow-up.
Completes the swap started for SDD and RDD. The capture showed a session
recalling a decision, but a reader who did not already know what Engram
was could not tell what they were looking at.
The concept here is time, so the shape is a timeline rather than the
sequence used for SDD or the branch used for RDD: three sessions cut by a
restart and by compaction, over one memory layer the cuts stop at. That
the break lines end at the memory card is the whole argument, drawn.
The paragraph now carries what the picture cannot — that the real cost of
a fresh session is re-explaining yesterday's decisions, not tokens.
Also corrects the hero-slot comment, which still claimed all six Features
images were stills. Features is now three diagrams and three captures.
docs/assets/features/engram.png joins {sdd,rdd}.png as unreferenced.
Completes the set. The capture showed an agent explaining determinism in prose, which asks the reader to trust a claim rather than see why it holds. Fourth concept, fourth shape: convergence. A different agent, a different model and a brand-new session all funnel into one engine, and one answer comes out. Drawing the funnel is the argument — the inputs vary, the output does not. The four public states (Working, Checking, Ready, Needs your decision) are shown explicitly, so "one valid next step" stops being abstract. The paragraph now carries the consequence rather than the mechanism: a guessed next step differs tomorrow and differs again for a teammate, which is the gap between a workflow and a suggestion. Hero-slot comment updated again: Features is now four diagrams and two captures.
sdd, rdd, engram and deterministic are superseded and unreferenced anywhere in the repo. 1.0M removed; the four SVGs that replaced them total 48K. features/ now holds only the two captures still doing a job: agents and gentle-shell, which prove the thing runs rather than explain a concept. brand/gentle-ai-logo.png (1.1M) is also unreferenced but is deliberately left in place — a brand logo is the kind of asset that gets hot-linked by raw URL from outside the repo, so removing it is not a safe cleanup decision to make silently.
The About-the-author section described consulting engagements but linked nowhere, so the paragraph made a claim the reader could not follow up on. Ports docs/consulting.md from docs/readme-redesign, where it was written for the same landing-page work, and links it from that paragraph. The content carries no version-dependent claims, so it ports verbatim; all five of its links are external and resolve. Does not port docs/install.md from the same branch. It duplicates docs/quickstart.md, which already covers prerequisites, version policy, the install commands, verification and hardening, and which this README already routes install traffic to. It also still names v2.6.0 as current stable when v2.7.0 shipped on 2026-09-08 — the defect tracked in Gentleman-Programming#4428. Adding a third install page would widen that surface rather than fix it.
…ing/fix/4372-note-not-blocker fix(sdd): keep informational edit-root notes out of blockedReasons
…g negotiated evidence (Gentleman-Programming#4494) The consent answer re-enters negotiated START, which rebuilt the live snapshot and refused a moved candidate with an opaque identity mismatch whose recovery advice re-derives into an unbounded loop while a concurrent writer advances (Gentleman-Programming#4494). Negotiated continuations now carry the five identity components beside the hash (--target-evidence), so the failing process decomposes the mismatch: a moved candidate names the differing components in context.target_drift and waits for the writer, while an all-components-equal drift is a derivation defect that stops instead of retrying. Without the token the shipped refusal bytes stay unchanged. Focused: go test ./internal/cli -run 'TestReviewTargetEvidence|TestReviewConsentAnswerNamesMovedCandidate|TestReviewStaleWithoutTargetEvidence|TestReviewStartRejects|TestReviewNegotiatedStartCommand|TestReviewConsentStaleMarker|TestReviewNegotiatedStaleIdentityOnly|TestConsentQuestionMatches|TestReviewProviderArtifact|TestEveryProductionRefusal' -count=1 -> ok Runtime harness: N/A (CLI contract behavior, covered by the envelope repro test against a live temp repository) Rollback: revert this commit; no other unit depends on the token.
…transitions (Gentleman-Programming#4494) The selectorless STATUS preflight renders the exact negotiated START the caller must run; without the evidence token that entry point produced negotiations whose later answers could not decompose a stale target. fresh_target_ready now carries --target-evidence beside --target, but only when the components recomputed from the published projection reproduce the exact target identity, so a transition whose target is not the live snapshot never carries contradicting evidence. Focused: go test ./internal/cli -run 'TestStatusStartTransition|TestNegotiatedStatusStartReplay|TestNegotiatedV2FreshStatus' -count=1 -> ok Runtime harness: N/A (transition rendering, asserted on live STATUS output) Rollback: revert this commit; START keeps working without the token.
…t a moving candidate (Gentleman-Programming#4494) With a live writer, every new candidate was a new stop-hook reminder, each instructing the agent to run the preflight again and pushing it back into the same unexecutable negotiation (Gentleman-Programming#4494). When a consent answer was spent on a moved candidate (recorded by START in the per-repository consent-stale marker), the hook stays quiet while the candidate keeps changing between Stop events and resumes the ordinary reminder as soon as the candidate is stable across two Stops. The window bounds the silence, and an expired, absent, or corrupt marker always restores today's reminder. Focused: go test ./internal/cli -run 'TestReviewStopHook' -count=1 -> ok Runtime harness: N/A (hook behavior, driven through stdin payloads) Rollback: revert this commit; the marker record simply goes unread.
…ing/fix/4494-self-describing-continuation fix(review): name the truthful stale-target cause from self-describing negotiated evidence (Gentleman-Programming#4494)
…andidate A selectorless negotiated STATUS on a clean worktree whose reviewed work is already committed froze zero paths and answered with the unroutable `external.select_base_ref` collect. Nothing could submit that selection -- `transition_input.submission` is a closed oneOf -- and retrying START with the collect's own target loops forever on `stale_target_identity` (Gentleman-Programming#4412). STATUS now resolves the remote default branch's unique merge-base and re-publishes the executable committed-range START the working `--base-ref <rev> --committed-only` route already produces, disclosing the resolved base commit in `--base-ref` so the caller can see and override the scope it is offered. The old collect stays the fallback for every repository shape the derivation cannot resolve: no origin/HEAD, a criss-cross history with several merge bases, an empty committed range, or a derived range whose trees still coincide.
…ing/fix/4412-derived-committed-range-start fix(review): offer the committed-range START for an empty workspace candidate
…man-Programming#4505) * fix(sdd): separate status inspection from consent preparation Keep native inspection read-only, prepare consent only on explicit continuation, and bind grants to the current change instance. Reconcile inherited recovery selections without weakening fresh admission. Include the three accepted AI work units, paired tests/assets/fixture migrations, and their OpenSpec planning record as one verified producer-side snapshot. Pi integration remains pending; this is not cross-repository completion. * fix(sdd): align deployed status and continuation guidance Use native v2 for every declared artifact store and separate pure status from explicitly authorized continuation across deployed adapters. Keep Claude lazy workflow references, compatible renderer sentinels, materialized configuration invariants, and the twelve matching goldens together. Verified with fresh focused and full uncached Go tests, vet, formatting, and diff checks. All 52 added named records and twelve golden cases pass. Native advisory review approved and acknowledged; attempt remediation settled complete. Real-host loading, Pi execution, driven bench, and installed-tuple acceptance remain outside this proof. Reverting this commit restores the prior adapter guidance and matching test/golden baseline without reverting the native status/continuation work in 3160153.
…ing/fix/3508-pi-legacy-cleanup fix(pi): retire legacy prompt blocks across lifecycle
…ing/fix/4513-research-ownership fix(sdd): make research persistence orchestrator-owned
…ing/fix/4513-adapter-parity fix(sdd): align research ownership across adapters
…adme-structure docs(readme): redesign as a landing page with concept diagrams
…/feat/engram-session-id-propagation fix(engram): use authoritative runtime session identity
bindRenderedBody required the event payload body and a freshly fetched body to be byte-identical. This repository's CodeRabbit configuration appends its auto-generated summary to every pull request description, so the two can never match: the comparison always threw, the step's catch reported a retrieval failure, and retrying could not help because a retry reuses the original event payload. Check Issue Reference is a required status check, so that code would have made it impossible for any pull request in this repository to pass. Parse the event body directly with the local comment stripping, which the parser already applies, and drop the fetch, bindRenderedBody and its tests. The source-level guard now asserts that the workflow parses the event body and does not fetch the pull request. Refs Gentleman-Programming#3352
…bers Two review findings on the reference parser. The keyword patterns required whitespace after the keyword, so a colon form such as `Closes: Gentleman-Programming#10` was ignored rather than treated as a reference. Whether GitHub also auto-closes on that form is unconfirmed, so accepting it is the safe direction: the colon form now names a reference that must point at an approved issue instead of being invisible to the gate. A colon with no whitespace, `Closes:Gentleman-Programming#10`, still fails closed as malformed. parseInt turned an oversized digit run into an unsafe number, so `99999999999999999999` reached the approval job as a plausible issue number. Numbers that are not safe positive integers now fail closed with their raw text and a reason, like every other malformed case. This also rejects `#0`, which previously parsed as issue zero. Refs Gentleman-Programming#3352
Windows Full Suite has been red on five consecutive main heads. All three failures are test-only; production behavior on Windows is unaffected. internal/components/opencodeplugin: plugin_test.go asserted that a restored plugin source keeps mode 0600. Go's os package does not implement Unix permission bits on Windows, so every regular file reports 0666 there and the assertion could never hold. assertRestoredSourceMode keeps the hardcoded 0600 check off Windows and compares against the mode recorded before the install on Windows, so a restore that changes the mode still fails there. The byte-exact restore assertions are untouched and still run on every platform. internal/opencode: catalog_process_windows_test.go builds a helper .exe inside t.TempDir() and leaves a grandchild running, so the implicit RemoveAll hits "Access is denied" on a still-mapped image. The generated helper now records its grandchild PID, and a t.Cleanup registered after t.TempDir() (so LIFO runs it first) terminates and reaps that process before the removal. The lifecycle fix covers all three descendant-helper tests in the file, because all three share the same escaped-grandchild leak. No assertion was weakened. The pattern mirrors the descendant-pid reaping already in internal/components/engram/healthprobe_test.go. Verified: gofmt clean; go vet clean; go test green on darwin for both packages; GOOS=windows go vet and go test -c clean; deadcode-ratchet reports no new unreachable functions. A darwin pass does not prove Windows behavior, so the decisive proof is the windows-full-suite.yml run dispatched on this branch. Closes Gentleman-Programming#4843
…trict-non-closing-references feat(pr-check): support strict non-closing issue references
TestOpenCodeV2CatalogAndRegistry started a V2 plugin and then slept a fixed interval before reading the cache the plugin writes asynchronously, so on a loaded runner the read raced the write and readdir threw ENOENT. Unit Tests is a required status check, so the failure blocked whichever pull request happened to trigger it, including pull requests that do not touch Go code. Replace the three fixed waits with one bounded poll of the condition each one asserts: the cache entry exists, the catalog reports the initial variant, the catalog reports the refreshed one, and the second workspace has written its own entry. The timeout keeps each original message prefix and carries a five second deadline, so a genuine regression still fails instead of hanging. Closes Gentleman-Programming#4846
…test
The plugin stages its cache beside the destination and renames it into place:
const temporary = `${destination}.${randomBytes(6).toString("hex")}.tmp`
await writeFile(temporary, JSON.stringify(variants), { mode: 0o600 })
await rename(temporary, destination)
so while a write is in flight the directory holds both the published
`<key>.json` and the staged `.tmp`. The harness enumerated raw directory
entries, so the read could pick the staged file and the workspace count could
include it, which would satisfy an assertion on data the plugin has not
published yet.
Derive one filtered list of published `.json` entries and use it for the read
and for both count checks.
Refs Gentleman-Programming#4846
…-catalog-test-race test(assets): wait for the plugin cache instead of a fixed 30ms sleep
The collision guard compared full journey IDs only, so two journeys could share a numeric prefix (j44-foo and j44-bar) and both land, defeating the prefix's purpose as a unique handle in --only invocations and issue threads. Extract the check into journeyIDCollisions, which reports full-ID and numeric-prefix collisions naming both defining files and both IDs, and wire TestJourneyIDsAreUniqueAcrossSourceFiles through it. A unit test pins both messages against synthetic sources. The corpus passes the new guard as-is: the j44 prefix pair this issue was reported against no longer collides, because journeySources now filters retired atomic journeys and j44-corrected-current-changes-delivery is retired. No journey is renamed. Closes Gentleman-Programming#3406
…ourney-number-prefix-collision test(bench): reject duplicate journey number prefixes
Replace the static README banner with an animated GIF of the same artwork.
…ing/test/4843-windows-full-suite-portability test(windows): make four Windows-only test assertions platform-correct
…adme-banner-animation docs(readme): animate the banner
…s, and minimal permissions
…mant-size-gate ci(policy): install dormant base-controlled 400-line gate
Four adapter process tests carried a Windows skip whose stated reason is stale: "the helper process uses POSIX argument handling". The helpers do not go through a shell, they exec the test binary directly with exec.CommandContext and the package reads its arguments from the environment, so nothing in these tests depends on POSIX argument handling. Remove the four guards and the codex test file's now-unused runtime import. The remaining Windows skip in the claude test file is untouched: that one covers a POSIX shebang script and is load-bearing. Three further guards named by the issue no longer exist, because the Pi reviewer provider adapter and its tests were removed from main; the issue's scope is therefore four guards rather than seven. Refs Gentleman-Programming#3687
…eviewerprovider-windows-guards test(reviewerprovider): run the Windows-guarded adapter process tests
The journey launched OpenCode against a fresh HOME with no egress policy, so startup bootstrapped @opencode-ai/plugin through npm Arborist and refreshed models.dev over the live network. That fetch raced the thirty-second scheduling barrier, which then reported zero arrivals as if four-reviewer scheduling had serialised. Route external egress through the suite's loopback-denying proxy, which the sibling pinned-transport journey already uses, and switch the models.dev refresh off. Keep the thirty-second barrier and the four-reviewer assertions as they are. On expiry, cancel the subprocess, drain its output and report the barrier state, and phrase the failure as a stage-neutral stall rather than a scheduling verdict. Refs Gentleman-Programming#4121
…pencode-4r-egress test(e2e): isolate the OpenCode 4R journey from live npm egress
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughChangesTelemetry opt-out handling
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to Whitespace-padded telemetry opt-out values now consistently disable telemetry, with coverage for the updated behavior. No merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Alan-TheGentleman
force-pushed
the
main
branch
from
September 24, 2026 09:28
c5da5fd to
f182ea2
Compare
This branch has not been deployed
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
13 participants
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.
🔗 Linked Issue
Closes #4866
🏷️ PR Type
type:bug— Bug fix (non-breaking change that fixes an issue)📝 Summary
Trims leading and trailing whitespace when evaluating the
GENTLE_AI_TELEMETRYenvironment variable, ensuring that accidental surrounding spaces (e.g.export GENTLE_AI_TELEMETRY=" 0 ") do not silently bypass the opt-out.📂 Changes
internal/telemetry/killswitch.gogetenv("GENTLE_AI_TELEMETRY")withstrings.TrimSpaceinternal/telemetry/killswitch_test.goenv opt-out trims whitespaceinternal/cli/telemetry_test.gooptout with whitespace🤖 AI Assistance
Tool/model (if known): Gemini CLI / Antigravity
Material scope: Analysis of telemetry kill-switch behavior, adding test cases, and preparing PR.
Verification performed: Ran full Go test suite (
go test ./internal/telemetry/... ./internal/cli/...) locally.🧪 Test Plan
go test ./...)gentle-ai telemetry statuswithGENTLE_AI_TELEMETRY=" 0 ")