Skip to content

Control automatic handoff availability#3

Open
grzegorznowak wants to merge 18 commits into
agenticoding:mainfrom
grzegorznowak:story-03-handoff-resume-control
Open

Control automatic handoff availability#3
grzegorznowak wants to merge 18 commits into
agenticoding:mainfrom
grzegorznowak:story-03-handoff-resume-control

Conversation

@grzegorznowak
Copy link
Copy Markdown
Collaborator

@grzegorznowak grzegorznowak commented May 24, 2026

Summary

This PR adds handoff.automaticEnabled to control automatic agent-initiated handoff while preserving explicit operator /handoff <direction> as the manual path.

The implementation now uses guard-based semantics: the handoff tool remains registered, prompt/watchdog guidance is suppressed when automatic handoff is disabled, and direct tool calls are rejected unless they are satisfying an active manual /handoff request.

Requirements

  • Default automatic handoff remains enabled when the setting is absent.
  • handoff.automaticEnabled=false suppresses agent handoff-call guidance during normal turns.
  • Direct handoff tool execution while automatic handoff is disabled is blocked unless an explicit /handoff <direction> request is active.
  • Explicit /handoff <direction> works when automatic handoff is disabled, including while the assistant is busy.
  • Manual requests are activated only when the generated /handoff user message actually starts, preventing old-turn stale tool calls from bypassing the guard.
  • A requested /handoff remains valid across intermediate notebook/tool provider turns before the model finally calls handoff.
  • /agenticoding-settings persists the global raw JSON boolean and warns when project settings override it.

Acceptance criteria

  • Reads handoff.automaticEnabled from global ~/.pi/agent/settings.json plus project <cwd>/.pi/settings.json, with project override and own nested keys only.
  • Missing files or absent key default to automaticEnabled: true.
  • Unsupported non-boolean values, invalid JSON, non-object roots, and non-ENOENT read failures fail closed to disabled with warning diagnostics.
  • Automatic-disabled normal turns avoid telling the model to call handoff or recommend /handoff.
  • Manual /handoff <direction> records a pending operator request, sends/queues the handoff prompt, routes through normal compaction, and clears state after success, error, or stale agent completion.
  • Busy manual /handoff <direction> queues the generated prompt as a follow-up instead of waiting inside the command handler.
  • Direct stale tool execution while automatic is disabled and no active manual request exists refuses before compaction and returns a diagnostic.
  • /agenticoding-settings writes only global nested handoff.automaticEnabled as a real JSON boolean, preserves unrelated settings, blocks invalid global writes, and allows global saves despite invalid project JSON with warning.
  • README and CHANGELOG document the new key, default, disabled/manual behavior, guard-based enforcement, and TUI/global-only/project override behavior.

Contract changes

  • Adds boolean handoff.automaticEnabled for automatic agent-initiated handoff availability.
  • Keeps /agenticoding-settings as the dedicated extension-owned settings surface.
  • Keeps /handoff <direction> focused on manual handoff initiation, not configuration.

Out of scope

  • Changing Pi core compaction behavior.
  • Disabling explicit operator /handoff <direction>.
  • Adding /handoff --enable, /handoff --disable, /handoff --wait, /handoff --proceed, or direct tool-call override parameters.
  • Adding project-scope writes to the settings TUI.

How to verify

Run from this branch/worktree:

node --loader /tmp/pi-agenticoding-test-loader.mjs --test \
  --test-name-pattern 'manual slash handoff|agent_end fallback|turn_end fallback|handoff compaction error' \
  agenticoding.test.ts

node --loader /tmp/pi-agenticoding-test-loader.mjs --test agenticoding.test.ts

Local verification passed:

  • Focused manual-handoff/stale-cleanup suite: 8/8
  • Full suite: 151/151
  • git diff --check

Live verification:

  • With project-local handoff.automaticEnabled=false, /handoff to discuss the importance of saying hello successfully compacted after the model used notebook tools before calling handoff.
Captura de pantalla_2026-05-28_12-31-26 Captura de pantalla_2026-05-28_12-41-57

@grzegorznowak grzegorznowak marked this pull request as draft May 24, 2026 17:29
@grzegorznowak
Copy link
Copy Markdown
Collaborator Author

Summary: The PR implements the requested handoff wait/proceed behavior and settings UI, but the raw settings merge can treat prototype/meta JSON keys as real handoff.resumeBehavior, which can violate the new safe default.

Reviewed groups sequentially: settings resolver/UI, handoff integration, then tests/docs.

Business / Product Assessment

Verdict: REQUEST CHANGES

Strengths

  • The default-wait, explicit-proceed, invalid JSON fallback, unsupported value fallback, and project override cases are covered by focused tests. Sources: agenticoding.test.ts:394, agenticoding.test.ts:436, agenticoding.test.ts:446, agenticoding.test.ts:459
  • User-facing docs describe the new default, opt-in proceed, global-only TUI save behavior, and project override semantics. Sources: README.md:43, README.md:52, README.md:129

In Scope Issues

  • Settings JSON can omit an own handoff.resumeBehavior but still resolve inherited prototype data as proceed. Sources: settings.ts:62, settings.ts:75, settings.ts:138

    Medium severity · Low likelihood

    Why: The acceptance criteria require missing handoff.resumeBehavior to resolve to wait. Because the merge target is a normal object and extraction reads inherited properties, a raw settings file containing a __proto__ object can make the resolver see handoff.resumeBehavior: "proceed" even though the real nested setting is absent. That can unexpectedly auto-send Proceed. after compaction.

    Assumptions / Preconditions: A global or project settings file contains JSON like { "__proto__": { "handoff": { "resumeBehavior": "proceed" } } } and no own top-level handoff.resumeBehavior.

    Downgrade Factors: This requires malformed or unusual local settings JSON; normal documented settings are handled correctly.

    Code Trail: readSettingsSource parses arbitrary raw JSON into a plain object, mergeSettings writes every key into a normal object, including __proto__, and extractResumeBehavior then reads settings.handoff without requiring an own property. The handoff resolver trusts that extracted value and returns "proceed" when it matches a supported value.

    Reproduction: Put the JSON above in ~/.pi/agent/settings.json or <project>/.pi/settings.json; the file has no own handoff.resumeBehavior, but the merged object exposes inherited handoff.resumeBehavior and can opt into auto-resume.

Out of Scope Issues

  • None.

Technical Assessment

Verdict: REQUEST CHANGES

Input Boundary Shape Risk Assessment

Status: Triggered
Boundary: Raw global/project Pi settings JSON -> nested merge and handoff.resumeBehavior resolver
Evidence / mitigation: Invalid JSON and unsupported own values are handled safely, but the boundary does not constrain prototype/meta keys before merging and reading stricter application settings assumptions.

Strengths

  • /handoff initiation and direct tool use share the same runtime behavior because the setting is resolved inside the handoff tool before compaction. Sources: handoff/tool.ts:126, handoff/tool.ts:128, handoff/command.ts:39
  • The settings UI save path preserves unrelated global settings keys while writing only nested handoff.resumeBehavior. Sources: settings.ts:172, settings.ts:182, settings.ts:187

In Scope Issues

  • None.

Out of Scope Issues

  • None.

Reusability

  • The PR keeps settings reading, model construction, display lines, and global write behavior in a separate module that can be reused by both runtime handoff logic and UI tests. Sources: settings.ts:115, settings.ts:192, settings.ts:239, settings.ts:349
  • GitHub reported no checks for this PR branch; I did not run local tests per the review sandbox rules. Sources: agenticoding.test.ts:394

review generated with CURe v. 0.7.3 · single-stage · sha bf278cb · model gpt-5.5/high · tok 602k/8k/610k · session agenticoding-pi-agenticoding-pr3-20260524-190841-2c39 · 4m5s

@grzegorznowak
Copy link
Copy Markdown
Collaborator Author

Summary: The PR largely implements the requested wait-by-default handoff behavior and settings surface, but configuration failure handling still has fail-open paths that can produce unexpected auto-resume or silent save failures.

Business / Product Assessment

Verdict: REQUEST CHANGES

Strengths

  • The default is now wait when handoff.resumeBehavior is absent, and Proceed. is only sent for explicit proceed. Sources: settings.ts:170, handoff/tool.ts:134
  • The settings display includes the resolved value, supported values, default behavior, global/project state, and global-only save note. Sources: settings.ts:272
  • User-facing docs describe the new default, opt-in proceed, TUI command, global save behavior, and project override semantics. Sources: README.md:120

In Scope Issues

  • Unreadable settings sources fail open instead of failing safely to wait Sources: settings.ts:130, settings.ts:157, settings.ts:194

    Medium severity · Low likelihood

    Why: The PR promises configuration problems fall back safely with warnings. Today, any settings read error is treated as “file missing,” so a broken project settings source can be ignored and a global "proceed" value can still auto-resume after compaction.

    Assumptions / Preconditions: A settings path exists but cannot be read, such as EACCES, EISDIR, or a transient filesystem error.

    Downgrade Factors: If Pi guarantees these settings files are always readable when present, impact drops. I did not find that guarantee in the reviewed code.

    Code Trail: handoff/tool.ts resolves resume behavior before registering the compaction callback. readSettingsSource catches all readFile errors and returns exists: false, invalid: false. resolveHandoffResumeBehavior only warns and returns wait for invalid sources, so non-ENOENT read failures are skipped. The write path uses the same “unreadable means missing” policy, which can also lose preservation guarantees if a file is writable but unreadable.

    Reproduction: Configure global settings as { "handoff": { "resumeBehavior": "proceed" } }, then make <cwd>/.pi/settings.json unreadable or a directory. Trigger handoff; the project source is treated as absent, no warning is emitted, and global proceed can send Proceed..

Out of Scope Issues

  • None.

Technical Assessment

Verdict: REQUEST CHANGES

Input Boundary Shape Risk Assessment

Status: Triggered
Boundary: Raw persisted JSON from ~/.pi/agent/settings.json and <cwd>/.pi/settings.json -> strict handoff.resumeBehavior enum and TUI settings model
Evidence / mitigation: JSON is parsed, cloned through own enumerable keys, and enum-validated before use. Prototype-key handling is covered statically. The missing-proof gap is non-JSON read failures, which are currently treated as absent instead of invalid or diagnosable. Sources: settings.ts:138, settings.ts:83, settings.ts:111, agenticoding.test.ts:446

Strengths

  • The settings merge path uses null-prototype objects and own-property access, reducing prototype pollution risk from raw settings JSON. Sources: settings.ts:62, settings.ts:66, settings.ts:83
  • Static tests cover default wait, explicit proceed, project override precedence, invalid JSON, unsupported values, prototype/meta keys, UI fallback, and global-only save behavior. Sources: agenticoding.test.ts:394, agenticoding.test.ts:436, agenticoding.test.ts:596

In Scope Issues

  • Settings TUI save failures can become unhandled async rejections Sources: settings.ts:338, settings.ts:219

    Medium severity · Low likelihood

    Why: A settings save that fails at mkdir or writeFile should produce a controlled user diagnostic and leave the panel in a known state. The current fire-and-forget async handler has no catch path, so filesystem failures can surface as unhandled rejections with no useful TUI feedback.

    Assumptions / Preconditions: The global settings directory or file cannot be created or written.

    Downgrade Factors: If the host framework globally catches rejected promises spawned inside component callbacks and reports them to users, impact is lower. The local code does not show that protection.

    Code Trail: The SettingsList change callback starts a void async IIFE. Inside it, model.save reaches writeGlobalHandoffResumeBehavior, which awaits mkdir and writeFile without catching write failures. A rejection skips buildAgenticodingSettingsModel, refreshSummary, and requestRender.

    Reproduction: Point HOME at a location where .pi/agent/settings.json cannot be written, open /agenticoding-settings, and change the value. The save promise rejects outside the component’s control instead of notifying the user.

Out of Scope Issues

  • None.

Reusability

  • The read/merge/extract helpers create a reasonable foundation for future raw Pi settings keys, though the module is intentionally handoff-specific today. Sources: settings.ts:91, settings.ts:104
  • The model/display/component split keeps persistence separate from the TUI rendering path and should be reusable for additional agenticoding settings. Sources: settings.ts:225, settings.ts:272, settings.ts:301

review generated with CURe v. 0.7.3 · single-stage · sha 85879c3 · model gpt-5.5/high · tok 594k/9k/603k · session agenticoding-pi-agenticoding-pr3-20260524-193138-4af8 · 5m14s

…ave failures

FB-002: readSettingsSource now distinguishes ENOENT (file genuinely
missing -> exists:false) from other read errors like EACCES/EISDIR
(exists:true, invalid:true). The resolveHandoffResumeBehavior function
already handles invalid sources with warnings and fallback to wait.

FB-003: The async IIFE in createAgenticodingSettingsComponent's
SettingsList change callback now wraps the save/rebuild sequence in
try/catch. On failure it calls notify() with an error-level message
instead of silently dropping the rejection as an unhandled promise.

Regression tests:
- non-ENOENT read error test (FB-002): makes global settings file
  unreadable via chmod 000, asserts invalid:true + warning + wait
- write failure test (FB-003): blocks the .pi/agent directory with a
  file, asserts writeGlobalHandoffResumeBehavior rejects with EEXIST
@grzegorznowak
Copy link
Copy Markdown
Collaborator Author

Summary: This PR changes handoff to wait by default, adds opt-in auto-proceed plus /agenticoding-settings, and I found no blocking product or technical issues; gh pr checks reported no checks for the branch.

Business / Product Assessment

Verdict: APPROVE

Strengths

  • Implements the requested default change: absent or invalid handoff.resumeBehavior resolves to wait, while proceed remains available. Sources: settings.ts:164, settings.ts:177
  • The settings UI surfaces resolved value, supported values, default behavior, global/project state, and project override warnings. Sources: settings.ts:287, settings.ts:259
  • User-facing documentation covers the new default, opt-in migration path, TUI command, and override semantics. Sources: README.md:120, README.md:128

In Scope Issues

  • None.

Out of Scope Issues

  • None.

Technical Assessment

Verdict: APPROVE

Input Boundary Shape Risk Assessment

Status: Triggered
Boundary: Raw Pi settings JSON files (~/.pi/agent/settings.json, <cwd>/.pi/settings.json) -> stricter handoff.resumeBehavior enum and settings UI model.
Evidence / mitigation: The code reads and parses both JSON sources, rejects non-object or malformed roots, ignores inherited/prototype settings keys, validates only wait/proceed, falls back to wait, and blocks global writes when existing global JSON cannot be safely read or parsed. Sources: settings.ts:130, settings.ts:142, settings.ts:164, settings.ts:201

Strengths

  • Settings parsing is conservative: malformed JSON, unsupported values, and read errors fail safe to wait rather than auto-resuming. Sources: settings.ts:167, settings.ts:185
  • The handoff tool resolves behavior once before compaction and only sends Proceed. when the resolved value is explicitly proceed. Sources: handoff/tool.ts:128, handoff/tool.ts:133
  • Test coverage statically covers default wait, opt-in proceed, invalid JSON, unsupported values, project overrides, global-only saves, and fallback UI behavior. Sources: agenticoding.test.ts:398, agenticoding.test.ts:440, agenticoding.test.ts:663

In Scope Issues

  • None.

Out of Scope Issues

  • None.

Reusability

  • The settings read/merge/resolve helpers are separated from the TUI and handoff tool, so future agenticoding settings can likely reuse the same shape. Sources: settings.ts:154, settings.ts:236
  • The command registration follows the existing extension wiring pattern and keeps /handoff behavior separate from /agenticoding-settings. Sources: index.ts:53, settings.ts:401

review generated with CURe v. 0.7.3 · single-stage · sha a9a3959 · model gpt-5.5/high · tok 938k/8k/946k · session agenticoding-pi-agenticoding-pr3-20260525-082158-f9e7 · 8m11s

@grzegorznowak grzegorznowak marked this pull request as ready for review May 25, 2026 08:42
@ofriw
Copy link
Copy Markdown
Contributor

ofriw commented May 26, 2026

First review LGTM. Waiting for merge with main.

@grzegorznowak grzegorznowak marked this pull request as draft May 26, 2026 15:14
…me-control

# Conflicts:
#	README.md
#	agenticoding.test.ts
#	handoff/tool.ts
@grzegorznowak
Copy link
Copy Markdown
Collaborator Author

Merged latest origin/main into story-03-handoff-resume-control and resolved conflicts while keeping the PR in draft.\n\nResolution summary:\n- Kept PR #6 notebook/topic terminology and rehydration changes.\n- Preserved story-03 handoff resume control: default wait, optional handoff.resumeBehavior = \"proceed\", and /agenticoding-settings docs/tests.\n- Resolved handoff/tool.ts so the handoff brief uses notebook-on-demand semantics and only sends Proceed. when the resolved setting is proceed.\n\nValidation:\n- node --experimental-strip-types --loader /tmp/pi-agenticoding-test-loader.mjs --test agenticoding.test.ts — 143 pass, 0 fail.\n\nHead: 852a181be688603ae7e361d8b091769cf86adecd. PR remains draft.

@grzegorznowak grzegorznowak changed the title Control handoff auto-resume after compaction Control automatic handoff availability May 27, 2026
@grzegorznowak
Copy link
Copy Markdown
Collaborator Author

Merged current origin/main into story-03-handoff-resume-control and pushed 15afdb0.

Conflict resolution:

  • kept Story 03 manual /handoff flow (idle wait, temporary tool activation, no queued follow-up/deliverAs path);
  • incorporated the main-branch wording “that aligns with the direction above” into the manual handoff prompt and matching test expectation;
  • accepted the auto-merged handoff tool guideline wording “that the next context will need”.

Verification after merge:

  • Focused: 25/25
  • Full: 149/149

@grzegorznowak
Copy link
Copy Markdown
Collaborator Author

Update: restored mainline post-handoff auto-Proceed. behavior per operator direction. New head 140f8b7 changes handoff/tool.ts to call pi.sendUserMessage("Proceed.") on successful compaction, keeps handoff.automaticEnabled as the autonomous-tool availability control, and keeps handoff.resumeBehavior ignored/non-configuring.\n\nVerification from /workspaces/chunkhound_workspace/pi-agenticoding with /tmp/pi-agenticoding-test-loader.mjs:\n- Focused automatic/settings/prompt/active-tools/manual handoff/watchdog pattern: 25/25 pass\n- Full agenticoding.test.ts: 149/149 pass

@grzegorznowak grzegorznowak force-pushed the story-03-handoff-resume-control branch 2 times, most recently from 278063f to 941d055 Compare May 28, 2026 10:28
@grzegorznowak grzegorznowak force-pushed the story-03-handoff-resume-control branch 2 times, most recently from 59d3dcc to 699484c Compare May 28, 2026 12:12
@grzegorznowak grzegorznowak marked this pull request as ready for review May 28, 2026 12:49
@grzegorznowak grzegorznowak force-pushed the story-03-handoff-resume-control branch from 699484c to ff0ab7f Compare May 28, 2026 14:02
@grzegorznowak grzegorznowak force-pushed the story-03-handoff-resume-control branch from ff0ab7f to 5a3ca23 Compare May 28, 2026 15:26
@grzegorznowak grzegorznowak requested a review from ofriw May 28, 2026 15:56
@grzegorznowak
Copy link
Copy Markdown
Collaborator Author

@ofriw Added the tool nudge tweak mentioned on Discord. It seems to be working stable. Please review when you can 🙏🏾

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.

2 participants