Skip to content

refactor(core): drop retired execute permission mode from capability audit - #3603

Merged
Astro-Han merged 2 commits into
apache:mainfrom
yunaremaia:refactor/drop-execute-permission-mode
Aug 23, 2026
Merged

refactor(core): drop retired execute permission mode from capability audit#3603
Astro-Han merged 2 commits into
apache:mainfrom
yunaremaia:refactor/drop-execute-permission-mode

Conversation

@yunaremaia

Copy link
Copy Markdown
Contributor

Summary

Removes the retired execute permission mode from CAPABILITY_AUDIT_PERMISSION_MODES and simplifies the capability audit surface, completing the cleanup described in #3385.

What changed

  • CAPABILITY_AUDIT_PERMISSION_MODES: removed execute — now [explore, ask] (matching PERMISSION_MODES in permission.ts, which already dropped it).
  • SkillAuditRecord.permissionMode: simplified from Exclude<CapabilityAuditPermissionMode, 'execute') to CapabilityAuditPermissionMode — the Exclude is no longer needed.
  • scheduledTaskPermissionMode(): active tasks now map to ask instead of the retired execute (behavioural equivalent; the runtime already folded executeask at all persistence sites).
  • executableScheduledTaskCount: filters enabled && permissionMode !== 'explore' instead of the now-unreachable permissionMode === 'execute' literal.
  • CLI activation-command.ts: retained as-is — the execute → ask alias serves external callers and is already documented with an explanatory comment.

What this does NOT change

The execute mode is already retired from PERMISSION_MODES, all persistence decode sites (decodePersistedPermissionMode in permission.ts), the protocol epoch, and the desktop/CLI UI surfaces. This PR removes the last audit-side reference.

Scope note

The useNewTaskChoice shadow state and resolveCreateSessionInput mentioned in #3385 appear to have been removed in prior work — they no longer exist in the current main. The remaining cleanup was the capability audit side handled here.

Testing

  • Existing tests in scheduled-task.test.ts, tool-result-record-schema.test.ts, and tool-result-preview.test.ts already exercise the retired-mode folding (execute → ask on persisted records) and rejection (execute on live wire). This refactor does not change that behaviour.
  • CI typecheck and test will validate; I was unable to run npm install locally (blocked by @xterm/xterm registry fetch failure on this host) so I did not run the full test suite. This is disclosed transparently.

AI disclosure

OpenAI Codex assisted with codebase analysis and this implementation. I reviewed the diff and take responsibility for the contribution.

Signed-off-by: Yunare Maia yunare@gmail.com

…y audit

Remove the 'execute' member from CAPABILITY_AUDIT_PERMISSION_MODES
and the associated Exclude<> workaround in SkillAuditRecord, since the
retired mode is already folded to 'ask' at all persistence decode sites.

- scheduledTaskPermissionMode: active tasks now map to 'ask' instead
  of the retired 'execute' (behavioural equivalent).
- executableScheduledTaskCount: filters enabled non-explore tasks instead
  of the now-unreachable 'execute' permissionMode literal.
- CLI activation-command retains the execute→ask alias for external
  callers per the original comment.

Refs: apache#3385

Generated-by: OpenAI Codex
Signed-off-by: Yunare Maia <yunare@gmail.com>

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed at exact head ec26f69bf40c6d98c0bd619ae62505c3247ad84f. No [P0][P3].

The change to watch here is executableScheduledTaskCount, because the predicate is rewritten rather than merely narrowed:

// before
scheduledTasks.filter((task) => task.permissionMode === 'execute')
// after
scheduledTasks.filter((task) => task.enabled && task.permissionMode !== 'explore')

Those select the same set, and it's worth writing down why, since the two expressions look nothing alike. enabled is derived as task.status === 'active' (scheduledTaskToAuditRecord), and explore is returned exactly for completed and expired. So the new predicate is active && not (completed | expired), which reduces to active. Under the old code, execute was returned for everything that was neither completed/expired nor paused — also active. Same membership, no drift in the reported count.

I checked specifically whether paused tasks could slip into the new count, since they map to ask and ask !== 'explore'. They can't: paused is not active, so enabled is false and the first conjunct rejects them.

The claim in the new comment — that the retired execute mode folded to ask identically — is consistent with execute having already been removed as a permission mode at the protocol layer, so no live caller can still be distinguishing the two.

The Exclude<CapabilityAuditPermissionMode, 'execute'> on SkillAuditRecord.permissionMode becomes redundant once execute leaves the union, and dropping it is correct rather than a widening: the field's producer at :146 only ever emits 'ask' or 'explore'.

CI note: this head's workflow run sat at action_required because it comes from a fork; I approved the run so it could execute. Gate conclusions should be drawn from that run's terminal state, not from this comment.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed at exact head 0b1d82097f175df3ada48ea830efdeff49306e3e (biome-formatting follow-up to ec26f69bf4; the delta is line-wrapping only). No [P0][P3].

What this pass verified beyond the earlier review:

  • No remaining execute consumers in the audit surface. CAPABILITY_AUDIT_PERMISSION_MODES / CapabilityAuditPermissionMode are referenced only inside capability-audit.ts. The report's UI consumers (capability-audit-strip.tsx, module-pages.tsx, skills-panel.tsx) read neither permissionMode nor executableScheduledTaskCount, so nothing downstream still branches on the dropped value.
  • No old-data read path can crash. The audit report is derived live (deriveCapabilityAuditReport over in-memory ScheduledTasks) and is never persisted under this enum. Persisted task templates go through the pre-existing decodePersistedScheduledTask fold (scheduled-task.ts), which maps a stored execute to ask via decodePersistedPermissionMode (permission.ts) — untouched by this PR.
  • The folding claim is backed in-repo. RETIRED_PERMISSION_MODES in packages/core/src/permission.ts documents that execute compiled to the same profile as ask, which is exactly what the new comment asserts.
  • executableScheduledTaskCount membership re-derived independently. Over SCHEDULED_TASK_STATUSES = active | paused | completed | expired: old predicate (permissionMode === 'execute') selected active only; new predicate (enabled && permissionMode !== 'explore') also reduces to active (paused fails enabled). Same count, and paused cannot slip in.

Gate: test is the only path-filtered check this change can trigger (no package.json/lockfile change, no sandbox/filesystem-worker paths) and it is terminal green on this exact head.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed on exact head 0b1d82097f175df3ada48ea830efdeff49306e3e. No findings.

Retiring a permission mode has two ways to go wrong, and both were checked rather than assumed:

Leftover consumers. 'execute' has no remaining consumer on the audit surface — the enum is referenced only within its own file, and the UI consumers read neither permissionMode nor executableScheduledTaskCount.

Old configs crashing on read. There is no such path here. The audit report is computed live from in-memory ScheduledTask values and never persisted, and persisted templates go through decodePersistedScheduledTask, which this PR does not touch and which still folds executeask via RETIRED_PERMISSION_MODES in permission.ts. So durable data written before this change still decodes.

The executableScheduledTaskCount predicate was also reduced independently on both sides and comes out the same: status === 'active', with paused excluded either way.

Gating: only test is triggered on this head — no package.json/lockfile changes and no sandbox paths, so audit and the Windows sandbox lane are correctly absent rather than missing. test is terminal success on the exact head.

@Astro-Han
Astro-Han merged commit 0df60ed into apache:main Aug 23, 2026
1 check passed
mikemikimike pushed a commit to mikemikimike/maka that referenced this pull request Aug 23, 2026
…audit (apache#3603)

* refactor(core): drop retired 'execute' permission mode from capability audit

Remove the 'execute' member from CAPABILITY_AUDIT_PERMISSION_MODES
and the associated Exclude<> workaround in SkillAuditRecord, since the
retired mode is already folded to 'ask' at all persistence decode sites.

- scheduledTaskPermissionMode: active tasks now map to 'ask' instead
  of the retired 'execute' (behavioural equivalent).
- executableScheduledTaskCount: filters enabled non-explore tasks instead
  of the now-unreachable 'execute' permissionMode literal.
- CLI activation-command retains the execute→ask alias for external
  callers per the original comment.

Refs: apache#3385

Generated-by: OpenAI Codex
Signed-off-by: Yunare Maia <yunare@gmail.com>

* style: apply biome formatting to capability-audit (CI fix)

---------

Signed-off-by: Yunare Maia <yunare@gmail.com>
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