Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 79 additions & 2 deletions app/lib/ui/session/tool_summary.dart
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,68 @@ final RegExp _trailingVersion = RegExp(
/// once instead of on every token of every segment.
final RegExp _trailingSlashes = RegExp(r'/+$');

/// Wrapper flags that take a **separated** value, keyed by wrapper. The value
/// belongs to the flag, not to the pipeline: `timeout -s KILL 120 ssh` is
/// `ssh`, and used to read `KILL`.
///
/// Keyed rather than one flat set because the same letter means different
/// things per binary: `nice -n 10` and `watch -n 2` take a value, while
/// `sudo -n` (non-interactive) takes none — a flat set would have eaten the
/// command out of `sudo -n systemctl restart nginx`. Attached forms
/// (`--signal=KILL`, `-n1`) need no entry: they are already one flag token.
const Map<String, Set<String>> _wrapperValueFlags = {
'timeout': {'-s', '--signal', '-k', '--kill-after'},
'sudo': {
'-u',
'--user',
'-g',
'--group',
'-p',
'--prompt',
'-C',
'-h',
'--host',
'-U',
'-r',
'--role',
'-t',
'--type',
},
'doas': {'-u', '-C'},
'nice': {'-n', '--adjustment'},
'watch': {'-n', '--interval'},
'xargs': {
'-I',
'-i',
'--replace',
'-n',
'--max-args',
'-P',
'--max-procs',
'-L',
'-s',
'--max-chars',
'-a',
'--arg-file',
'-d',
'--delimiter',
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
'env': {'-u', '--unset', '-C', '--chdir', '-S', '--split-string'},
'stdbuf': {'-i', '--input', '-o', '--output', '-e', '--error'},
'time': {'-o', '--output', '-f', '--format'},
'script': {'-c', '--command', '-T', '--log-timing', '-o', '--log-out'},
Comment thread
leduckhc marked this conversation as resolved.
Outdated
'strace': {'-e', '-o', '-p', '-s', '-E', '-P', '-u'},
'ltrace': {'-e', '-o', '-p', '-s'},
'dtruss': {'-p', '-n'},
'caffeinate': {'-t', '-w'},
'exec': {'-a'},
};

/// A wrapper's numeric operand: `timeout 120`, `timeout 1.5s`, `timeout 30m`.
/// Only consulted once a wrapper has been skipped, so a real binary whose name
/// is digits is unaffected.
final RegExp _wrapperOperand = RegExp(r'^[0-9]+(?:\.[0-9]+)?[smhd]?$');
Comment thread
macroscopeapp[bot] marked this conversation as resolved.
Outdated

/// The distinct commands [command] runs, in first-seen order, joined with
/// `, ` — the payload of a collapsed shell row. Empty when the command is
/// empty or is nothing but prologue.
Expand Down Expand Up @@ -516,6 +578,10 @@ String? _segmentName(String segment) {
if (_compoundHeads.contains(words.first)) return null;

var i = 0;
// `timeout 120 ssh …` used to report `120`: the duration is the *wrapper's*
// argument, not a command. The most recent wrapper is remembered rather than
// assumed, so both operand rules below apply only when one was really seen.
String? wrapper;
while (i < words.length) {
final word = words[i];
final base = _unversioned(_basename(word));
Expand All @@ -525,11 +591,22 @@ String? _segmentName(String segment) {
i += _bareRedirection.hasMatch(word) ? 2 : 1;
continue;
}
if (_wrappers.contains(base)) {
wrapper = base;
i++;
continue;
}
// A wrapper flag with a separated value takes the next token with it.
if (wrapper != null &&
(_wrapperValueFlags[wrapper]?.contains(word) ?? false)) {
i += 2;
continue;
}
if (_assignment.hasMatch(word) ||
_shellKeywords.contains(word) ||
_wrappers.contains(base) ||
word.startsWith('-') ||
word.startsWith('«')) {
word.startsWith('«') ||
(wrapper != null && _wrapperOperand.hasMatch(word))) {
Comment thread
macroscopeapp[bot] marked this conversation as resolved.
Outdated
i++;
continue;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Expand Down
37 changes: 37 additions & 0 deletions app/test/tool_summary_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,43 @@ void main() {
expect(commandNames('./scripts/deploy.sh --dry-run'), 'deploy.sh');
});

// A wrapper's own operand is not the command: `timeout 120 ssh …` showed
// `Run 120` on the real app. Durations and flag values belong to the
// wrapper, so the scan keeps walking until it meets a real name.
test('T4 skips a wrapper operand', () {
expect(commandNames('timeout 120 ssh host uptime'), 'ssh');
expect(commandNames('timeout 1.5s curl -s http://x'), 'curl');
expect(commandNames('timeout -k 5 30m pnpm test'), 'pnpm test');
expect(commandNames('watch -n 2 git status'), 'git status');
expect(commandNames('nice -n 10 make -j4'), 'make');
});

// Same bug one step further out: the value of a *separated* wrapper flag is
// the flag's, not a command. `timeout -s KILL 120 ssh` reported `KILL`.
// The table is keyed by wrapper because the same letter can be boolean
// elsewhere — `sudo -n` takes no value, and eating its next word would lose
// the command entirely.
test('T4 skips the value of a wrapper flag', () {
expect(commandNames('timeout -s KILL 120 ssh host uptime'), 'ssh');
expect(commandNames('timeout --signal KILL 5 curl -s http://x'), 'curl');
expect(
commandNames('sudo -u root systemctl restart nginx'),
'systemctl restart',
);
expect(commandNames('xargs -I {} grep -l TODO {}'), 'grep');
expect(commandNames('env -u HOME python3 x.py'), 'python');
});

// A boolean flag on the same wrapper must not swallow the command.
test('T4 keeps the command after a valueless wrapper flag', () {
expect(
commandNames('sudo -n systemctl restart nginx'),
'systemctl restart',
);
expect(commandNames('env -i bash -lc "echo hi"'), 'bash');
expect(commandNames('timeout -k 5 30 pnpm test'), 'pnpm test');
});

test('T4 normalises a versioned interpreter', () {
expect(commandNames('python3 -c "print(1)"'), 'python');
expect(commandNames('python3.12 tool/wait.py'), 'python');
Expand Down
36 changes: 34 additions & 2 deletions mockups/tool-one-liner.html
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,9 @@ <h2 class="sec"><span class="n">2</span>What “the commands it runs” means, p
<span class="ex">for f in *.dart; do wc -l "$f"; done | sort → <i>wc, sort</i></span></li>

<li><b>Unwrap wrappers, then take the basename.</b> <code>sudo env time nice nohup xargs timeout stdbuf</code> are
skipped, as are leading flags; <code>~/flutter/bin/flutter</code> → <code>flutter</code>,
skipped along with their own operands — a numeric duration (<code>timeout 120 ssh</code> is <i>ssh</i>, not
<i>120</i>) and the value of a separated flag (<code>timeout -s KILL 120 ssh</code> is <i>ssh</i>, not
<i>KILL</i>) — as are leading flags; <code>~/flutter/bin/flutter</code> → <code>flutter</code>,
<code>./scripts/deploy.sh</code> → <code>deploy.sh</code>. Versioned interpreters normalise
(<code>python3</code>, <code>python3.12</code> → <code>python</code>; <code>pip3</code> → <code>pip</code>) —
the brief's example says <code>python</code>.
Expand Down Expand Up @@ -724,6 +726,30 @@ <h2 class="sec"><span class="n">10</span>Deltas — ordered by value per line ch
const WRAPPERS = new Set(['sudo','doas','env','time','nice','nohup','exec','command','builtin','stdbuf',
'xargs','timeout','caffeinate','script','strace','ltrace','dtruss','watch']);

/* A wrapper's numeric operand: `timeout 120`, `timeout 1.5s`, `watch -n 2`. */
const WRAPPER_ARG = /^[0-9]+(?:\.[0-9]+)?[smhd]?$/;

/* Wrapper flags taking a SEPARATED value, keyed by wrapper: the value is the
flag's, not a command (`timeout -s KILL 120 ssh` is ssh, not KILL). Keyed,
because `nice -n 10` takes a value while `sudo -n` does not. */
const WRAPPER_VALUE_FLAGS = {
timeout:['-s','--signal','-k','--kill-after'],
sudo:['-u','--user','-g','--group','-p','--prompt','-C','-h','--host','-U','-r','--role','-t','--type'],
doas:['-u','-C'],
nice:['-n','--adjustment'],
watch:['-n','--interval'],
xargs:['-I','-i','--replace','-n','--max-args','-P','--max-procs','-L','-s','--max-chars','-a','--arg-file','-d','--delimiter'],
env:['-u','--unset','-C','--chdir','-S','--split-string'],
stdbuf:['-i','--input','-o','--output','-e','--error'],
time:['-o','--output','-f','--format'],
script:['-c','--command','-T','--log-timing','-o','--log-out'],
strace:['-e','-o','-p','-s','-E','-P','-u'],
ltrace:['-e','-o','-p','-s'],
dtruss:['-p','-n'],
caffeinate:['-t','-w'],
exec:['-a'],
};

const KEYWORDS = new Set(['if','then','else','elif','fi','while','until','do','done','case','esac','in',
'select','function','{','}','[[',']]','!','(',')','&&','||','&','coproc']);

Expand Down Expand Up @@ -873,12 +899,18 @@ <h2 class="sec"><span class="n">10</span>Deltas — ordered by value per line ch
if(!toks.length) return null;
if(HEADERS.has(toks[0])) return null; /* loop header */
let i = 0;
let wrapper = null; /* most recent wrapper skipped */
while(i < toks.length){
const t = toks[i], b = normalise(basename(t));
/* a bare operator swallows the token after it, so `> out.txt grep foo`
still finds `grep` rather than naming the redirect's target */
if(REDIR.test(t)){ i += BARE_REDIR.test(t) ? 2 : 1; continue; }
if(ASSIGN.test(t) || KEYWORDS.has(t) || WRAPPERS.has(b) || t.startsWith('-') || t === '«heredoc»'){ i++; continue; }
if(WRAPPERS.has(b)){ wrapper = b; i++; continue; }
/* a wrapper flag with a separated value takes the next token with it */
if(wrapper && (WRAPPER_VALUE_FLAGS[wrapper] || []).includes(t)){ i += 2; continue; }
/* the wrapper's own operand: `timeout 120 ssh …` is ssh, not 120 */
if(ASSIGN.test(t) || KEYWORDS.has(t) || t.startsWith('-') || t === '«heredoc»'
|| (wrapper && WRAPPER_ARG.test(t))){ i++; continue; }
if(PROLOGUE.has(b)) return null; /* cd / export / set … */
break;
}
Expand Down
Loading