added shell command execution

diff --git a/nl-shell-harness-spec.md b/nl-shell-harness-spec.md
index d588d0b59282c3716ec76ea660749629f7bb3710..128d8fd7447d721c4f02cbac054385f764319aa5 100644 (file)
--- a/nl-shell-harness-spec.md
+++ b/nl-shell-harness-spec.md
@@ -87,7 +87,7 @@ Raw input becomes an array of typed tokens via a single-pass lexer (`Lexer.token
 
 Literals are never embedded back into a string with markers — they're carried as token data, so nothing downstream can accidentally lowercase, split, or "correct" a filename, and no restore/unmasking step is needed. `token.key` returns the word text or the literal's **kind** (not its value), so `delete old.log` and `delete report.pdf` normalize to the identical match string `delete file` for Tier 1/2 comparison, while `token.text` on a literal still holds the real value for execution.
 
 
 Literals are never embedded back into a string with markers — they're carried as token data, so nothing downstream can accidentally lowercase, split, or "correct" a filename, and no restore/unmasking step is needed. `token.key` returns the word text or the literal's **kind** (not its value), so `delete old.log` and `delete report.pdf` normalize to the identical match string `delete file` for Tier 1/2 comparison, while `token.text` on a literal still holds the real value for execution.
 

-Known false-positive guards: a bare `?` is never a glob character, so it always ends a question. (The `e.g.`/`i.e.` abbreviation guard described in the original source listing below is **not present** in the current `nlshell.rb` implementation — a gap to revisit.)
+Known false-positive guards: a bare `?` is never a glob character, so it always ends a question. The `e.g.`/`i.e.` abbreviation guard described in the original source listing below is now present in `nlshell.rb`: the file-literal path strips trailing sentence punctuation first, then checks the remaining text against `ABBREVIATIONS`, consuming one more `.` as the abbreviation's own (so it reads as a `:word`, not a sentence-ending `punct`).

 
 Full source (drop-in, no dependencies beyond Ruby's `strscan` stdlib):
 
 
 Full source (drop-in, no dependencies beyond Ruby's `strscan` stdlib):
 


@@ -340,10 +340,20 @@ Collapses the former `Classifier` pre-filter for `conditional`/`loop` into the s
 5. **Executor:** `Executor.call` currently flattens every arg via `.text`; it needs a branch so a `:body`/`:then`/`:else` arg executes its nested nodes recursively instead of being passed as a string. **Implemented**: `Executor.call` passes non-`Token` args through unchanged, and a new `Executor.run(node)` is the shared permission-checked entry point (confirms `:ask` tools, then calls) — used by both the REPL loop and the `conditional` tool's own action, so a destructive body still requires its own confirmation regardless of nesting.
 6. **Limits:** the depth cap (§7, 3) and loop iteration cap (§10, 20) must be enforced in this new recursive path, not just documented. **Not implemented** — nesting depth is currently unbounded in practice (e.g. `if A exists do if B exists do archive B` resolves fine), and there is no loop tool yet to iterate.
 
 5. **Executor:** `Executor.call` currently flattens every arg via `.text`; it needs a branch so a `:body`/`:then`/`:else` arg executes its nested nodes recursively instead of being passed as a string. **Implemented**: `Executor.call` passes non-`Token` args through unchanged, and a new `Executor.run(node)` is the shared permission-checked entry point (confirms `:ask` tools, then calls) — used by both the REPL loop and the `conditional` tool's own action, so a destructive body still requires its own confirmation regardless of nesting.
 6. **Limits:** the depth cap (§7, 3) and loop iteration cap (§10, 20) must be enforced in this new recursive path, not just documented. **Not implemented** — nesting depth is currently unbounded in practice (e.g. `if A exists do if B exists do archive B` resolves fine), and there is no loop tool yet to iterate.
 

-**Known gap, discovered during implementation — a connective word can't double as rule syntax.** `builtins`' `connectives` list (§5.5) makes `"then"` an unconditional clause separator at the `Lexer.clauses` stage. A rule pattern like `"if :target exists then :body"` can never match, because clause-splitting tears it into two clauses before Tier1/Tier2 ever see the full span — the pattern's literal `"then"` and the lexer's connective `"then"` collide. Worked around for the minimal slice by using `"do"` as the condition/body joiner instead (`"if :target exists do :body"`), but this is a generalizable problem: any future control-flow tool that wants a connective word (`"then"`, `"and"`, `"next"`, etc.) as literal syntax will hit the same wall, and loop syntax (`"for each :item in :list do :body"` or similar) needs to be designed around it too. Needs a real fix before generalizing past the minimal slice — e.g. scoping connective-splitting to happen only outside a span a control-flow tool's pattern would claim, or reserving a connective-free joiner vocabulary for rule authors.
+**Resolved gap, discovered during implementation — a connective word can't double as rule syntax.** `builtins`' `connectives` list (§5.5) makes `"then"` an unconditional clause separator at the `Lexer.clauses` stage. A rule pattern like `"if :target exists then :body"` can never match, because clause-splitting tears it into two clauses before Tier1/Tier2 ever see the full span — the pattern's literal `"then"` and the lexer's connective `"then"` collide. Considered two fixes: (a) scope connective-splitting to skip a span a control-flow tool's pattern would claim, or (b) reserve connectives as a no-go vocabulary for rule literal syntax and enforce it. Went with (b) — (a) would make `Lexer.clauses` (pure tokenizing/segmentation) reach into `Tier1`'s job (pattern matching), and makes "is this connective a real separator here?" implicit and input-dependent, the opposite of this project's deterministic/auditable goal. **Implemented** as `Ruleset.connective_collision(pattern, connectives)`, checked in two places: `RulesModule#tool` raises `ArgumentError` at ruleset-build time if an author-declared rule's literal words contain a connective sequence (e.g. `"archive :target then :body"` is rejected, forcing `"do"` instead — as `conditional` already does); the `teach` tool's action runs the same check at runtime and refuses the trigger with a user-facing message rather than silently learning a rule that can never fire. Loop syntax (`"for each :item in :list do :body"` or similar) should keep following the same `"do"`-joiner convention.

 
 Suggested next slice: generalize from a single flat `:body` action to a list of actions (still no `else`, no loops), then add the predicate struct, then loops — validating one extension at a time.
 
 
 Suggested next slice: generalize from a single flat `:body` action to a list of actions (still no `else`, no loops), then add the predicate struct, then loops — validating one extension at a time.
 

+### 6.6 Command passthrough (`execute` tool)
+
+A fourth, last-resort resolution step after Tier 1/2 both miss: if a clause's first non-punctuation token is a recognized `$PATH` executable (Lexer's `:command` token, §5.1), it runs as a literal shell command instead of failing to resolve. **Implemented** as `CommandTool.lookup`, tried from `Utterance.resolve` only after `Tier1.lookup`/`Tier2.lookup` both miss, so a command name that also happens to be a declared verb (e.g. a tool literally named `archive`) still resolves through the normal rule first.
+
+This doesn't fit the `ToolBuilder`/Tier1/Tier2 pattern-matching machinery — there's no fixed trigger phrase to index, since the "pattern" is just "starts with any known executable" and the rest of the clause is arbitrary argv. `CommandTool::TOOL` is therefore a bare `Ruleset::Tool` built by hand (empty `rules`, so it never enters `Tier1.rules`/`Tier2`'s vocabulary) with its own `execute` action.
+
+**Security:** the action runs `Open3.capture3(*argv.map(&:text))` — passing argv as separate Ruby arguments, not one interpolated shell string — so Ruby/`Open3` execs the binary directly with no shell in between. Shell metacharacters in a literal token's text (`;`, `|`, `` ` ``, `$(...)`) are inert; there is no command-injection surface regardless of what a slot's value contains. The tradeoff: no glob expansion or `$VAR` substitution either, since there's no shell to do it — literals are passed through exactly as tokenized.
+
+Permission is always `:ask`, never `:yolo` — this is the least-constrained action in the system (arbitrary argv, no fixed semantics to reason about ahead of time), so per §10's destructive-action policy it must always confirm. `Executor.run`'s confirmation prompt was generalized to show the resolved args (not just the tool name) specifically so this confirmation isn't blind — e.g. `run execute git status? [y/N]` rather than just `run execute?`.
+

 ---
 
 ## 7. Node Schemas (JSON Schema, abbreviated)
 ---
 
 ## 7. Node Schemas (JSON Schema, abbreviated)

diff --git a/nlshell.rb b/nlshell.rb
index 8d8283b82a1d42541c8dc9bc9c5c08191bc2c875..cbb0e255d635dbfce82bc1341452723b8025d010 100755 (executable)
--- a/nlshell.rb
+++ b/nlshell.rb
@@ -3,6 +3,7 @@
 # frozen_string_literal: true
 require "set"
 require "strscan"
 # frozen_string_literal: true
 require "set"
 require "strscan"

+require "open3"

 
 # Scan the environment for possible shell commands
 ShellCommands = Set.new
 
 # Scan the environment for possible shell commands
 ShellCommands = Set.new


@@ -54,6 +55,10 @@ module Lexer
   # Sentence punctuation that greedy patterns may swallow ("rm ~/old.log.")
   TRAILING = /[.,;:!?]+\z/
 
   # Sentence punctuation that greedy patterns may swallow ("rm ~/old.log.")
   TRAILING = /[.,;:!?]+\z/
 

+  # Look like filenames but are ordinary words - their final period is part
+  # of the abbreviation, not a sentence end.
+  ABBREVIATIONS = %w[e.g i.e].freeze
+

   # Ends a sentence. ";" also separates commands.
   BOUNDARIES = %w[. ? ! ;].freeze
 
   # Ends a sentence. ";" also separates commands.
   BOUNDARIES = %w[. ? ! ;].freeze
 


@@ -74,14 +79,19 @@ module Lexer
 
       if kind == :string
         text = text[1..-2] # drop quotes, keep content
 
       if kind == :string
         text = text[1..-2] # drop quotes, keep content

-      elsif kind == :file and ShellCommands.include?(text)
-        return Token.new(:command, text, kind)

       else
         tail = text[TRAILING].to_s
         unless tail.empty?
           sc.pos -= tail.bytesize # hand sentence punctuation back to the scanner
           text = text.delete_suffix(tail)
         end
       else
         tail = text[TRAILING].to_s
         unless tail.empty?
           sc.pos -= tail.bytesize # hand sentence punctuation back to the scanner
           text = text.delete_suffix(tail)
         end

+
+        return Token.new(:command, text, kind) if kind == :file && ShellCommands.include?(text)
+
+        if kind == :file && ABBREVIATIONS.include?(text.downcase)
+          dot = sc.skip(/\./) ? "." : "" # its final period belongs to it, not a sentence end
+          return Token.new(:word, "#{text}#{dot}")
+        end

       end
 
       return Token.new(:literal, text, kind)
       end
 
       return Token.new(:literal, text, kind)


@@ -453,16 +463,28 @@ module Executor
 
   # Confirms first for an :ask tool, same policy a top-level clause gets - the
   # one gate between "resolved" and "actually runs", so a conditional's body
 
   # Confirms first for an :ask tool, same policy a top-level clause gets - the
   # one gate between "resolved" and "actually runs", so a conditional's body

-  # can't bypass permission checking.
+  # can't bypass permission checking. Shows the resolved args, not just the
+  # tool name, so a user never confirms blind - essential for `execute`,
+  # where the "args" are the literal command about to run.

   def self.run(node)
     tool = node[:tool]
     if tool.permission == :ask
   def self.run(node)
     tool = node[:tool]
     if tool.permission == :ask

-      print "   run #{tool.name}? [y/N] "
+      print "   run #{tool.name} #{describe(node[:args])}? [y/N] "

       return false unless gets&.chomp&.downcase == "y"
     end
 
     call(tool, node[:args])
   end
       return false unless gets&.chomp&.downcase == "y"
     end
 
     call(tool, node[:args])
   end

+
+  def self.describe(args)
+    args.map do |_, v|
+      case v
+      when Lexer::Token then v.text
+      when Array then v.map(&:text).join(" ")
+      else v.to_s
+      end
+    end.join(" ")
+  end

 end
 
 
 end
 
 


@@ -501,14 +523,17 @@ module Utterance
     Result.new(raw, clauses.map { |tokens| resolve(tokens) })
   end
 
     Result.new(raw, clauses.map { |tokens| resolve(tokens) })
   end
 

-  # Tries Tier 1 first (strict positional match), falling back to Tier 2
-  # (fuzzy bag-of-words) only on a Tier 1 miss - tags which tier resolved it
-  # so the REPL can report that, or nil/nil if neither did.
+  # Tries Tier 1 first (strict positional match), then Tier 2 (fuzzy
+  # bag-of-words), then - last resort - raw shell command passthrough, so a
+  # recognized $PATH executable still runs even with no matching rule. Tags
+  # which tier resolved it so the REPL can report that, or nil/nil if none did.

   def self.resolve(tokens)
     if (node = Tier1.lookup(tokens))
       Clause.new(tokens, node, :tier1)
     elsif (node = Tier2.lookup(tokens))
       Clause.new(tokens, node, :tier2)
   def self.resolve(tokens)
     if (node = Tier1.lookup(tokens))
       Clause.new(tokens, node, :tier1)
     elsif (node = Tier2.lookup(tokens))
       Clause.new(tokens, node, :tier2)

+    elsif (node = CommandTool.lookup(tokens))
+      Clause.new(tokens, node, :command)

     else
       Clause.new(tokens, nil, nil)
     end
     else
       Clause.new(tokens, nil, nil)
     end


@@ -526,6 +551,17 @@ module Ruleset
     pattern.split.each_with_object(Set.new) { |w, s| s << w.delete_prefix(":").to_sym if w.start_with?(":") }
   end
 
     pattern.split.each_with_object(Set.new) { |w, s| s << w.delete_prefix(":").to_sym if w.start_with?(":") }
   end
 

+  # A rule can't use a connective word/phrase as literal syntax: Lexer.clauses
+  # (SS5.5) splits on it before the rule ever sees the full span (e.g. "if
+  # :target exists then :body" is torn in two before Tier1 gets a look).
+  # Returns the colliding connective sequence, or nil. Shared by author-time
+  # rules (RulesModule#tool) and runtime-taught ones (the "teach" tool) so
+  # neither path can register a pattern clause-splitting would break.
+  def self.connective_collision(pattern, connectives)
+    words = pattern.split
+    connectives.find { |seq| words.each_cons(seq.length).any? { |window| window == seq } }
+  end
+

   # Registers pattern -> tool in rules, after checking pattern's :slot names
   # match reference's. The one mechanism behind both author-declared verb
   # synonyms (ToolBuilder#verb_synonyms) and user-taught ones (the "teach"
   # Registers pattern -> tool in rules, after checking pattern's :slot names
   # match reference's. The one mechanism behind both author-declared verb
   # synonyms (ToolBuilder#verb_synonyms) and user-taught ones (the "teach"


@@ -635,6 +671,7 @@ module Ruleset
   class RulesModule
     def initialize
       @rules = {}
   class RulesModule
     def initialize
       @rules = {}

+      @connectives = []

     end
 
     def types(hash)
     end
 
     def types(hash)


@@ -648,7 +685,14 @@ module Ruleset
     def tool(name, &block)
       builder = ToolBuilder.new(name)
       builder.instance_eval(&block)
     def tool(name, &block)
       builder = ToolBuilder.new(name)
       builder.instance_eval(&block)

-      register(builder.build)
+      built = builder.build
+      built.rules.each do |pattern|
+        collision = Ruleset.connective_collision(pattern, @connectives)
+        next unless collision
+
+        raise ArgumentError, "tool #{name.inspect}: rule #{pattern.inspect} uses connective #{collision.join(' ').inspect} as literal syntax - clause-splitting (SS5.5) would break it before the rule ever matches; pick a different joiner word"
+      end
+      register(built)

     end
 
     def literal_patterns    = @types
     end
 
     def literal_patterns    = @types


@@ -680,6 +724,31 @@ module Ruleset
   end
 end
 
   end
 end
 

+# Last-resort resolution (used by Utterance.resolve after Tier1/2 both miss):
+# a clause whose first non-punct token is a recognized $PATH executable
+# (Lexer's :command token) runs as a literal shell command. Matched by token
+# type, not a fixed trigger phrase, so it never goes through ToolBuilder/
+# Tier1/Tier2's pattern machinery - there's no vocabulary to index, since the
+# "pattern" is just "starts with any known executable".
+module CommandTool
+  # Open3.capture3 with separate argv elements execs the binary directly -
+  # no shell is ever invoked, so there's no shell-metacharacter/command-
+  # injection surface regardless of what a slot's literal text contains.
+  TOOL = Ruleset::Tool.new("execute", [], lambda { |argv:|
+    out, err, status = Open3.capture3(*argv.map(&:text))
+    print out
+    warn err unless err.empty?
+    puts "   exit #{status.exitstatus}" unless status.success?
+  }, :ask, Set.new)
+
+  def self.lookup(clause)
+    argv = clause.reject(&:punct?)
+    return nil unless argv.first&.command?
+
+    { type: "action", name: TOOL.name, args: { argv: argv }, tool: TOOL }
+  end
+end
+

 # Create default ruleset
 builtins = Ruleset.create("builtins") do
   types(
 # Create default ruleset
 builtins = Ruleset.create("builtins") do
   types(


@@ -775,6 +844,11 @@ builtins = Ruleset.create("builtins") do
         next
       end
 
         next
       end
 

+      if (collision = Ruleset.connective_collision(trigger, Lexer.connectives))
+        puts "   #{trigger.inspect} uses #{collision.join(' ').inspect} as literal words, but that's a clause-splitting connective - pick a different phrasing"
+        next
+      end
+

       # Same slot-check-and-register mechanism as author-declared verb
       # synonyms (Ruleset::ToolBuilder#verb_synonyms).
       unless Ruleset.register_rule(Tier1.rules, trigger, tool, reference: target)
       # Same slot-check-and-register mechanism as author-declared verb
       # synonyms (Ruleset::ToolBuilder#verb_synonyms).
       unless Ruleset.register_rule(Tier1.rules, trigger, tool, reference: target)

diff --git a/test/nlshell_test.rb b/test/nlshell_test.rb
index 9f0fef0cf49382e063789017deb1f49b05453ccf..0b1e499c9b24376a8ece1a74d198ebaca6fb9382 100644 (file)
--- a/test/nlshell_test.rb
+++ b/test/nlshell_test.rb
@@ -29,6 +29,14 @@ class LexerTest < Minitest::Test
     assert_equal 2, sentences.length
   end
 
     assert_equal 2, sentences.length
   end
 

+  # Regression: "e.g."/"i.e." must not be read as a file literal whose
+  # trailing period then ends the sentence early.
+  def test_abbreviation_period_does_not_end_a_sentence
+    tokens = Lexer.tokenize("keep text files, e.g. notes.txt, archived.")
+    assert_equal 1, Lexer.sentences(tokens).length
+    assert_equal :word, tokens.find { |t| t.text == "e.g." }.type
+  end
+

   def test_clauses_split_on_connective
     tokens = Lexer.downcase_words(Lexer.tokenize("delete old.log then archive new.log"))
     clauses = Lexer.clauses(tokens)
   def test_clauses_split_on_connective
     tokens = Lexer.downcase_words(Lexer.tokenize("delete old.log then archive new.log"))
     clauses = Lexer.clauses(tokens)


@@ -85,6 +93,44 @@ class UtteranceTest < Minitest::Test
     assert_nil clause.node
     assert_nil clause.tier
   end
     assert_nil clause.node
     assert_nil clause.tier
   end

+
+  # Last-resort fallback: no rule matches "ls", but it's a recognized $PATH
+  # executable, so it resolves via command passthrough instead of failing.
+  def test_resolves_a_recognized_executable_via_command_passthrough
+    clause = Utterance.build("ls").clauses.first
+    assert_equal :command, clause.tier
+    assert_equal "execute", clause.node[:name]
+  end
+end
+
+class CommandToolTest < Minitest::Test
+  def test_lookup_matches_a_clause_starting_with_a_command_token
+    tokens = Lexer.downcase_words(Lexer.tokenize("ls -la"))
+    node = CommandTool.lookup(tokens)
+    refute_nil node
+    assert_equal "execute", node[:name]
+    assert_equal %w[ls -la], node[:args][:argv].map(&:text)
+  end
+
+  def test_lookup_misses_on_an_ordinary_word
+    tokens = Lexer.downcase_words(Lexer.tokenize("bork the garble"))
+    assert_nil CommandTool.lookup(tokens)
+  end
+
+  def test_lookup_drops_trailing_sentence_punctuation
+    tokens = Lexer.downcase_words(Lexer.tokenize("ls."))
+    node = CommandTool.lookup(tokens)
+    assert_equal ["ls"], node[:args][:argv].map(&:text)
+  end
+
+  # The whole point of argv-form Open3 (no shell string) is that shell
+  # metacharacters in a literal's text can't be interpreted - this is the
+  # property that makes passthrough safe to add at all.
+  def test_execute_does_not_invoke_a_shell
+    argv = [Lexer::Token.new(:command, "echo"), Lexer::Token.new(:literal, "a; echo pwned", :string)]
+    out, = capture_io { CommandTool::TOOL.action.call(argv: argv) }
+    assert_equal "a; echo pwned\n", out
+  end

 end
 
 class Tier1Test < Minitest::Test
 end
 
 class Tier1Test < Minitest::Test


@@ -178,6 +224,19 @@ class RulesetTest < Minitest::Test
     assert_match(/don't match/, error.message)
   end
 
     assert_match(/don't match/, error.message)
   end
 

+  # A connective word used as literal rule syntax would get torn out by
+  # Lexer.clauses before Tier1 ever sees the full pattern - must fail loudly
+  # at build time instead of silently never matching.
+  def test_tool_rejects_rule_using_a_connective_as_literal_syntax
+    error = assert_raises(ArgumentError) do
+      Ruleset.create("t") do
+        connectives [["then"]]
+        tool("bad") { rule "do :x then :y"; action { |x:, y:| } }
+      end
+    end
+    assert_match(/connective "then"/, error.message)
+  end
+

   def test_tool_rejects_rule_starting_with_slot
     assert_raises(ArgumentError) do
       Ruleset.create("t") do
   def test_tool_rejects_rule_starting_with_slot
     assert_raises(ArgumentError) do
       Ruleset.create("t") do


@@ -229,6 +288,14 @@ class SharedRegistrationTest < Minitest::Test
     refute Ruleset.register_rule(rules, "wipe :thing", tool, reference: "delete :target")
     assert_nil rules["wipe :thing"]
   end
     refute Ruleset.register_rule(rules, "wipe :thing", tool, reference: "delete :target")
     assert_nil rules["wipe :thing"]
   end

+
+  def test_connective_collision_finds_a_literal_match
+    assert_equal ["then"], Ruleset.connective_collision("archive :target then :body", [["then"], ["next"]])
+  end
+
+  def test_connective_collision_ignores_a_slot_name
+    assert_nil Ruleset.connective_collision("if :target exists do :body", [["then"], ["next"]])
+  end

 end
 
 class ExecutorTest < Minitest::Test
 end
 
 class ExecutorTest < Minitest::Test


@@ -289,11 +356,27 @@ class ReplIntegrationTest < Minitest::Test
     assert_match(/remembered: "wipe" means "delete"/, out)
   end
 
     assert_match(/remembered: "wipe" means "delete"/, out)
   end
 

+  # A taught trigger that uses a connective word as literal syntax would be
+  # torn apart by clause-splitting before it could ever match - reject it at
+  # teach time instead of silently learning a rule that never fires.
+  def test_teach_rejects_a_trigger_that_collides_with_a_connective
+    out = run_repl(%(teach "zap :target then" means "delete :target"\ny\n))
+    assert_match(/clause-splitting connective/, out)
+    refute_match(/learned:/, out)
+  end
+

   def test_declining_confirmation_skips_execution
     out = run_repl(%(delete old.log\nn\n))
     refute_match(/Deleting old\.log/, out)
   end
 
   def test_declining_confirmation_skips_execution
     out = run_repl(%(delete old.log\nn\n))
     refute_match(/Deleting old\.log/, out)
   end
 

+  # The confirm prompt must show the literal command, not just the tool name
+  # - the user can't confirm blind on the riskiest action in the system.
+  def test_command_passthrough_prompts_with_the_literal_command
+    out = run_repl("ls\nn\n")
+    assert_match(/run execute ls\? \[y\/N\]/, out)
+  end
+

   def test_unresolvable_input_reports_couldnt_tell
     out = run_repl(%(bork the garble\n))
     assert_match(/couldn't tell what kind of request this is/, out)
   def test_unresolvable_input_reports_couldnt_tell
     out = run_repl(%(bork the garble\n))
     assert_match(/couldn't tell what kind of request this is/, out)