From e7afa83245397021ce2f961251732c22c85db38c Mon Sep 17 00:00:00 2001 From: Thieu Nguyen <79964592+thieung@users.noreply.github.com> Date: Mon, 30 Mar 2026 08:37:30 +0700 Subject: [PATCH] =?UTF-8?q?fix:=20dep=5Fscanner=20false=20positive=20?= =?UTF-8?q?=E2=80=94=20JS=20import=20inside=20Python=20f-string=20(#564)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: skip JS ES module imports in Python string literals during dep scan dep_scanner falsely matched JS `import mermaid from '...'` inside Python f-strings as a pip dependency, causing functional skills to be archived. Add jsFromStringRe check: if a line matching pyImportRe also contains `from '...'`/`from "..."` (JS ES module pattern), skip it. Valid Python `import X` never has `from '...'` on the same line. Closes #544 * fix: add missing stub method + additional edge case tests - Add MigrateUserDataOnMerge to stubAgentStore in context_file_interceptor_test.go (fixes CI build failure — stub was missing new AgentStore interface method) - Add TestScanFile_MultipleJSImportsInsidePythonString for broader false positive coverage --------- Co-authored-by: viettranx --- internal/skills/dep_scanner.go | 8 +- .../skills/dep_scanner_false_positive_test.go | 91 +++++++++++++++++++ .../tools/context_file_interceptor_test.go | 3 + 3 files changed, 101 insertions(+), 1 deletion(-) create mode 100644 internal/skills/dep_scanner_false_positive_test.go diff --git a/internal/skills/dep_scanner.go b/internal/skills/dep_scanner.go index b61c7ebb..446a0107 100644 --- a/internal/skills/dep_scanner.go +++ b/internal/skills/dep_scanner.go @@ -120,6 +120,9 @@ var ( nodeRequireRe = regexp.MustCompile(`require\(['"]([\w@][^'"]*)['"]\)`) nodeESImportRe = regexp.MustCompile(`from\s+['"]([^'"./][^'"]*?)['"]`) shebangRe = regexp.MustCompile(`^#!\s*/usr/bin/env\s+(\S+)`) + // Detects JS ES module pattern: `import X from '...'` or `from '...'`. + // Used to skip false positives when JS imports appear inside Python string literals. + jsFromStringRe = regexp.MustCompile(`from\s+['"]`) ) func scanFile(path string, pyImports, nodeImports map[string]bool, binaries map[string]bool) { @@ -143,7 +146,10 @@ func scanFile(path string, pyImports, nodeImports map[string]bool, binaries map[ for _, line := range strings.Split(content, "\n") { line = strings.TrimSpace(line) if m := pyImportRe.FindStringSubmatch(line); len(m) > 1 { - pyImports[m[1]] = true + // Skip JS ES module imports inside string literals (e.g. `import mermaid from '...'`) + if !jsFromStringRe.MatchString(line) { + pyImports[m[1]] = true + } } if m := pyFromRe.FindStringSubmatch(line); len(m) > 1 { pyImports[m[1]] = true diff --git a/internal/skills/dep_scanner_false_positive_test.go b/internal/skills/dep_scanner_false_positive_test.go new file mode 100644 index 00000000..806319a4 --- /dev/null +++ b/internal/skills/dep_scanner_false_positive_test.go @@ -0,0 +1,91 @@ +package skills + +import ( + "os" + "path/filepath" + "testing" +) + +func TestScanFile_JSImportInsidePythonFString(t *testing.T) { + dir := t.TempDir() + pyFile := filepath.Join(dir, "render.py") + + // Python file with JS ES module import inside f-string (issue #544) + content := `#!/usr/bin/env python3 +import sys +import json + +def render_html(text): + mermaid_init = f""" + +""" + return f"{text}{mermaid_init}" +` + if err := os.WriteFile(pyFile, []byte(content), 0644); err != nil { + t.Fatal(err) + } + + pyImports := make(map[string]bool) + nodeImports := make(map[string]bool) + binaries := make(map[string]bool) + + scanFile(pyFile, pyImports, nodeImports, binaries) + + // sys and json are real Python imports — should be detected + if !pyImports["sys"] { + t.Error("expected sys to be detected as Python import") + } + if !pyImports["json"] { + t.Error("expected json to be detected as Python import") + } + + // mermaid is a JS import inside f-string — should NOT be detected + if pyImports["mermaid"] { + t.Error("FALSE POSITIVE: mermaid detected as Python import — it's a JS import inside f-string") + } +} + +func TestScanFile_MultipleJSImportsInsidePythonString(t *testing.T) { + dir := t.TempDir() + pyFile := filepath.Join(dir, "template.py") + + // Multiple JS imports inside a Python string + real Python imports + content := `import os +import subprocess + +TEMPLATE = """ + +""" +` + if err := os.WriteFile(pyFile, []byte(content), 0644); err != nil { + t.Fatal(err) + } + + pyImports := make(map[string]bool) + nodeImports := make(map[string]bool) + binaries := make(map[string]bool) + + scanFile(pyFile, pyImports, nodeImports, binaries) + + // Real Python imports + if !pyImports["os"] { + t.Error("expected os to be detected as Python import") + } + if !pyImports["subprocess"] { + t.Error("expected subprocess to be detected as Python import") + } + + // JS imports inside string — should NOT be detected + if pyImports["React"] { + t.Error("FALSE POSITIVE: React detected as Python import") + } + if pyImports["lodash"] { + t.Error("FALSE POSITIVE: lodash detected as Python import") + } +} diff --git a/internal/tools/context_file_interceptor_test.go b/internal/tools/context_file_interceptor_test.go index 62f3cf4b..9a6a10ff 100644 --- a/internal/tools/context_file_interceptor_test.go +++ b/internal/tools/context_file_interceptor_test.go @@ -90,6 +90,9 @@ func (s *stubAgentStore) EnsureUserProfile(_ context.Context, _ uuid.UUID, _ str func (s *stubAgentStore) PropagateContextFile(_ context.Context, _ uuid.UUID, _ string) (int, error) { return 0, nil } +func (s *stubAgentStore) MigrateUserDataOnMerge(_ context.Context, _ []string, _ string) error { + return nil +} // ---- Tests ----