Skip to content

owlstronaut/tap upgrade #8549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
wants to merge 18 commits into
base: latest
Choose a base branch
from
Draft

owlstronaut/tap upgrade #8549

wants to merge 18 commits into from
+10,825 −10,358

Conversation

owlstronaut
Copy link
Contributor

  • chore: [email protected]
  • chore: t.mock -> t.mockRequire
  • chore: template-oss-apply
  • chore: update tap for every workspace to avoid collisions
  • chore: allow map files in node modules
  • chore: update node_modules ignore
  • chore: fix exit handler test
  • chore: fixes update-notifier
  • chore: fix diff
  • chore: doctor tests
  • exec
  • explore
  • ls
  • run-script
  • log
  • chore: generate snapshots
  • chore: use c8 ignore statements where possible

github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 4, 2025
View reviewed changes
test/lib/commands/explore.js
@@ -70,7 +70,7 @@

t.ok(PJ_CALLED.endsWith('/pkg'))
t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
t.match(output, /Exploring \{CWD\}\/[\w-_/]+\nType 'exit' or \^D when finished/)
t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)

Check failure

Code scanning / CodeQL

Inefficient regular expression High test

This part of the regular expression may cause exponential backtracking on strings starting with 'Exploring {CWD}/' and containing many repetitions of 'a'.

Copilot Autofix

AI 13 days ago

The problematic pattern (.+)+ can cause exponential backtracking and should be replaced by an equivalent but unambiguous expression. The intent is to match at least one or more non-empty sequences after Exploring {CWD}/, presumably a path. A better approach is to match "one or more of any character except newline" directly: use .+ (not nested in a repeated group), or if multiple components are intended, consider (.+) or aggregate all repeated groups into one. Alternatively, to be even more precise, replace (.+)+ with [^\n]+, matching any number of non-newline characters, or at least one component. To maintain functionality, replace /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/ with /Exploring \{CWD\}\/[^\n]+\nType 'exit' or \^D when finished/. No additional imports or definitions are needed.

Make this change only on line 73 (and line 86, which contains the same pattern).

Suggested changeset 1
test/lib/commands/explore.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/test/lib/commands/explore.js b/test/lib/commands/explore.js
--- a/test/lib/commands/explore.js
+++ b/test/lib/commands/explore.js
@@ -70,7 +70,7 @@
 
   t.ok(PJ_CALLED.endsWith('/pkg'))
   t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
-  t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)
+  t.match(output, /Exploring \{CWD\}\/[^\n]+\nType 'exit' or \^D when finished/)
 })
 
 t.test('interactive tracks exit code', async t => {
@@ -83,7 +83,7 @@
 
     t.ok(PJ_CALLED.endsWith('/pkg'))
     t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
-    t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)
+    t.match(output, /Exploring \{CWD\}\/[^\n]+\nType 'exit' or \^D when finished/)
 
     t.equal(process.exitCode, 99)
   })
EOF
@@ -70,7 +70,7 @@

t.ok(PJ_CALLED.endsWith('/pkg'))
t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)
t.match(output, /Exploring \{CWD\}\/[^\n]+\nType 'exit' or \^D when finished/)
})

t.test('interactive tracks exit code', async t => {
@@ -83,7 +83,7 @@

t.ok(PJ_CALLED.endsWith('/pkg'))
t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)
t.match(output, /Exploring \{CWD\}\/[^\n]+\nType 'exit' or \^D when finished/)

t.equal(process.exitCode, 99)
})
Copilot is powered by AI and may make mistakes. Always verify output.
test/lib/commands/explore.js
@@ -83,7 +83,7 @@

t.ok(PJ_CALLED.endsWith('/pkg'))
t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
t.match(output, /Exploring \{CWD\}\/[\w-_/]+\nType 'exit' or \^D when finished/)
t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)

Check failure

Code scanning / CodeQL

Inefficient regular expression High test

This part of the regular expression may cause exponential backtracking on strings starting with 'Exploring {CWD}/' and containing many repetitions of 'a'.

Copilot Autofix

AI 13 days ago

The problem is with the use of (.+)+. In practice, this would match one or more repetitions of one or more characters, but is ambiguous and inefficient. The intention seems to be to match the path segment after {CWD}/. To remove the ambiguity, we should rewrite the regex to avoid the nested repetition and prevent multiple ways of matching the same substring. One solid approach is to use a character class that matches anything except a path delimiter or newline, e.g. ([^/\n]+) (or, if slashes may appear, at least avoid matching newlines: ([^\n]+)). Alternatively, if it is meant to greedily match everything up to \n, simply use a non-newline-matching greedy clause, i.e. ([^\n]+). This fix can be applied on line 86 (and 73, for consistency) inside test/lib/commands/explore.js. No new imports or dependencies are required.

Suggested changeset 1
test/lib/commands/explore.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/test/lib/commands/explore.js b/test/lib/commands/explore.js
--- a/test/lib/commands/explore.js
+++ b/test/lib/commands/explore.js
@@ -70,7 +70,7 @@
 
   t.ok(PJ_CALLED.endsWith('/pkg'))
   t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
-  t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)
+  t.match(output, /Exploring \{CWD\}\/([^\n]+)\nType 'exit' or \^D when finished/)
 })
 
 t.test('interactive tracks exit code', async t => {
@@ -83,7 +83,7 @@
 
     t.ok(PJ_CALLED.endsWith('/pkg'))
     t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
-    t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)
+    t.match(output, /Exploring \{CWD\}\/([^\n]+)\nType 'exit' or \^D when finished/)
 
     t.equal(process.exitCode, 99)
   })
EOF
@@ -70,7 +70,7 @@

t.ok(PJ_CALLED.endsWith('/pkg'))
t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)
t.match(output, /Exploring \{CWD\}\/([^\n]+)\nType 'exit' or \^D when finished/)
})

t.test('interactive tracks exit code', async t => {
@@ -83,7 +83,7 @@

t.ok(PJ_CALLED.endsWith('/pkg'))
t.strictSame(RUN_SCRIPT_EXEC, 'shell-command')
t.match(output, /Exploring \{CWD\}\/(.+)+\nType 'exit' or \^D when finished/)
t.match(output, /Exploring \{CWD\}\/([^\n]+)\nType 'exit' or \^D when finished/)

t.equal(process.exitCode, 99)
})
Copilot is powered by AI and may make mistakes. Always verify output.
@owlstronaut
Copy link
Contributor Author

Most of these changes are taken from #8085, but i found the merge/rebase to be prohibitively difficult to update that one. This also converts all istanbul ignore comments to c8. We drop to 86.46% coverage with this change because of the differences between c8 and istanbul tapjs/tapjs#877

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@owlstronaut