feat: /toggle-review command for pi-hard-no runtime on/off#119
Open
royosherove wants to merge 3 commits into
Open
feat: /toggle-review command for pi-hard-no runtime on/off#119royosherove wants to merge 3 commits into
royosherove wants to merge 3 commits into
Conversation
added 3 commits
May 11, 2026 14:20
New slash command (alias: /toggle-code-review) that flips ~/.pi/.hardno/settings.json `enabled` boolean. Takes effect on the NEXT agent turn \u2014 no session restart needed \u2014 because pi-hard-no v1.3.0+ re-reads that field at each agent_end. Implementation: - src/gateway/code-review-toggle.ts * resolveSettingsPath() / readEnabled() / toggleEnabled() * Atomic write (tmp + rename), preserves other keys in the file * Treats missing file / missing enabled key as default `true` * Recovers from malformed JSON by overwriting - src/gateway/commands.ts: handleToggleReview() posts confirmation - src/gateway/gateway.ts: wired into handleOrAbort (snappy path, not thread-queue-gated, matches /verbose) - src/gateway/code-review-toggle.test.ts: 15 unit tests Command depends on pi-hard-no \u2265 1.3.0 being installed for runtime effect; the write itself always succeeds (plain JSON), so the command is safe even if the extension isn't loaded \u2014 just a no-op at the extension end. Tests: 15 new / 471 total green. Typecheck clean.
F6 (Medium): toggle now respects pi-hard-no's local-vs-global file
precedence. New ToggleOptions { cwd?, home? } \u2014 when cwd is provided
and <cwd>/.hardno/settings.json exists, writes there instead of global.
Gateway passes agent.getInfo().cwd so the toggle hits the same file
pi-hard-no will read on the next agent_end. Previously: roundhouse
wrote global, pi-hard-no read local (enabled still true), toggle
silently ineffective.
F1 Codex (High) / F2 reviewer (Medium): lost-update race on concurrent
writers. toggleEnabled() now captures mtime at read and retries (up
to 3x) if another writer (e.g. pi-hard-no's own Alt+R handler) updates
the file between our read and rename. Also cleans up orphan tmp files
on any write/rename failure.
API surface changes:
- resolveSettingsPath(opts) now returns { path, isLocal } instead of a bare string
- resolveGlobalSettingsPath() kept for the case where cwd isn't known
- toggleEnabled(opts) signature: ToggleOptions object (was home: string)
- readEnabled(opts) signature: ToggleOptions object (was home: string)
- ToggleResult.wroteLocal: boolean \u2014 surfaced to handler for UX clarity
("wrote to project-local config" vs "global")
User-facing: handleToggleReview now reports scope + path:
"Wrote to project-local config (~/repos/x/.hardno/settings.json).
Takes effect on the next agent turn \u2014 no restart needed."
Tests: 21 new (was 15) / 477 total green. New coverage:
- resolveSettingsPath with/without cwd, with/without local file
- toggleEnabled routing: local present/absent, global fallback
- end-to-end: toggle writes local, read picks it up (F6 regression)
Architect review on pi-hard-no flagged missing docs for the new toggle; same rationale applies here \u2014 it's a user-facing slash command that users won't discover without README mention. - Added row to the Telegram bot commands table (+ alias) - Added details section explaining persistence + routing semantics
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41be16c7bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // Race check: if another writer updated the file between our read and | ||
| // our rename, discard this tmp and retry so we don't clobber their edits. | ||
| if (readMtime !== null && attempt < MAX_WRITE_ATTEMPTS) { |
There was a problem hiding this comment.
The mtime conflict guard is skipped on the final retry because of attempt < MAX_WRITE_ATTEMPTS, so a concurrent writer that updates settings.json between our read and rename on attempt 3 will be silently overwritten. This reintroduces lost updates exactly under the contention this loop is meant to handle (e.g., /toggle-review racing with pi-hard-no’s own toggle), so the last attempt should still detect conflicts and fail/retry instead of renaming unconditionally.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
New slash command (
/toggle-review, alias/toggle-code-review) that flips pi-hard-no's auto code review on/off persistently and without a session restart.Pairs with pi-hard-no PR #7 which adds the runtime re-read mechanism.
How it works
/toggle-reviewin Telegram<agentCwd>/.hardno/settings.jsonif present, else~/.pi/.hardno/settings.json(matches pi-hard-no's read order)enabledboolean, preserving all other fieldsagent_end\u2014 effect on the next agent turn, no restartSafety
enabled, re-writes/verbose,/stop)Dispatch placement
Wired into
handleOrAbort(not the command registry) so it works mid-turn, matching/verboseand/stopsemantics. Registry dispatch is serialized behind the thread queue; toggle should be immediate.Review history
2 review cycles + architect pass. Findings addressed:
Tests
code-review-toggle.test.ts(was 15 baseline)resolveSettingsPathwith/without cwd, toggle routing (local present/absent), end-to-end (toggle writes local \u2192 read picks it up, no masking)API notes
Slight breaking change to module internals (not public):
toggleEnabled(opts: ToggleOptions)\u2014 wastoggleEnabled(home?: string)readEnabled(opts: ToggleOptions)\u2014 wasreadEnabled(home?: string)ToggleResult.wroteLocal: booleanfor UX clarityRequires