Skip to content

feat(commands): add inline overrides for slash commands - #1183

Open
MayurK-cmd wants to merge 2 commits into
Nano-Collective:mainfrom
MayurK-cmd:fix/1151-once-scope-config-override
Open

MayurK-cmd wants to merge 2 commits into
Nano-Collective:mainfrom
MayurK-cmd:fix/1151-once-scope-config-override

Conversation

@MayurK-cmd

@MayurK-cmd MayurK-cmd commented Sep 4, 2026 •

Copy link
Copy Markdown

Description

Closes #1151

Slash commands now accept inline ?key=value overrides that scope a setting to the current command only. Lets users test a value for one command without round-tripping through /settings or otherwise mutating the session state.

/compact ?threshold=80              # session-override key, applied via auto-compact store
/context-max 128k ?once             # session-override key, applied via models.dev store
/compact --mechanical ?preview      # legacy flag (forwarded as --preview)
/some-command ?unknown=1            # ignored; preserved in args so the handler can error normally

How it works

  • parseInlineOverrides(args) (new, pure) splits an args array into {args, overrides}. Only tokens starting with ? are
    considered; bad keys (leading digit, whitespace) are kept in args so the command's own "unknown arg" path runs.
  • expandOverrideArgs(overrides) turns recognised legacy keys (preview, llm, mechanical, aggressive, conservative,
    auto-on, auto-off) into the corresponding --flag value tokens, so existing handlers see them as if the user had typed the
    long form.
  • applyOnceOverrides(overrides) is async and writes recognised session-override keys (threshold, auto-compact, context-max)
    to the existing setAutoCompactThreshold / setAutoCompactEnabled / setSessionContextLimit setters. It returns a restore
    callback that the dispatcher calls in a finally block, so the override only persists for the single command invocation.
  • The slash-command dispatcher in source/app/utils/app-util.ts is wrapped in a try/finally so restore always runs, even if a
    handler throws.

Adding a new inline-override key is a one-line change: add a case in applyOnceOverrides and (if it isn't an existing --flag)
optionally add an entry in LEGACY_FLAG_NAMES.

Why async

applyOnceOverrides defers the heavy auto-compact / models-dev-client imports via dynamic import() so the parser itself is import-free. Without this, the spec timed out under AVA's serial worker because importing @/utils/auto-compact pulls in the chat-handler / config / tokenization init graph.

Files

  • source/utils/inline-overrides.ts (new)
  • source/utils/inline-overrides.spec.ts (new)
  • source/app/utils/app-util.ts (wrap dispatcher in try/finally, rebuild commandParts from parseInlineOverrides +
    expandOverrideArgs)
  • source/app/utils/app-util.spec.ts (4 new dispatcher tests)
  • .changeset/inline-once-overrides.md (minor: new feature)
  • .gitignore (.pnpm-store/)

Out of scope

The pre-existing bash command - queues a completed BashProgress with showOutput test in app-util.spec.ts still fails on this branch (it has been failing since #826 / July 2026). It is unrelated to this change and is not fixed here. Worth a separate issue.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Changeset

  • Added a changeset (pnpm changeset) describing this change for the changelog

.changeset/inline-once-overrides.md is a minor changeset naming @nanocollective/nanocoder and explaining the new ?key=value
syntax. Validated locally with node scripts/validate-changesets.js (72 changesets checked, all resolve).

Docs-only or internal chores need no changeset (or run pnpm changeset --empty to note that intentionally).

Testing

Automated Tests

  • New features include passing tests in .spec.ts/tsx files
  • All existing tests pass (pnpm test:all completes successfully)
  • Tests cover both success and error scenarios

Verified locally:

  • pnpm test:ava source/utils/inline-overrides.spec.ts → 16/16 passed
  • pnpm test:ava source/app/utils/app-util.spec.ts → all 4 new inline-override tests passed; one pre-existing unrelated failure
    (bash command - queues a completed BashProgress with showOutput, present since feat: add session-scoped artifact lifecycle across CLI and VS Code #826) — see "Out of scope" above
  • pnpm test:types → clean
  • pnpm test:lint → 516 files, 0 fixes needed

The 16 parser/helper tests cover: empty input, single key=value, mixed positional + override, bare ?flag (no =), ?key=value
with extra = in value, dotted/dashed/underscored keys, invalid keys preserved in args, expandOverrideArgs for both boolean and key=value forms and unknown keys, plus applyOnceOverrides smoke tests for empty input, unknown keys, unparseable threshold values, and unparseable context-max values.

Manual Testing

  • Tested with Ollama
  • Tested with OpenRouter
  • Tested with OpenAI-compatible API
  • Tested MCP integration (if applicable)

This change is provider-agnostic — the override is wired into the slash-command dispatcher, not into any provider's chat pipeline.
Manual end-to-end testing against a real provider was not done as part of this PR; the change was validated via the unit/
integration tests listed above. The session-override stores it writes to (auto-compact, setSessionContextLimit) are already
covered by their own specs and used in production by /compact --threshold and /context-max <n>.

Checklist

  • If this was for an open issue, I was assigned to it
  • Code follows project style guidelines
  • Self-review completed
  • Documentation updated (if needed)
  • No breaking changes (or clearly documented)
  • Appropriate logging added using structured logging (see CONTRIBUTING.md)

No documentation update included in this PR — the override syntax is a small extension of existing --flag behaviour and the new docblock on applyOnceOverrides documents the public surface. Happy to add a docs page if reviewers want one.

No logging added: the override is purely a session-override passthrough that uses the existing setter functions. A successful
override is observable via the normal command result (e.g. /compact ?threshold=80 prints the same "Auto-compact threshold set to 80%" success message as /compact --threshold 80). Failures (unparseable values) are silently ignored per the same behaviour the existing --threshold parser already follows (the value simply isn't applied).

@github-actions github-actions Bot added the area:tui Terminal UI label Sep 4, 2026
@MayurK-cmd MayurK-cmd changed the title feat(commands): add inline \?key=value\ overrides for slash commands … feat(commands): add inline overrides for slash commands Sep 4, 2026

@will-lamerton will-lamerton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice, clean parser and the try/finally around the dispatcher is the right structural move. Types, lint, changeset validation and the 16 parser specs all pass locally for me. A few correctness issues need fixing before this can go in.

Blocking

1. restore() wipes a pre-existing session override instead of restoring it.

restorations.push(() => setAutoCompactThreshold(null)) resets to the config default, not to whatever was there before. Verified on the branch:

before (user ran /compact --threshold 90):  90
during (/compact ?threshold=80):            80
after restore (expected 90):                null

Same for auto-compact (setAutoCompactEnabled(null)) and context-max (resetSessionContextLimit()). A "temporary" override silently destroys the user's real session setting, which is the opposite of what #1151 asks for. Please read the current value first (autoCompactSessionOverrides.threshold, getSessionContextLimit()) and restore that.

2. ?context-max=128k sets the limit to 128 tokens.

applyOnceOverrides uses Number.parseInt(String(value), 10), while every other caller uses parseContextLimit from source/utils/parse-context-limit.ts, which handles the k suffix. Confirmed: getSessionContextLimit() returns 128. Please use parseContextLimit.

3. Both examples in the PR description do not do what the description says.

threshold is not in LEGACY_FLAG_NAMES, so expandOverrideArgs returns [] (your own test asserts this). /compact ?threshold=80 therefore runs a plain manual compaction, does not print Auto-compact threshold set to 80%, and the temporary threshold has no effect at all because manual /compact never reads it - only the auto-compact path does. And ?once in /context-max 128k ?once is not a recognised key anywhere, so it is a no-op while 128k sets the limit permanently. Either wire these up or correct the description.

4. The override is stripped for ~11 handlers and passed through raw to every other command.

handleBuiltInCommand(message, options) still receives the original message and re-splits it, so every lazy-registry command (/model, /provider, ...) sees the literal ?foo=1 token, and ?preview never becomes --preview for them. handleRetryCommand also re-derives its args from raw message. The "handlers stay oblivious to the override feature" comment only holds for the commandParts handlers.

5. No test covers the feature actually working.

All four applyOnceOverrides tests are t.notThrows(() => restore()) on empty/unknown/unparseable input. None asserts that an override is applied or correctly reverted, which is why issue 1 is invisible to the suite. The app-util additions are described as "4 new dispatcher tests" but none of them invoke handleSlashCommand. Please add a test that sets a prior value, applies an override, restores, and asserts the prior value is back.

Minor

  • UTF-8 BOM (EF BB BF) at the top of source/utils/inline-overrides.ts and .changeset/inline-once-overrides.md. Biome and @changesets/parse both tolerate it, but no other file in the repo has one. Please strip it.
  • Validation asymmetry: /compact --threshold 40 errors with a message, ?threshold=40 silently clamps to 50.
  • .gitignore .pnpm-store/ is unrelated scope creep and is one of the two merge conflicts against main (the other is app-util.ts). Please rebase.
  • New imports sit at line ~1088 mid-file in app-util.spec.ts. Specs are biome-excluded so nothing complains, but it is against the file's convention.
  • typeof arg !== 'string' in parseInlineOverrides is unreachable given the readonly string[] parameter type.
  • The parseInlineOverrides(['? bad']) case cannot occur in practice, the dispatcher splits on /\s+/ so no token can contain a space.

@MayurK-cmd
MayurK-cmd force-pushed the fix/1151-once-scope-config-override branch from 4646207 to 67eddb5 Compare September 7, 2026 10:30

@will-lamerton will-lamerton left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the rework. Items 1, 2, 4 (for built-ins and retry) and 5 are properly fixed, and the test coverage is genuinely solid now. I re-verified on a clean worktree at 22297efb: tsc clean, biome lint and biome format clean, ava on inline-overrides.spec.ts + app-util.spec.ts + auto-compact.spec.ts gives 137 passed / 0 failed (the bash command - queues a completed BashProgress with showOutput failure did not reproduce for me), no conflicts against main. Two things still need fixing.

Blocking

1. knip fails, and it will break the release workflow.

The split into source/utils/auto-compact-session.ts re-exports two symbols nothing consumes:

Unused exports (1)
autoCompactSession  source/utils/auto-compact.ts:26:2
Unused exported types (1)
AutoCompactSessionOverrides  type  source/utils/auto-compact.ts:24:14

main is knip-clean. pnpm test:knip runs in release.yml, not pr-checks, so this merges green and then breaks release on main. Please drop the two unused re-exports.

2. The changeset examples are no-ops (this is my earlier item 3, still open).

autoCompactSessionOverrides.threshold / .enabled are read in exactly two places, maybeAutoCompact (source/utils/auto-compact.ts:89-112) and useAppHandlers.tsx:377-391. Both are the automatic compaction path, which only runs on a chat turn, never during a slash command. Since restoreOnce() fires in the dispatcher's finally, the override is applied and reverted with nothing in between that reads it.

So both examples in .changeset/inline-once-overrides.md do nothing:

  • /compact ?threshold=80 - past the flag loop handleCompactCommand only uses mode, preview and strategy. It never consults the threshold.
  • /context-max 128k ?auto-compact=on - 128k sets the limit permanently, and auto-compact=on is applied then reverted unread.

The one key that genuinely works today is ?context-max on /usage: source/commands/usage.tsx:177 reads getSessionContextLimit() inside the awaited handler, so /usage ?context-max=200k really does render against 200k. That is the example the changeset should lead with. Either wire threshold / auto-compact so some command actually observes them, or narrow the changeset and the PR description to the case that works. As it stands the CHANGELOG would ship two examples that do nothing.

The PR description is also still the original text: it documents ?once (not a recognised key anywhere) and claims /compact ?threshold=80 prints Auto-compact threshold set to 80%. Please bring it in line with the code.

Non-blocking, but worth a decision

  • Custom commands still bypass the feature entirely. handleCustomCommand(message, commandName, options) at source/app/utils/app-util.ts:681 runs before the override parsing with the raw message, so .nanocoder/commands/*.md see ?foo=1 literally. Fine as a deliberate scope call (it is also why free-text custom commands do not silently lose ?word tokens), just say so somewhere.
  • Unknown ?foo=bar keys are now dropped entirely before the handler sees them, so /compact ?threshhold=80 is an invisible no-op. Combined with --threshold 40 erroring while ?threshold=40 is silently skipped, the override path has no error surface at all. Worth at least a "unknown override key" message.
  • expandOverrideArgs([{key: 'preview', value: 'yes'}]) returns ['--preview', 'yes'], leaving a stray yes positional (your own test asserts this). Harmless for /compact, wrong shape for any handler that reads args positionally.
  • import {parseInput} from '@/command-parser' is still out of order in the app-util.spec.ts import block. Cosmetic, biome excludes specs.

The BOMs, the .gitignore creep, the unreachable typeof arg !== 'string' guard and the '? bad' case are all resolved. Fix the two blockers above and I think this is good to go.

@github-actions

Copy link
Copy Markdown
Contributor

Hi @MayurK-cmd, thanks for this PR! It looks like a maintainer has left feedback
or review activity and there are still some outstanding items to wrap up.

Whenever you get a chance, could you take a look at the open comments?
If anything is unclear or you'd like a hand, just reply here and we'll help you get it across the line.

@MayurK-cmd
MayurK-cmd force-pushed the fix/1151-once-scope-config-override branch from 87a63c6 to 2afa9cd Compare September 18, 2026 03:47
@MayurK-cmd

Copy link
Copy Markdown
Author

Rebased onto current main (was 245 behind) and addressed the two blockers. Conflicts in app-util.ts (kept the #1162 MCP-prompt intercept; MCP prompts and custom commands both bypass ? parsing with the raw message) and auto-compact.ts (kept the auto-compact-session.ts split, now using clampThreshold, with auto-compact.ts re-exporting so existing importers are untouched) are resolved.

  1. knip: dropped the unused AutoCompactSessionOverrides type re-export — pnpm exec knip exits 0.
  2. Changeset/docs: narrowed to what actually works. /usage ?context-max=200k is the documented working example; threshold/auto-compact stay as tested apply/restore plumbing but are explicitly marked reserved since no built-in reads them synchronously during dispatch. Also fixed expandOverrideArgs valued booleans (?preview=yes → --preview) and removed the .pnpm-store/.gitignore creep.

Tests: tsc/lint/format/knip clean; inline-overrides.spec 21/21, auto-compact.spec 41/41, new app-util.spec override tests 5/5. The bash … showOutput failure is the pre-existing #826 one, unrelated. Also updated the PR description — the old text still documents ?once and claims /compact ?threshold=80 prints a confirmation, which isn't what the code does.

@akramcodez

Copy link
Copy Markdown
Member

/re-review

@github-actions

Copy link
Copy Markdown
Contributor

nc-review: comments — 2 important, 3 nits

@MayurK-cmd — a few things worth a look, none blocking.

Adds inline ?key=value overrides to slash commands, plumbed through the existing session-override stores (auto-compact threshold/enabled and session context limit) with apply-then-restore semantics via a try/finally in the dispatcher. The parser, the legacy-flag expander, and the once-scoped apply helper are well-tested in isolation; the dispatcher integration test exercises the round-trip end-to-end via /usage. Two important issues: the PR description's example list claims unknown overrides are 'preserved in args so the handler can error normally', but the implementation (and its own docblock) silently drops them — those two stories contradict each other. And the issue's headline example, /compact ?threshold=80, does not actually take effect for the /compact run itself, because handleCompactCommand does not read the auto-compact threshold synchronously; the changeset admits this is 'reserved plumbing', so closing #1151 will lose the rest of the work.

🟠 important · completeness · .changeset/inline-once-overrides.md:10

The PR is titled feat(commands): add inline overrides for slash commands and the description claims to close #1151, whose headline example is /compact ?threshold=80 — 'test a value for one command'. With this diff in place, typing /compact ?threshold=80 will: (a) write setAutoCompactThreshold(80) into the session-override store, (b) run handleCompactCommand with commandParts = ['compact'] (the ? tokens are stripped), and (c) restore the prior threshold. But handleCompactCommand does not read autoCompactSessionOverrides.threshold during the compaction — it calls compressMessages(allMessages, tokenizer, {mode}) (compact-handler.ts:236) and never references the threshold store at runtime. The threshold override is therefore a no-op for /compact. Only /usage (which reads getSessionContextLimit() synchronously in usage.tsx:177) actually demonstrates the round-trip working end-to-end.

The changeset openly says threshold and auto-compact are 'reserved plumbing, not advertised as working command examples'. That is honest disclosure, but the PR still says Closes #1151, which will auto-close the issue on merge. The remaining work — wiring getAutoCompactThreshold() reads into handleCompactCommand (or wherever the consumer lives) — is a one-line follow-up that will be lost. Either (a) drop Closes #1151 from the changeset description and leave the issue open with a comment pointing at this PR, or (b) add the consumer wiring here so the headline example actually works.

🟠 important · correctness · source/utils/inline-overrides.ts:70

The PR description's example list says:

/some-command ?unknown=1            # ignored; preserved in args so the handler can error normally

That is not what the code does. parseInlineOverrides puts every ?… token whose key matches KEY_PATTERN into the overrides array (regardless of whether the key is recognised), and the dispatcher's cleanedMessage is rebuilt from [...positional, ...expandedFlags] — overrides for unrecognised keys are not appended to either list. So a ?unknown=1 token is stripped from cleanedMessage and never reaches the handler. The file's own docblock (around the KEY_PATTERN regex) and the changeset both describe the actual behaviour as 'silently dropped'. Three places now disagree about what unknown keys do:

  • PR description example list: 'preserved in args so the handler can error normally'
  • source/utils/inline-overrides.ts docblock: 'silently dropped: the dispatcher never forwards them to the handler'
  • .changeset/inline-once-overrides.md: 'Unknown ?foo=bar keys are silently ignored (no error message).'

Pick one. The cleanest fix is to either (a) actually preserve unrecognised ?key=value tokens in commandParts/cleanedMessage so handlers can produce their own 'unknown arg' error, matching the PR description's promise to the user, or (b) fix the description to match the current silent-drop behaviour. Whichever direction is chosen, the three places need to say the same thing.

⚪ nit · scope · source/utils/auto-compact.ts:17

The PR description's 'Files' list includes '.gitignore (.pnpm-store/)' as one of the touched files, but no .gitignore hunk appears in the diff. The metadata's changed_files also does not list it. Either remove the line from the description or include the change — leaving it as a phantom change misrepresents the PR's scope to reviewers.

⚪ nit · scope · source/utils/auto-compact-session.ts:1

The diff includes a non-trivial refactor — extracting AutoCompactSessionOverrides, the four set* helpers, and resetAutoCompactSession from source/utils/auto-compact.ts into a new source/utils/auto-compact-session.ts, with auto-compact.ts re-exporting them. The refactor is motivated (the spec timed out under AVA's serial worker because @/utils/auto-compact pulls in the chat-handler / config / tokenization init graph), and the solution is reasonable: auto-compact-session.ts has no heavy imports and inline-overrides.ts can dynamic-import it without paying the cost in the parser spec.

The refactor is well-scoped to the problem, and the spec files (inline-overrides.spec.ts) genuinely benefit. Flagging only because it is technically a drive-by restructure mixed into a feature PR, and a reviewer who wants to bisect 'what changed for the override feature' has to read through the extraction too. A short note in the PR description (or PR title) acknowledging 'and also extracted auto-compact-session.ts to keep the parser import-free') would make the review easier.

⚪ nit · tests · source/app/utils/app-util.spec.ts:1240

Of the four new dispatcher tests in this file, the third — inline overrides - parseInput keeps the '?' token in args for downstream cleaning — does not actually exercise the dispatcher. It calls parseInput('/usage ?context-max=200k') and asserts that the returned args array still contains ['?context-max=200k']. That is a property of source/command-parser.ts (which the feature does not touch), not of the new override plumbing. The test passes both before and after this PR; it does not regress if parseInlineOverrides is broken, because the dispatcher never sees the parseInput output — handleSlashCommand re-splits the raw message. The real end-to-end coverage lives in test.serial('inline overrides - dispatcher applies a ?context-max override and restores the prior value'), which is sound. The third test can either be removed (it adds noise to a behavioural review) or rewritten to actually exercise the dispatcher with ?unknown=1 so the contradiction between the description and the implementation (see the correctness finding above) gets a regression test.


🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional

Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.

@github-actions github-actions Bot added the agent:comments nc-review left non-blocking findings label Sep 21, 2026
@MayurK-cmd
MayurK-cmd force-pushed the fix/1151-once-scope-config-override branch from 2afa9cd to a18e443 Compare September 24, 2026 02:16
@akramcodez

Copy link
Copy Markdown
Member

Hey @MayurK-cmd! Could you please resolve the merge conflicts first? The branch is currently conflicting in compact-handler.ts and auto-compact.ts.

Also, the required Unit Tests & Coverage Analysis check is currently failing, so please fix the failing test and push the updated changes. Once both are resolved, we can take another look.

@MayurK-cmd
MayurK-cmd force-pushed the fix/1151-once-scope-config-override branch 2 times, most recently from a18e443 to 0bb9c05 Compare September 29, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:comments nc-review left non-blocking findings area:tui Terminal UI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Allow ?config= override on slash commands for one-off tuning

3 participants