9.1 KiB
issue, issue_title
| issue | issue_title |
|---|---|
| 526 | pi-permission-system: move yolo into recorded authority (composition-stage ask→allow rewrite) |
Retro: #526 — Move yolo into recorded authority (composition-stage ask→allow rewrite)
Stage: Planning (2026-07-05T00:00:00Z)
Session summary
Planned Phase 8 Step 2: relocate yolo mode from the prompt path into a composition-stage ask→allow rewrite over the composed ruleset, tagged origin: "yolo".
The rewrite lands in PermissionManager.check (post-cache, behind an injected () => boolean reader), with deriveResolution + a GateRunner yolo fast-path preserving auto_approved review-log/decision-event parity.
Produced a 4-step TDD plan (rule helper → manager rewrite + wiring → resolution/runner → docs) and a retro breadcrumb.
Observations
- Post-cache, not cache-key.
The
resolvedPermissionsCacheis keyed byagentName+ loader stamp only. Applying the rewrite insidecheck()overfullRules(rather than inresolvePermissions) keepsgetComposedConfigRules/getToolPermissionyolo-free, satisfying the display-unchanged goal without touching the cache key. The synthesized universal*/*default is part ofcomposedRules, so an unmatched surface is covered automatically. - Single runner choke point.
Both
ToolCallGatePipelineandSkillInputGatePipelineroute throughGateRunner.run, so one yolo fast-path (check.origin === "yolo") covers all gated surfaces. Mirrors the existing session-hit fast-path. - Prompter-arm reachability verified.
Traced every
ask-producing path: tool/bash/mcp/path/external_directory and skill-input all resolve viamanager.check(yolo-rewritten); the skill-readpreResolvedstate comes from the yolo-aware skill sanitizer, so it is alreadyallowunder yolo. Noaskreaches the prompter under yolo — confirming [#527] can safely delete the arm. - Two observable-output forks surfaced to the operator via
ask_user. (1) Review-log entry shape — chose the runner'slogContextconvention (toolCallId, notrequestId) over reconstructing the prompter's exact fields. (2) Skill-read under yolo — acceptedpolicy_allow/origin: "builtin"(the sanitizer already resolves it to allow) rather than threadingorigin: "yolo"through theSkillPromptEntry→preResolvedchain. - Batch tail deferral.
Batch "yolo-recorded-authority" (Steps 2, 3; tail = [#527]).
Plan marker is
mid-batch — defer; the release-please PR stays open until [#527] lands. - Doc-sync traps noted.
The architecture doc inline-copies
RuleOrigin/Rule(must add"yolo"); the "yolo checks on the ask path" health-metric row is not flipped in this step (arms removed only in [#527]);permission-prompter.mdupdate rides with [#527] since the arm still exists. NoREADME.mdcommand-surface change. - ADR-0002 boundary preserved.
rewriteAsksToYolois a string-onlyRulesettransform inrule.ts; the manager imports it without breaching theno-restricted-importsAccessPathguard.
Stage: Implementation — TDD (2026-07-05T15:45:00Z)
Session summary
Executed all 4 planned TDD cycles for the yolo composition-stage rewrite: (1) rewriteAsksToYolo + "yolo" RuleOrigin in rule.ts; (2) the post-cache rewrite in PermissionManager.check behind an injected isYoloEnabled reader, wired in index.ts; (3) deriveResolution allow+autoApproved→auto_approved plus a GateRunner yolo fast-path; (4) architecture-doc sync (inline RuleOrigin listing, Step 2 ✅ on heading + S2 Mermaid node, Landed: note).
Test count moved +16 (2283 → 2299); full suite, pnpm run check, root pnpm run lint, and pnpm fallow dead-code all green.
Observations
- Plan deviation (simplification).
The plan suggested threading
isYoloEnabledthroughtest/helpers/manager-harness.ts; instead the newtest/permission-manager-yolo.test.tsconstructsPermissionManagerdirectly with the already-exportedcreateInMemoryPolicyLoader+ an injectedisYoloEnabledclosure. Narrower, no harness surface added. The reviewer confirmed this is a clean simplification, not a gap. - Lint caught unnecessary optional chains in the first Red.
@typescript-eslint/no-unnecessary-conditionfired onrewritten?.after aconst [rewritten] = ...destructure (element type is non-nullish withnoUncheckedIndexedAccessoff); array-indexresult[0]?.was not flagged. Fixed the destructured accesses to plain member access before committing. - Pre-completion reviewer: PASS — ready for
/ship-issue. Deterministic checks green; cross-step invariants (#478 singlecheckentry point, #506/ADR-0002 string boundary, #525 manager-harness fixtures, display-unchanged) all verified.mmdcrendered the modifiedarchitecture.mdcharts including theS2✅ node. No WARN findings. - Ship-time note. Release is mid-batch — defer (batch "yolo-recorded-authority", tail = [#527]); the release-please PR stays open until [#527] lands. Confirm at ship time.
- Skill-read parity nuance held as designed.
No code path change was needed for skill-reads: the yolo-aware sanitizer already resolves a skill's state to
allowbefore the gate, so a skill-read auto-allows and logspolicy_allow/origin: "builtin"(accepted, operator-confirmed).
Stage: Final Retrospective (2026-07-05T23:38:59Z)
Session summary
Planned, TDD-implemented, and shipped the yolo composition-stage ask→allow rewrite (Phase 8 Step 2) across three stages in one continuous session.
Four TDD cycles landed (+16 tests, 2283→2299), the pre-completion-reviewer returned PASS first try, and the change was pushed with green CI; the release was deferred per the plan's mid-batch **Release:** marker (batch tail = [#527]).
Observations
What went well
- Plan-time
ask_useron output-shape forks prevented rework. Two genuine parity ambiguities the issue's "parity holds" wording left open — theauto_approvedreview-log entry shape (runnerlogContextvs. prompterpromptDetails) and skill-read reporting (policy_allowvs.auto_approved) — were resolved with the operator at plan time. Both could have surfaced as a pre-completion WARN or post-ship surprise; resolving them up front produced a first-try PASS and zero TDD rework. - Exhaustive reachability trace during planning.
The plan verified that every
ask-producing path (tool/bash/mcp/path/external_directory/skill-input viamanager.check, and skill-readpreResolvedvia the yolo-aware sanitizer) resolves toallowunder yolo, so the prompter arm becomes provably unreachable. This de-risked [#527] and meant the TDD had no runtime surprises. - Validated plan deviation — simpler than planned.
Skipped the planned
test/helpers/manager-harness.tsextension and constructed the manager directly with the already-exportedcreateInMemoryPolicyLoader+ an injectedisYoloEnabledclosure; the reviewer confirmed this is a clean simplification, not a coverage gap. - Deterministic release decision.
The plan's grep-able
**Release:**marker made the ship-time defer a single crispask_user, sourced from the plan rather than inferred from prose.
What caused friction (agent side)
other— the first TDD-Step-1 commit was rejected by the pre-commit lint hook: the new test usedrewritten?.fieldafter aconst [rewritten] = rewriteAsksToYolo(...)destructure, which@typescript-eslint/no-unnecessary-conditionrejects (the destructured element type is non-nullish). Notably, array-index access (result[0]?.field) in the same file was not flagged — an asymmetry that made the failure non-obvious. Impact: one fix + re-run cycle (~2 tool calls); no logic rework, deterministically caught by the hook before the commit landed.
What caused friction (user side)
- None. The two plan-time forks and the ship-time defer were clean, well-scoped decisions with no earlier-context opportunities missed.
Diagnostic details
- Model-performance correlation — the
deepseek-v4-flashandclaude-fable-5model_changeentries had no assistant turns under them (verified by interleavingmessage+model_changeviaread_session); they were transient selections that never ran, not a lightweight-model-on-judgment mismatch. Ship ran onclaude-sonnet-5, retro onclaude-opus-4-8, and thepre-completion-reviewersubagent ran on its configured model for judgment-heavy review — all appropriate. - Feedback-loop gap analysis —
pnpm run checkran incrementally after Steps 2 and 3 (the interface/type-changing steps), alongside the per-commit lint hook, with the full check/lint/test/fallow suite at the end. Incremental, no end-only verification gap. - Escalation-delay / unused-tool — no rabbit-holes; no error sequence exceeded one fix cycle, and no subagent/tool was needed but skipped.
Changes made
- Appended this Final Retrospective stage entry to
packages/pi-permission-system/docs/retro/0526-yolo-recorded-authority.md. NoAGENTS.md, skill, or prompt edits — the operator confirmed retro-file-only (the single friction point was deterministically caught by the pre-commit lint hook, below the bar for a rule change).