Skip to content

Share one predicate between isSingleShellCommand and shellApprovalScopes - #403

Merged
TheGreatAxios merged 4 commits into
mainfrom
cl-5677-issingleshellcommand-and-shellapprovalscopes-redefine-is
Aug 8, 2026
Merged

Share one predicate between isSingleShellCommand and shellApprovalScopes#403
TheGreatAxios merged 4 commits into
mainfrom
cl-5677-issingleshellcommand-and-shellapprovalscopes-redefine-is

Conversation

@TheGreatAxios

Copy link
Copy Markdown
Collaborator

Summary

isSingleShellCommand (src/permission/gate.ts:37-42, gating preApprove) and shellApprovalScopes (src/permission/classify.ts:391-401, gating the interactive approval scope ladder) each independently combined splitChainedCommand/tokenize to answer "is this a single shell command, or a chain." They disagreed on a comment-trailing command like echo hi && # why: shellApprovalScopes filters comment-only segments before counting (treats it as one command), while isSingleShellCommand did not (treats it as a two-segment chain).

isSingleShellCommand is now a single command, both offer the full per-command scope ladder consistently. Exported from classify.ts, built on the same comment-filtered segment list shellApprovalScopes derives its ladder from; gate.ts imports it instead of maintaining an independent copy.

Test plan

  • RED: added a test in src/permission/permission.test.ts ("agrees with the interactive scope ladder on whether a comment-trailing command is single") that failed before the fix, proving the two predicates disagreed on echo hi && # why.
  • bun run typecheck
  • bun run build
  • bun run test — 4171 pass, 1 pre-existing unrelated failure (src/agent/lsp-availability.test.ts, an environment check for typescript-language-server availability, untouched by this change)

@linear-code

linear-code Bot commented Aug 8, 2026

Copy link
Copy Markdown

CL-5677

evaluateApprovals, isRequestCoveredByGrant, and hasExactFullCommandGrant
each reimplemented the same tool/providerModel/cwd scoping condition
independently (src/permission/authz-grants.ts, src/permission/gate.ts),
so a scoping-dimension change required editing three sites in lockstep.
A fourth copy, matcher.ts's isApproved, was dead in production and only
exercised by its own test, and lacked the specificity-ranking behavior
evaluateApprovals gets from @intx/authz's evaluateGrants.

All three live call sites now delegate to a single exported
grantScopeMatches predicate. The dead isApproved matcher and its tests
are removed.
…ectly

The other two live call sites (evaluateApprovals, isRequestCoveredByGrant)
get a direct cross-check against grantScopeMatches; hasExactFullCommandGrant
isn't exported, so it only had indirect coverage through evaluate()'s
multi-segment replay path elsewhere in the suite. Drive that path directly
with grants grantScopeMatches would refuse (wrong cwd, wrong providerModel)
to confirm the replay never fires when the shared predicate says no.
Both answered "is this a single shell command, or a chain" by
independently combining splitChainedCommand/tokenize, so a
segmenting-rule change to one silently didn't reach the other. They
disagreed on a comment-trailing command like "echo hi && # why":
shellApprovalScopes filtered the comment-only segment before counting
(treating it as one command), while gate.ts's isSingleShellCommand did
not (treating it as a two-segment chain).

isSingleShellCommand now lives in classify.ts, built on the same
comment-filtered segment list shellApprovalScopes derives its ladder
from, and gate.ts imports it instead of maintaining its own copy.
grantScopeMatches consolidation's sibling PR fixed a live disagreement
on comment-trailing chains, but the shared realShellSegments predicate
also silently narrows pure-comment input from single-command to
not-a-command, and treats a leading-comment-then-chain by its trailing
real segment only. Neither was covered by a test, so a future
segmenting change could flip either back with nothing catching it.
@TheGreatAxios
TheGreatAxios force-pushed the cl-5677-issingleshellcommand-and-shellapprovalscopes-redefine-is branch from be7b1dc to b7e7388 Compare August 8, 2026 19:27
@TheGreatAxios
TheGreatAxios merged commit abc8176 into main Aug 8, 2026
3 checks passed
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.

1 participant