Skip to content

Share YOLO mode across session families#497

Merged
pascalandr merged 9 commits into
NeuralNomadsAI:devfrom
OfflinePing:feat/subagents-inherit-yolo-mode
May 25, 2026
Merged

Share YOLO mode across session families#497
pascalandr merged 9 commits into
NeuralNomadsAI:devfrom
OfflinePing:feat/subagents-inherit-yolo-mode

Conversation

@OfflinePing
Copy link
Copy Markdown
Contributor

@OfflinePing OfflinePing commented May 23, 2026

Summary

  • make YOLO/permission auto-accept resolve by session family root so master and subagent sessions share the same state
  • keep fork/revert sessions as their own family root
  • keep existing local YOLO persistence behavior; no server-backed persistence, migration, global setting, or confirmation dialog
  • re-drain queued child permissions when session metadata arrives so permissions that arrived before parent metadata can be auto-accepted

Solves #495

Validation

  • npm run typecheck --workspace @codenomad/ui
  • npx tsx --test packages/ui/src/stores/permission-auto-accept.test.ts
  • git diff --check

Build Artifact

  • Not generated in this update

Remember YOLO Mode for newly created sessions when the active session has auto-accept enabled, so users do not need to re-enable it for each fresh session.

Add an explicit persisted subagent inheritance preference under Appearance > Interaction. The setting defaults off, requires a warning confirmation before enabling, and only lets non-fork child sessions inherit YOLO when their parent session already has YOLO enabled.

Keep existing approval safety defaults intact by avoiding hydration-time mutation of historical sessions, and cover the inheritance rules plus confirmation dialog arguments with targeted Node tests.

Validation: npm run typecheck --workspace @codenomad/ui; npx tsx --test packages/ui/src/stores/permission-auto-accept.test.ts packages/ui/src/stores/preferences.test.ts; git diff --check; npm run build:linux --workspace @neuralnomads/codenomad-electron-app
@OfflinePing
Copy link
Copy Markdown
Contributor Author

image

Picture with the Setting in the background + Modal to Confirm

@github-actions
Copy link
Copy Markdown

ghost commented May 23, 2026

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/26331023049

Artifacts expire in 7 days.
Artifacts:

  • pr-497-d5818b928bba90de0900055edc8f0ffd865cbe61-electron-macos
  • pr-497-d5818b928bba90de0900055edc8f0ffd865cbe61-tauri-linux
  • pr-497-d5818b928bba90de0900055edc8f0ffd865cbe61-tauri-windows
  • pr-497-d5818b928bba90de0900055edc8f0ffd865cbe61-tauri-macos
  • pr-497-d5818b928bba90de0900055edc8f0ffd865cbe61-tauri-macos-arm64
  • pr-497-d5818b928bba90de0900055edc8f0ffd865cbe61-electron-windows
  • pr-497-d5818b928bba90de0900055edc8f0ffd865cbe61-electron-linux

@pascalandr
Copy link
Copy Markdown
Contributor

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot Bot commented May 23, 2026

Request understood: I reviewed the latest PR state as a gatekeeper for regressions and implementation quality. Assumption: the requested outcome is a review-only pass, not code changes.

Gatekeeper Result

I would not approve this PR yet. There is one blocking behavioral regression in the new YOLO inheritance path.

Findings

  1. Blocking: fork/revert child sessions can incorrectly inherit YOLO mode

    packages/ui/src/stores/session-events.ts:465-500

    In the handleSessionUpdate new-session path, newSession is constructed without copying info.revert, then adoptSubagentPermissionAutoAccept(instanceId, newSession) is called. The inheritance rule explicitly excludes revert/fork sessions via !session.revert, but this code path always passes a session whose revert is absent.

    setSessionRevertV2(instanceId, info.id, info.revert ?? null) does not update the Session object; it only writes message-store metadata (packages/ui/src/stores/message-v2/bridge.ts:312-316). So a session.updated event for a forked/reverted child with parentID and revert can still enable auto-accept when the parent has YOLO enabled and subagent inheritance is on.

    Why this matters: the PR summary says inheritance should not apply to fork/revert sessions, and the unit test covers the pure rule helper, but the production event adapter bypasses that protection by dropping revert before calling the helper.

    Suggested fix: include revert in newSession before adoption, or call adoptSubagentPermissionAutoAccept with an object that carries revert: info.revert ? {...} : undefined. Add an event-path test, not just a helper test, so this cannot regress.

Implementation Notes

  • The helper split in permission-auto-accept-rules.ts is a good direction: it keeps the risky policy decision testable.
  • The Appearance setting rollback on cancelled confirmation is reasonable, but only covered at the dialog-arg helper level. I would prefer one UI/store integration test proving the preference remains false when the confirmation returns false.
  • The optimistic select override has no .catch(...); if a future async toggle rejects, the UI can stay optimistically wrong. Not blocking for the current showConfirmDialog behavior, but the contract now allows async setters, so this is a sharp edge.

Validation Run

  • npx tsx --test packages/ui/src/stores/permission-auto-accept.test.ts packages/ui/src/stores/preferences.test.ts: passed, 6 tests.
  • git diff --check origin/dev...HEAD: passed.
  • npm run typecheck --workspace @codenomad/ui: failed in this local worktree due widespread missing dependency/type resolution errors (solid-js, @opencode-ai/sdk, Node built-ins, Tauri modules). I did not treat this as PR-specific because the failures indicate the workspace dependencies/types are not installed/resolved in this bot environment.

Verdict

Block until the session.updated new-session path preserves revert before applying subagent YOLO inheritance. After that, rerun the targeted tests and add coverage for the event adapter path.

--
Yours,
CodeNomadBot

@pascalandr pascalandr self-requested a review May 23, 2026 15:05
Ensure session.updated events carry revert metadata into the session model before evaluating subagent YOLO inheritance. This prevents fork/revert child sessions from bypassing the policy rule that excludes reverted sessions.

Add focused adapter coverage for the event path and extend confirmation tests for cancelled enables and disabling without prompts. Also rollback optimistic settings UI state when async setters reject.

Validation: npm run typecheck --workspace @codenomad/ui; npx tsx --test packages/ui/src/stores/permission-auto-accept.test.ts packages/ui/src/stores/preferences.test.ts packages/ui/src/stores/session-event-adapters.test.ts; git diff --check
@github-actions
Copy link
Copy Markdown

ghost commented May 23, 2026

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/26337650167

Artifacts expire in 7 days.
Artifacts:

  • pr-497-11e20e8f6bc6431044b0c4ec5bbb56e23f3a5bd4-electron-macos
  • pr-497-11e20e8f6bc6431044b0c4ec5bbb56e23f3a5bd4-tauri-macos
  • pr-497-11e20e8f6bc6431044b0c4ec5bbb56e23f3a5bd4-tauri-linux
  • pr-497-11e20e8f6bc6431044b0c4ec5bbb56e23f3a5bd4-tauri-windows
  • pr-497-11e20e8f6bc6431044b0c4ec5bbb56e23f3a5bd4-tauri-macos-arm64
  • pr-497-11e20e8f6bc6431044b0c4ec5bbb56e23f3a5bd4-electron-windows
  • pr-497-11e20e8f6bc6431044b0c4ec5bbb56e23f3a5bd4-electron-linux

@shantur
Copy link
Copy Markdown
Collaborator

shantur commented May 23, 2026

@OfflinePing - I don't see the reason why only the main agent has YOLO mode enabled and not subagents?

@OfflinePing
Copy link
Copy Markdown
Contributor Author

OfflinePing commented May 23, 2026

@OfflinePing - I don't see the reason why only the main agent has YOLO mode enabled and not subagents?

Sorry I dont understand the question really.

Right now only the I often tell my Main Session to use Subagents; if it spawns 5-10 of them I dont want to have to give them Yolo-mode or manually approve commands for them. I think they should be able to adopt the Yolo-Mode from the Master Session. Currently this is not implemented.

Although I got to work on this Line:
"remember YOLO Mode for newly created sessions when the active session already has it enabled"

I meant, once I enable it for a Session it should stay enabled even across restarts. <- Working on this one

Demo of the behaviour:

recording_2026-05-23_21.mp4

@pascalandr
Copy link
Copy Markdown
Contributor

@CodeNomadBot
same review

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot Bot commented May 23, 2026

Request understood: I reran the same gatekeeper review against the latest PR state after commit 11e20e8 (fix(ui): preserve fork state for YOLO inheritance). Assumption: this is review-only, not a request to modify the branch.

Gatekeeper Result

Still not ready to approve. The previous fork/revert metadata blocker appears fixed, but there is a remaining race in the subagent inheritance flow that can leave a subagent permission stuck waiting even after YOLO is inherited.

Findings

  1. Blocking: inherited YOLO does not drain permissions that were queued before session adoption

    packages/ui/src/stores/session-events.ts:246-250 and packages/ui/src/stores/session-events.ts:478-481

    The new adoption path calls adoptSubagentPermissionAutoAccept(instanceId, session) when a subagent session is first discovered. That only flips the per-session auto-accept flag. It does not drain any permission request that may already be queued for that session.

    packages/ui/src/stores/instances.ts:860 drains auto-accept only when a permission is added to the queue. packages/ui/src/stores/instances.ts:903-908 drains again when a user manually toggles YOLO on. There is no equivalent drain when YOLO is enabled by inheritance.

    Why this matters: SSE/event ordering is not guaranteed to favor session.updated before permission-related events. If a subagent emits a permission request before the session has been fetched/created locally and inheritance is applied, addPermissionToQueue sees auto-accept disabled and leaves it pending. When adoptSubagentPermissionAutoAccept later enables YOLO for that child, nothing revisits the already pending queue, so the user still has to approve manually. That directly undercuts the feature goal: inherited YOLO should avoid manual approvals for spawned subagents.

    Suggested fix: whenever adoptSubagentPermissionAutoAccept(...) returns true, trigger the same queue drain used by manual toggling for that instance/session. Keep it scoped to that session if possible, or expose a small store action from instances.ts rather than coupling permission-auto-accept.ts back into the permission queue.

  2. Resolved: fork/revert sessions no longer appear to inherit YOLO incorrectly

    packages/ui/src/stores/session-event-adapters.ts:7-42 and packages/ui/src/stores/session-event-adapters.test.ts:7-39

    The new adapter preserves revert before inheritance checks, and the targeted regression test covers both forked and normal child sessions. This addresses the blocker from my previous review.

Implementation Notes

  • The adapter extraction is a reasonable minimal fix and improves testability without broad refactoring.
  • The confirmation rollback path is improved: rejected async setters now restore the previous select override.
  • I would still prefer an integration-style test for the inherited-YOLO drain behavior once fixed, because this feature depends on event ordering.

Validation Run

  • npx tsx --test packages/ui/src/stores/permission-auto-accept.test.ts packages/ui/src/stores/preferences.test.ts packages/ui/src/stores/session-event-adapters.test.ts: passed, 10 tests.
  • git diff --check origin/dev...HEAD: passed.
  • npm run typecheck --workspace @codenomad/ui: failed in this bot worktree because dependencies/types are not resolved (solid-js, @opencode-ai/sdk, Node built-ins, Tauri modules, etc.). I did not treat that as PR-specific in this environment.

Verdict

Block until inherited YOLO also drains already-queued permissions for the adopted subagent session. After that, the prior fork/revert blocker looks covered.

--
Yours,
CodeNomadBot

Make subagent YOLO inheritance follow the parent session directly and persist auto-accept state with workspace-scoped keys so approvals survive instance recreation.

Add server-state persistence, legacy localStorage migration, and queued permission draining for newly inherited child sessions. Remove the separate subagent inheritance preference and confirmation UI because inheritance is now intentionally tied to parent YOLO state.

Validation: npm run test --workspace packages/ui; npm run typecheck --workspace packages/ui
@OfflinePing OfflinePing changed the title Persist YOLO mode and add subagent inheritance control Persist YOLO mode and inherit in subagents May 23, 2026
@OfflinePing
Copy link
Copy Markdown
Contributor Author

YOLO behavior is now simple:

  • If the master session has YOLO enabled, its subagents automatically inherit YOLO.
  • If YOLO is disabled on the master session, inherited YOLO is disabled for its subagents too.
  • You can still turn YOLO off for one subagent without affecting the master or other subagents.
  • If the master YOLO is turned off and later back on, subagents inherit YOLO again.

Persistence:

  • The master session’s YOLO state stays persisted across app restarts.
  • There is no separate global “subagents inherit YOLO” setting anymore.
  • Individual subagent manual-off overrides are temporary and do not permanently block future inheritance.

@github-actions
Copy link
Copy Markdown

ghost commented May 23, 2026

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/26344550871

Artifacts expire in 7 days.
Artifacts:

  • pr-497-ed26da1e687e5942cad81dd33fac6f5838192bf7-tauri-linux
  • pr-497-ed26da1e687e5942cad81dd33fac6f5838192bf7-electron-macos
  • pr-497-ed26da1e687e5942cad81dd33fac6f5838192bf7-tauri-macos
  • pr-497-ed26da1e687e5942cad81dd33fac6f5838192bf7-tauri-windows
  • pr-497-ed26da1e687e5942cad81dd33fac6f5838192bf7-tauri-macos-arm64
  • pr-497-ed26da1e687e5942cad81dd33fac6f5838192bf7-electron-linux
  • pr-497-ed26da1e687e5942cad81dd33fac6f5838192bf7-electron-windows

1 similar comment
@github-actions
Copy link
Copy Markdown

ghost commented May 23, 2026

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/26344550871

Artifacts expire in 7 days.
Artifacts:

  • pr-497-ed26da1e687e5942cad81dd33fac6f5838192bf7-tauri-linux
  • pr-497-ed26da1e687e5942cad81dd33fac6f5838192bf7-electron-macos
  • pr-497-ed26da1e687e5942cad81dd33fac6f5838192bf7-tauri-macos
  • pr-497-ed26da1e687e5942cad81dd33fac6f5838192bf7-tauri-windows
  • pr-497-ed26da1e687e5942cad81dd33fac6f5838192bf7-tauri-macos-arm64
  • pr-497-ed26da1e687e5942cad81dd33fac6f5838192bf7-electron-linux
  • pr-497-ed26da1e687e5942cad81dd33fac6f5838192bf7-electron-windows

@pascalandr
Copy link
Copy Markdown
Contributor

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality
Be especially careful about the amount of change, the diff seems way too large for this implementation

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot Bot commented May 23, 2026

Request understood: I reviewed the latest PR state as a gatekeeper, with extra scrutiny on regressions, implementation quality, and whether the amount of change is justified. Assumption: this is review-only, not a request to modify the branch.

Gatekeeper Result

I would not approve this PR yet. The previous queued-permission drain issue is addressed, but the latest implementation introduces a new behavioral regression in per-session YOLO control and the diff is now much larger than the feature requires.

Findings

  1. Blocking: manually enabled subagent YOLO is cleared whenever the parent does not have YOLO

    packages/ui/src/stores/permission-auto-accept.ts:241-261

    syncInheritedPermissionAutoAcceptForChildren disables every eligible child whenever parentEnabled is false:

    setPermissionAutoAcceptEnabledInternal(instanceId, session.id, false, { manual: false })

    That happens without checking whether the child was actually enabled by inheritance. Because session-state.ts now calls syncAllInheritedPermissionAutoAccept() after every setSessions update (packages/ui/src/stores/session-state.ts:32-39), a user cannot reliably keep YOLO enabled on an individual subagent when the parent is off. The next session state update will see the parent disabled and forcibly clear the child.

    This is a regression from the existing per-session YOLO model: the right-panel switch is still exposed for every active session, so users can manually enable a subagent independently. Parent inheritance should clear only child state that was inherited from that parent, not manually chosen child state.

    Suggested fix: when parentEnabled is false, only disable children where inheritedParentBySession.get(childKey) === parentSessionId. Do not clear independently/manual child YOLO state. Add a test for: parent off, child manually enabled, syncInheritedPermissionAutoAcceptForChildren(...) or syncAllInheritedPermissionAutoAccept() runs, child remains enabled.

  2. High risk: the implementation scope has expanded far beyond the requested behavior

    Diff is now 18 files changed, 1098 insertions(+), 51 deletions(-), including new server-backed persistence machinery, local/server merge logic, retry/echo suppression, global session-provider registration, and a broad test runner change.

    For the requested behavior, the necessary change should be much smaller: persist per-session YOLO with a stable workspace/session key, inherit from parent for eligible subagents, and drain queued permissions after adoption. The current design adds multiple new cross-store global callbacks and persistence reconciliation paths, which substantially increases regression surface.

    The most concerning coupling is:

    • instances.ts registers scopes and permission drainers into permission-auto-accept.ts.
    • session-state.ts registers the session provider and invokes inheritance synchronization after every session write.
    • permission-auto-accept.ts writes localStorage, patches server UI state, mutates inherited-child state, and calls back into permission draining.

    This is a lot of global bidirectional behavior for a narrowly scoped feature. I would prefer a smaller implementation where inheritance is applied only at explicit points: parent toggle, child session creation/fetch, and pending-permission drain.

  3. Non-blocking but important: test coverage misses the manual-child override regression

    packages/ui/src/stores/permission-auto-accept.test.ts:401-448

    The new inheritance test covers child manual-off while parent is enabled, and parent off/on restoring inheritance. It does not cover child manual-on while parent is disabled. That is exactly the case currently broken by unconditional child clearing.

Resolved From Previous Review

  • The queued-permission drain issue appears addressed via adoptSubagentPermissionAutoAcceptAndDrain(...) in session-events.ts.
  • The fork/revert metadata issue remains covered by the adapter test.

Validation Run

  • git diff --check origin/dev...HEAD: passed.
  • npm run test --workspace packages/ui: failed in this bot worktree because dependencies such as solid-js and debug are not resolvable here. Some pure tests ran before that failure, but I cannot use this run as full validation.
  • npm run typecheck --workspace packages/ui: failed in this bot worktree for the same dependency/type-resolution issue. I did not treat the missing dependency resolution itself as PR-specific, but the run did expose newly added test/type sites that would need to be clean in CI.

Verdict

Block until inherited state synchronization only clears children that were actually inherited from the parent, and add a regression test for independently enabled child YOLO with a disabled parent. I also strongly recommend reducing the diff: keep persistence and inheritance focused, and avoid global cross-store synchronization unless a smaller explicit-event approach cannot cover the behavior.

--
Yours,
CodeNomadBot

@pascalandr
Copy link
Copy Markdown
Contributor

pascalandr commented May 24, 2026

@OfflinePing As discussed in discord this PR should not contain the YOLO persistance finally. And thinking more about it, we should stick for even simpler form, (I should have done it when I implemented YOLO mode) indeed we can just say Yolo mode is for a session familly = (master + sub sessions) as suggested by Shantur so this become a dead simple implementation ~ 40 lignes :
for exemple here is a quick diff on top of dev for that :

diff --git a/packages/ui/src/stores/instances.ts b/packages/ui/src/stores/instances.ts
index ccf14f16..753b6f71 100644
--- a/packages/ui/src/stores/instances.ts
+++ b/packages/ui/src/stores/instances.ts
@@ -28,7 +28,7 @@ import {
 } from "./worktrees"
 import { fetchCommands, clearCommands } from "./commands"
 import { serverSettings } from "./preferences"
-import { setSessionPendingPermission, setSessionPendingQuestion } from "./session-state"
+import { sessions, setSessionPendingPermission, setSessionPendingQuestion } from "./session-state"
 import { setHasInstances } from "./ui"
 import { messageStoreBus } from "./message-v2/bus"
 import { upsertPermissionV2, removePermissionV2, upsertQuestionV2, removeQuestionV2 } from "./message-v2/bridge"
@@ -38,7 +38,14 @@ import {
   markPermissionReplied,
   pruneRepliedPermissions,
 } from "./permission-replies"
-import { clearAutoAcceptPermission, drainAutoAcceptPermissions, isPermissionAutoAcceptEnabled, togglePermissionAutoAccept } from "./permission-auto-accept"
+import {
+  clearAutoAcceptPermission,
+  drainAutoAcceptPermissions,
+  isPermissionAutoAcceptEnabled,
+  resolvePermissionAutoAcceptFamilyRoot,
+  setPermissionAutoAcceptFamilyRootResolver,
+  togglePermissionAutoAccept,
+} from "./permission-auto-accept"
 import { clearCacheForInstance } from "../lib/global-cache"
 import { getLogger } from "../lib/logger"
 import { mergeInstanceMetadata, clearInstanceMetadata } from "./instance-metadata"
@@ -47,6 +54,12 @@ import { activeSidecarToken } from "./sidecars"
 
 const log = getLogger("api")
 
+setPermissionAutoAcceptFamilyRootResolver((instanceId, sessionId) => {
+  const instanceSessions = sessions().get(instanceId)
+  if (!instanceSessions) return sessionId
+  return resolvePermissionAutoAcceptFamilyRoot(sessionId, (id) => instanceSessions.get(id))
+})
+
 const [instances, setInstances] = createSignal<Map<string, Instance>>(new Map())
 
 const [activeInstanceId, setActiveInstanceId] = createSignal<string | null>(null)
diff --git a/packages/ui/src/stores/permission-auto-accept.ts b/packages/ui/src/stores/permission-auto-accept.ts
index 1b1ea402..1c26b8e8 100644
--- a/packages/ui/src/stores/permission-auto-accept.ts
+++ b/packages/ui/src/stores/permission-auto-accept.ts
@@ -9,9 +9,38 @@ const log = getLogger("api")
 
 type AutoAcceptResponder = (instanceId: string, sessionId: string, requestId: string, reply: PermissionReply) => Promise<void>
 type PendingPermissionChecker = (instanceId: string, requestId: string) => boolean
+type PermissionAutoAcceptSession = {
+  id: string
+  parentId?: string | null
+  revert?: unknown
+}
+type SessionLookup = (sessionId: string) => PermissionAutoAcceptSession | undefined
+type FamilyRootResolver = (instanceId: string, sessionId: string) => string
+
+let resolveFamilyRoot: FamilyRootResolver = (_instanceId, sessionId) => sessionId
+
+export function resolvePermissionAutoAcceptFamilyRoot(sessionId: string, getSession: SessionLookup): string {
+  let currentId = sessionId
+  const seen = new Set<string>()
+
+  while (currentId && !seen.has(currentId)) {
+    seen.add(currentId)
+    const session = getSession(currentId)
+    if (!session) return currentId
+    if (session.revert) return session.id
+    if (!session.parentId) return session.id
+    currentId = session.parentId
+  }
+
+  return currentId || sessionId
+}
+
+export function setPermissionAutoAcceptFamilyRootResolver(resolver: FamilyRootResolver) {
+  resolveFamilyRoot = resolver
+}
 
 function makeKey(instanceId: string, sessionId: string) {
-  return `${instanceId}:${sessionId}`
+  return `${instanceId}:${resolveFamilyRoot(instanceId, sessionId)}`
 }
 
 function readInitialState() {

@pascalandr
Copy link
Copy Markdown
Contributor

@CodeNomadBot
what do you think about the proposed diff over dev ?

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot Bot commented May 25, 2026

Request understood: I reviewed the proposed minimal diff from the comment as a candidate replacement over dev, not the current large PR diff. Assumption: you want an opinion on whether that approach is safer and what needs fixing before using it.

Short Take

I strongly prefer the proposed direction over the current PR. It matches the simpler product model: YOLO is a session-family property, not a parent setting plus child overrides plus persistence reconciliation.

But I would not merge the exact proposed diff as-is. It needs a couple of small fixes/tests around event ordering and incomplete session trees.

What Is Good

  • Much smaller blast radius: roughly two stores touched instead of adding server-backed persistence, migration, retry, echo suppression, and global sync callbacks.
  • The model is easier to reason about: makeKey(instanceId, sessionId) normalizes to the family root, so permission auto-accept naturally applies to master + subagents.
  • Fork/revert behavior is handled in the right place: stop walking up when session.revert is present, so forks keep their own YOLO scope.
  • Existing drain points mostly continue to work because isPermissionAutoAcceptEnabled(instanceId, childSessionId) resolves to the parent/root key when the session tree is known.

Findings

  1. Needs fix: queued permission can still miss auto-accept if the permission arrives before session metadata

    The proposed diff changes key resolution, but it does not add a drain when session metadata later becomes available.

    Scenario:

    • Child permission arrives first.
    • sessions().get(instanceId) does not yet know the child/parent relationship.
    • isPermissionAutoAcceptEnabled(instanceId, childSessionId) resolves against the child id, not the family root, and does not auto-accept.
    • Later session.updated or session fetch hydrates the child parentId.
    • The already-queued permission is not revisited unless another permission sync/event happens.

    This is the same event-ordering class we discussed earlier, but the minimal fix can still be tiny: after adding or hydrating a session with a parentId, drain queued permissions for that child session, or call the existing drain over the queue after session updates that add parent metadata.

  2. Needs fix: missing parent lookup currently resolves to the missing parent id

    In the proposed resolver:

    const session = getSession(currentId)
    if (!session) return currentId

    If the child is known but the parent is not loaded yet, the first loop reads the child, sets currentId = child.parentId, then the next loop returns the missing parent id. That means toggling or checking a child can write/read a key for a session object that is not actually present yet.

    That may be okay for inheriting from a known persisted parent key, but it is risky for manual toggles: enabling YOLO on a child while the parent is missing could accidentally enable the parent/family later when the parent appears.

    Safer behavior: return the last known session id when the chain becomes incomplete, or explicitly only resolve to a parent/root when every hop is present. Add a test for child-known/parent-missing.

  3. Product semantics need to be reflected in the UI

    With family-root keys, toggling YOLO on a subagent toggles the entire family root if the parent chain is known. That is consistent with “YOLO mode is for a session family,” but it is different from per-session semantics.

    I think that is acceptable if intentional, but the UI copy should avoid implying the switch only affects the selected subagent. Otherwise users will be surprised when toggling a child affects the master and siblings.

Recommended Minimal Shape

I would proceed with this approach, not the large current PR, with these additions only:

  • Add resolvePermissionAutoAcceptFamilyRoot and root resolver as proposed.
  • Keep fork/revert as its own root.
  • Add a small drain call when a child session is created/hydrated with parent metadata, so already-queued child permissions are rechecked.
  • Make incomplete parent chains safe and tested.
  • Add focused tests for root resolution: normal child, nested child if supported, fork/revert, cycle guard, parent missing, and queued permission before session metadata.

Verdict

The proposed diff is the right direction and far better than the current large PR, but it needs the event-ordering drain and incomplete-tree handling before I would approve it. Keep it close to this size; do not reintroduce server-backed YOLO persistence or global synchronization machinery for this feature.

--
Yours,
CodeNomadBot

Remove the added localStorage and server-state persistence for YOLO auto-accept so the per-session toggle returns to runtime-only behavior.

Keep the subagent adoption feature intact: child sessions can still inherit YOLO while the parent session has it enabled, and inherited children continue to track parent changes within the running app.

Also remove new-session YOLO carryover from the active session so only explicit subagent inheritance controls this behavior.

Validation: npm run typecheck --workspace @codenomad/ui; npx tsx --test packages/ui/src/stores/permission-auto-accept.test.ts packages/ui/src/stores/session-event-adapters.test.ts; git diff --check
@github-actions
Copy link
Copy Markdown

ghost commented May 25, 2026

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/26393149215

Artifacts expire in 7 days.
Artifacts:

  • pr-497-ec1042e591d9ed536030274c39ae34819c455f78-electron-macos
  • pr-497-ec1042e591d9ed536030274c39ae34819c455f78-tauri-macos
  • pr-497-ec1042e591d9ed536030274c39ae34819c455f78-tauri-windows
  • pr-497-ec1042e591d9ed536030274c39ae34819c455f78-tauri-linux
  • pr-497-ec1042e591d9ed536030274c39ae34819c455f78-tauri-macos-arm64
  • pr-497-ec1042e591d9ed536030274c39ae34819c455f78-electron-windows
  • pr-497-ec1042e591d9ed536030274c39ae34819c455f78-electron-linux

@pascalandr
Copy link
Copy Markdown
Contributor

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot Bot commented May 25, 2026

Request understood: I reviewed the latest pushed PR state against dev as a gatekeeper for regressions and implementation quality. Assumption: this is review-only, not a request to modify the branch.

Gatekeeper Result

Still blocked. The branch is smaller than the previous server-backed persistence version, but it still has serious behavior regressions and it did not adopt the minimal family-root approach discussed in the thread.

Findings

  1. Blocking: this PR removes existing YOLO persistence from dev

    packages/ui/src/stores/permission-auto-accept.ts:21-37

    On dev, YOLO state is read from and written to localStorage via codenomad:permission-auto-accept:v1. In the latest PR, that storage layer is gone and the state initializes as an empty in-memory map:

    const [autoAcceptState, setAutoAcceptState] = createSignal(new Map<string, boolean>())

    That means any currently persisted YOLO state is ignored and any newly toggled state is lost on reload. Even if we no longer want the new server-backed persistence, deleting the existing localStorage behavior is still a regression over dev and contradicts the PR title/body, which still says “persist YOLO”.

    If the product decision is truly “no persistence at all,” that should be explicit and the PR summary should say it removes existing persistence. Otherwise, restore the existing localStorage read/write path.

  2. Blocking: manually enabled child YOLO is still cleared when the parent is disabled

    packages/ui/src/stores/permission-auto-accept.ts:124-144

    The inherited-state sync still does this whenever parentEnabled is false:

    setPermissionAutoAcceptEnabledInternal(instanceId, session.id, false, { manual: false })

    It does not check whether the child was actually enabled by inheritance. Because session-state.ts now calls syncAllInheritedPermissionAutoAccept() after every setSessions write, a user can manually enable YOLO on a subagent while the parent is off and then lose it on the next session-state update.

    This is the same blocker from the previous review. The branch still has a hybrid model: the UI exposes a per-session switch, but the sync code sometimes treats child state as parent-owned. Either switch to the minimal family-root model fully, or only clear child state that was actually inherited from that parent.

  3. High risk: the implementation is still more complex than the proposed minimal approach

    Latest diff is still 17 files changed, 459 insertions(+), 77 deletions(-). The server-backed persistence file is gone, but the PR still keeps:

    • global session-provider registration from session-state.ts into permission-auto-accept.ts
    • global permission-drainer registration from instances.ts into permission-auto-accept.ts
    • inherited child state maps and manual-disable tracking
    • syncAllInheritedPermissionAutoAccept() called after every session state mutation
    • new session-event adapter/test plumbing

    That is still a lot of cross-store state synchronization for a feature that can be modeled by resolving a session to its family root key. I still prefer the proposed small family-root implementation, with the two fixes noted earlier: re-drain when child metadata arrives and handle incomplete parent chains safely.

  4. Test gap: no coverage for the blocking manual-child regression

    packages/ui/src/stores/permission-auto-accept.test.ts:35-80

    The inheritance test covers child manual-off while parent is enabled. It still does not cover child manual-on while parent is disabled. That is the broken case.

Validation Run

  • git diff --check origin/dev...HEAD: passed.
  • npm run test --workspace packages/ui: failed in this bot worktree because dependencies such as solid-js are not resolvable here. Some unrelated pure tests passed before the failure, but this cannot be treated as full validation.
  • npm run typecheck --workspace packages/ui: failed in this bot worktree for the same dependency/type-resolution issue. I did not treat the environment resolution itself as PR-specific.

Verdict

Do not merge this state. At minimum, restore or explicitly justify removal of existing localStorage persistence, fix inherited sync so it does not erase manually enabled child YOLO, and add the missing regression test. My recommended path remains replacing this with the small family-root approach rather than continuing to patch the cross-store inherited-state model.

--
Yours,
CodeNomadBot

Replace the inherited-subagent YOLO implementation with a smaller session-family root lookup. This keeps existing per-session persistence semantics while making master and subagent sessions share the same YOLO key, with fork/revert sessions acting as their own family root.

The cleanup also removes the earlier settings, adapter, and test-runner plumbing from the effective PR diff so the branch only carries the minimal runtime behavior change.
@pascalandr
Copy link
Copy Markdown
Contributor

@CodeNomadBot

I want you to perform the gatekeeper checks for latest PR state

check for regressions
check for better possible implementations
Be the PR gatekeeper
Be ruthless about code quality

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot Bot commented May 25, 2026

Request understood: I reviewed the latest pushed PR state against dev as a gatekeeper for regressions and implementation quality. Assumption: this is review-only, not a request to modify the branch.

Gatekeeper Result

This is much closer to the right shape: the diff is now tiny (2 files changed, 45 insertions, 3 deletions) and the current PR finally follows the family-root direction instead of the previous cross-store inherited-state model.

I still would not merge it yet. The exact two safety gaps from the proposed-diff review remain unfixed, and the PR metadata/validation is stale.

Findings

  1. Blocking: permissions can still miss YOLO if they arrive before child session metadata

    packages/ui/src/stores/permission-auto-accept.ts:77-129 and packages/ui/src/stores/instances.ts:916-920

    The implementation now resolves makeKey(instanceId, sessionId) through the current sessions() tree. That works only after the child’s parentId is known.

    Failing order:

    • Parent has YOLO enabled.
    • Child permission arrives before the child session is hydrated in sessions().
    • isPermissionAutoAcceptEnabled(instanceId, childId) resolves to instanceId:childId, not instanceId:parentId, so the permission stays pending.
    • Later session metadata arrives and child.parentId becomes known.
    • Nothing re-drains the already queued child permission.

    Existing drain points only run when a permission is added or when the user manually toggles YOLO. This diff does not add a small drain after child session hydration/update, so the event-ordering bug remains.

    Minimal fix: when a session is created/fetched/updated with a parentId, call the existing drain path for that session or the queue. Keep this explicit and small.

  2. Blocking: incomplete parent chains resolve to a missing parent id

    packages/ui/src/stores/permission-auto-accept.ts:22-35

    The resolver currently does this:

    const session = getSession(currentId)
    if (!session) return currentId

    If the child is loaded but the parent is not, the loop advances from childId to parentId, then returns parentId even though that parent session is not loaded. That can make a manual child toggle write/read the missing parent’s key and later affect the family once the parent appears.

    Safer behavior: keep track of the last known session id and return that when the chain becomes incomplete, or only resolve to a parent/root when every hop is present. Add a focused test for child-known/parent-missing.

  3. High: PR description and validation are now stale/misleading

    The PR body still claims server-backed UI state, legacy migration, and npm run test --workspace packages/ui. The latest diff no longer contains server-backed persistence or migration, and packages/ui currently has no test script in this branch.

    Validation result from this bot worktree:

    • git diff --check origin/dev...HEAD: passed.
    • npm run test --workspace packages/ui: failed because the workspace has no test script.
    • npm run typecheck --workspace packages/ui: failed due the same dependency/type-resolution issue seen in prior bot runs (solid-js, SDK/Tauri modules, Node test types missing in this environment), so I cannot treat the typecheck output as PR-specific here.

What Improved

  • The current diff is appropriately small compared with previous iterations.
  • Existing localStorage persistence from dev is preserved now.
  • The previous “manual child YOLO gets cleared by sync” regression is gone because the inherited-state sync machinery was removed.

Verdict

Close, but still block until the event-ordering drain and incomplete-parent-chain handling are fixed. After that, update the PR body to match the actual implementation and add focused tests for root resolution and permission-before-session-metadata ordering.

--
Yours,
CodeNomadBot

@github-actions
Copy link
Copy Markdown

ghost commented May 25, 2026

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/26395260493

Artifacts expire in 7 days.
Artifacts:

  • pr-497-0dbd127ba8c4008fb7350ccba8bd1724cd3bf302-tauri-macos
  • pr-497-0dbd127ba8c4008fb7350ccba8bd1724cd3bf302-electron-macos
  • pr-497-0dbd127ba8c4008fb7350ccba8bd1724cd3bf302-tauri-linux
  • pr-497-0dbd127ba8c4008fb7350ccba8bd1724cd3bf302-tauri-windows
  • pr-497-0dbd127ba8c4008fb7350ccba8bd1724cd3bf302-tauri-macos-arm64
  • pr-497-0dbd127ba8c4008fb7350ccba8bd1724cd3bf302-electron-windows
  • pr-497-0dbd127ba8c4008fb7350ccba8bd1724cd3bf302-electron-linux

@OfflinePing OfflinePing changed the title Persist YOLO mode and inherit in subagents Share YOLO mode across session families May 25, 2026
Keep incomplete session-family chains rooted at the last loaded session so child toggles do not write to a missing parent key. Add focused resolver coverage for missing parents, loaded chains, and fork roots.

Re-drain queued child permissions after fetched or updated session metadata establishes a parent relationship, covering permissions that arrived before the child session was hydrated.
@github-actions
Copy link
Copy Markdown

ghost commented May 25, 2026

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/26396470458

Artifacts expire in 7 days.
Artifacts: (none found on this run)

@github-actions
Copy link
Copy Markdown

ghost commented May 25, 2026

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/26396481956

Artifacts expire in 7 days.
Artifacts:

  • pr-497-5ac9f25229505fbe851d340095e61dfedd6f0ed9-tauri-macos
  • pr-497-5ac9f25229505fbe851d340095e61dfedd6f0ed9-electron-macos
  • pr-497-5ac9f25229505fbe851d340095e61dfedd6f0ed9-tauri-linux
  • pr-497-5ac9f25229505fbe851d340095e61dfedd6f0ed9-tauri-windows
  • pr-497-5ac9f25229505fbe851d340095e61dfedd6f0ed9-tauri-macos-arm64
  • pr-497-5ac9f25229505fbe851d340095e61dfedd6f0ed9-electron-windows
  • pr-497-5ac9f25229505fbe851d340095e61dfedd6f0ed9-electron-linux

@pascalandr
Copy link
Copy Markdown
Contributor

@CodeNomadBot
same review

@codenomadbot
Copy link
Copy Markdown
Contributor

codenomadbot Bot commented May 25, 2026

Request understood: I reviewed the latest pushed PR state after @CodeNomadBot same review as a gatekeeper for regressions, implementation quality, and diff scope. Assumption: this is review-only; I did not modify the branch.

Gatekeeper Result

No blocking findings in the latest diff.

The two prior blockers are addressed in the current 5ac9f25 state:

  • packages/ui/src/stores/permission-auto-accept.ts:22-38 now keeps the last known session as the root when the parent chain is incomplete, so a loaded child no longer resolves to an unloaded parent id.
  • packages/ui/src/stores/session-events.ts:223-260 and packages/ui/src/stores/session-events.ts:497-552 now re-drain queued child permissions after fetched/SSE session metadata establishes a parentId, covering the permission-before-child-metadata race that previously left permissions pending.

The implementation is also back to the right size and shape: 4 files changed, 111 insertions(+), 3 deletions(-), no server-backed persistence, no migration machinery, and existing localStorage behavior is preserved.

Notes

  • Fork/revert handling is preserved by treating sessions with revert metadata as their own auto-accept root in resolvePermissionAutoAcceptFamilyRoot.
  • The new resolver tests cover missing parents, full parent chains, and fork roots. I would still prefer an end-to-end queue drain test for the permission-before-session-metadata case, but the production drain path is now present and small enough that I would not block on the missing test.
  • Residual edge case to keep in mind: if a child is known with an unloaded parent, a queued permission intentionally stays keyed to the child until the full chain is available. The current fix drains when child metadata arrives/updates, not when an unrelated parent later appears. Given initial hydration fetches sessions before pending permissions and the main live race is child metadata arriving after the permission, I do not consider this a merge blocker for this diff.

Validation

  • git diff --check origin/dev...HEAD: passed.
  • npx tsx --test packages/ui/src/stores/permission-auto-accept.test.ts: could not run in this bot worktree because solid-js is missing from the local dependency install.
  • npm run typecheck --workspace @codenomad/ui: could not complete in this bot worktree due broad missing dependency/type resolution issues (solid-js, SDK/Tauri modules, Node test types), matching prior environment failures rather than a PR-specific signal.

Verdict

Approve from the gatekeeper perspective. CI with installed dependencies still needs to be the source of truth for typecheck/test execution.

--
Yours,
CodeNomadBot

@github-actions
Copy link
Copy Markdown

ghost commented May 25, 2026

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/26409332817

Artifacts expire in 7 days.
Artifacts:

  • pr-497-7c76918e6e5f1f309af922b80ec961946389e595-tauri-macos
  • pr-497-7c76918e6e5f1f309af922b80ec961946389e595-tauri-linux
  • pr-497-7c76918e6e5f1f309af922b80ec961946389e595-tauri-windows
  • pr-497-7c76918e6e5f1f309af922b80ec961946389e595-electron-macos
  • pr-497-7c76918e6e5f1f309af922b80ec961946389e595-tauri-macos-arm64
  • pr-497-7c76918e6e5f1f309af922b80ec961946389e595-electron-windows
  • pr-497-7c76918e6e5f1f309af922b80ec961946389e595-electron-linux

Copy link
Copy Markdown
Contributor

@pascalandr pascalandr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current code drains only the updated session, this can miss nested/out-of-order cases where a queued grandchild permission becomes eligible only after its parent metadata loads.

Suggested code change:

  1. Replace/export a broader helper in instances.ts:
function drainAutoAcceptPermissionsForInstance(instanceId: string): void {
  drainAutoAcceptPermissions(
    instanceId,
    getPermissionQueue(instanceId),
    sendPermissionResponse,
    hasPendingPermission,
  )
}
  1. In session-events.ts, after session metadata fetch/update establishes parentId, call:
drainAutoAcceptPermissionsForInstance(instanceId)

instead of:

drainAutoAcceptPermissionsForSession(instanceId, sessionId)

This should cover direct children, nested subagents, and out-of-order metadata hydration.

Replace the session-specific auto-accept drain with an instance-wide queue drain when session metadata establishes a parent relationship. This covers nested and out-of-order subagent hydration where a queued grandchild permission only becomes eligible after an intermediate parent is loaded.
@OfflinePing OfflinePing requested a review from pascalandr May 25, 2026 19:52
@github-actions
Copy link
Copy Markdown

ghost commented May 25, 2026

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/26417296971

Artifacts expire in 7 days.
Artifacts:

  • pr-497-f050c540b53b9107b6ae745b0e4c14aab4b26ccd-electron-windows
  • pr-497-f050c540b53b9107b6ae745b0e4c14aab4b26ccd-electron-linux

@github-actions
Copy link
Copy Markdown

ghost commented May 25, 2026

PR builds are available as GitHub Actions artifacts:

https://github.com/NeuralNomadsAI/CodeNomad/actions/runs/26417505776

Artifacts expire in 7 days.
Artifacts:

  • pr-497-d08e7620057cf5b8fbe7e735195a34f09601a0aa-tauri-macos
  • pr-497-d08e7620057cf5b8fbe7e735195a34f09601a0aa-tauri-linux
  • pr-497-d08e7620057cf5b8fbe7e735195a34f09601a0aa-tauri-windows
  • pr-497-d08e7620057cf5b8fbe7e735195a34f09601a0aa-electron-macos
  • pr-497-d08e7620057cf5b8fbe7e735195a34f09601a0aa-tauri-macos-arm64
  • pr-497-d08e7620057cf5b8fbe7e735195a34f09601a0aa-electron-windows
  • pr-497-d08e7620057cf5b8fbe7e735195a34f09601a0aa-electron-linux

@pascalandr pascalandr merged commit 8c0afa6 into NeuralNomadsAI:dev May 25, 2026
11 checks passed
@pascalandr
Copy link
Copy Markdown
Contributor

Thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants