* test: fix five Windows-host failures across sandbox, read_file, lark shim, and extension checks
Windows dev hosts currently fail five backend test cases that CI (ubuntu)
never exercises:
- test_local_sandbox_command_timeout: the fixture forces a bare /bin/sh
shell and the Windows dispatch spawns it directly (WinError 2), and
make(1) is not a given on Windows. Stub both runners; the contract under
test is the timeout marker assembly, not the runner.
- test_local_sandbox_provider_mounts: the Popen-capture mock delegated to
the real spawn, which cannot execute a POSIX shell path on Windows.
Capture at the runner seam instead and assert the rewritten path
shape-agnostically (the Windows runners pass the command string, not a
[shell, -c, command] list).
- test_read_file_tool_binary: text-mode write_text leaks CRLF into the
fixture on Windows; write the mount fixture as bytes (#6042 follow-up).
- test_lark_shim_atomic: skip the exact 0o755/0o644 assertions on Windows
hosts, whose stat mode approximates POSIX bits (#6257 pattern).
- test_extension_check_script: zipfile normalizes os.sep in entry names on
Windows reads, so a writestr-built fixture can never carry a verbatim
backslash entry there. Inject the hostile name into namelist unless the
archive already stores it, pinning the guard contract identically.
On a Windows 11 host these five files go from five failed cases to green
(102 passed, ruff check/format clean). Two more Windows-host failures
(skill-review NUL probe, jina shared-deadline classification) need deeper
root-causing and are left for follow-ups.
* test(skill-review): review the committed skill tree, not local residue
The video-generation review test scans the live directory, so untracked
build residue inside skills/public/video-generation fails it: once bytecode
caching drops a scripts/__pycache__ directory into the tree (importing the
skill script writes one), skillscan's package-undecodable-script finding is
correct and the test can never pass again until the residue is deleted by
hand. Reproduced on Windows: a planted scripts/__pycache__ pyc fails the
test; deleting it makes it pass.
- Materialize the tracked skill tree (git ls-files) into a temp dir and
review that, so the test reviews exactly the committed package.
- tests/skills/skill_loader.py disables bytecode caching around
exec_module, so the root skill suite stops polluting the committed tree.
Follows up on #6786 (item 6).
* docs(test): state the argv-list contract on both sandbox runners
The Windows dispatch also passes the resolved command as an argv list, so
the string fallback was unreachable on every platform.
---------
Co-authored-by: xiaodu55 <xiaodu55@users.noreply.github.com>
* fix(skillscan): report secrets bound by unpacking and container literals
The AST walk behind secret-env-assignment claims "precision, never less
coverage", but it only read a single assignment target and a bare string
value, so a credential moved into `host, api_key = endpoint, "..."` or
into `api_key = ["..."]` stopped being reported even though the pre-#5648
line sweep still reported the same bytes.
Pair unpacking targets with values positionally (fixed head and tail only,
so a runtime-sized slice names nothing) and assert on each literal a
container or conditional holds, in source order.
* fix(skillscan): distinguish mapping labels from credentials
---------
Co-authored-by: sxh313 <sxh313@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
- measure the link-scan benchmark with perf_counter: monotonic ticks at
~15.6 ms on Windows and the linear scan finishes inside one tick, so
the ratio assertion divided by exactly 0.0;
- guard the owner-only mode assertion on Windows, where chmod cannot
express 0o600 (same class as #5932);
- pin strictly increasing updated_at values in the shelf-ordering test:
datetime.now(UTC) has ~15.6 ms granularity on Windows, so rapid inserts
can share one timestamp and the id tiebreak reorders the entries.
Fixes#6166
Co-authored-by: xiaodu55 <xiaodu55@users.noreply.github.com>
* fix(skills): bail out of the markdown-link scan when no link construct exists (#5714)
* fix(skills): scan markdown links linearly and skip failed target runs (#5714)
* fix(skills): make link scan exactly match finditer, blank spans by position
Address review feedback: drive the scan by openers (the regex engine's
own start-position scan) so it reproduces finditer exactly, blank
matched spans by position instead of str.replace, pin the equivalence
with tests, and add CHANGELOG entries.
* fix: resolve skill review test lint errors
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(skills): strip section anchors from code-span resource refs
Markdown-link targets already drop their #fragment before the
reference is resolved, but code-span refs did not: a prose reference
like `references/faq.md#pricing` kept the anchor and was resolved as
a literal path, producing a resource.missing finding for a file that
exists and is referenced. The bare-path token scanner stops at '#',
so only the code-span branch leaked fragments.
Split the fragment off code-span tokens the same way the link branch
does, then strip trailing sentence punctuation. Real dotted filenames
keep their dots: the split happens at the first '#', and '.' is only
stripped from the very end.
Signed-off-by: yetuge <2219677952@qq.com>
* fix(skills): prefer exact hash-bearing filenames over fragment stripping
Package paths permit '#', so a code-span reference to a real file like
references/C#.md was truncated to references/C by the fragment split,
producing both a false resource.missing and an orphan report (the bare
path token stopped at '#' the same way). Keep the whole reference in
every extraction branch (link target, code span, bare path token now
includes '#' in its character class) and resolve the fragment
existence-aware: prefer the exact token when it names a real file and
treat the suffix after the first '#' as a section fragment only when
it does not.
Red/green: a new test pins references/C#.md resolving to the real file
while references/faq.md#pricing still strips to faq.md — it fails on
the reviewed head and passes with the change; full
test_skill_review_core.py is 36 passed.
Signed-off-by: yetuge <2219677952@qq.com>
* fix(skills): strip the fragment before normalizing the fallback path
The exact-path preference checked the normalized reference, so a token
like references/faq.md#/../other.md normalized to references/other.md;
when that file existed the check succeeded and the fragment-removal
branch was skipped, recording the wrong edge and orphaning faq.md.
Check the literal token against the package files instead, and drop
the fragment before the remaining path is normalized — normalizing a
hash-bearing segment must never retarget an edge.
Signed-off-by: yetuge <2219677952@qq.com>
* fix(skills): canonicalize leading segments in the hash-filename lookup
The literal-file check compared the verbatim joined path, whose leading
relative segments are uncanonicalized: from references/sub/guide.md, a
reference to ../C#.md joins to references/sub/../C#.md, matches no
snapshot key, and falls through to fragment stripping — truncating the
real references/C#.md into a false missing and an orphan. Compare
against the normpath-canonicalized whole token instead, and treat the
post-'#' text as a fragment whenever it is path-like (carries / or ..)
so it can still never participate in normalization and retarget an
edge.
Signed-off-by: yetuge <2219677952@qq.com>
* fix(skills): check the canonical whole path before stripping a fragment
The path-like-fragment heuristic dropped any post-'#' text carrying '/':
a real package file such as references/C#/readme.md (a '#' inside a
directory name) was truncated to references/C, producing a false
resource.missing and orphaning readme.md. Only a '..' segment in the
post-'#' text forces the strip-first fallback — that is the one case
where fragment text participates in normalization and can retarget an
edge (faq.md#/../other.md). Every other fragment first gets the
canonicalized whole-token lookup, which already canonicalizes leading
relative segments for nested sources.
Signed-off-by: yetuge <2219677952@qq.com>
* fix(skills): always strip markdown link fragments and blind the bare-path pass to links
Markdown link targets have fixed fragment semantics: the text after '#'
is a URL fragment, never part of the filename, so a link to a file
literally named references/faq.md#pricing would have to percent-encode
it. The exact-file branch applied the hash-filename preference to link
targets too, recording a valid edge and suppressing resource.missing
for a broken link when a literal same-named file existed. Links strip
the fragment again, and their full construct is blanked out of the
residual text so the code-span and bare-path passes — literal-path
contexts where '#' may be part of a real filename — never see
link-internal text.
Signed-off-by: yetuge <2219677952@qq.com>
---------
Signed-off-by: yetuge <2219677952@qq.com>
* fix(skills): strip trailing sentence punctuation from extracted resource refs
The resource-graph extractor's _PATH_TOKEN_RE includes '.' in its path
character class, so a bare package-relative path at the end of a sentence
(e.g. "Read references/setup.md.") is extracted with the trailing period
still attached. build_resource_graph then emits a false-positive
resource.missing finding for "references/setup.md." and, because the
real file is never matched, reports the actual references/setup.md as an
unreferenced orphan.
Other sentence punctuation (",;:!?()") already falls outside the regex
character class and is correctly dropped; only '.' is ambiguous because it
is both a valid path character and prose punctuation.
Strip trailing '.', '?' and '!' from every extracted reference in
_extract_references. Real dotted filenames are unaffected: rstrip only
removes punctuation at the very end, so references/config.yaml and a link
target such as references/v1.0.md keep their dots.
Adds two regression tests covering prose-terminated bare paths and real
dotted filenames.
* style(tests): format skill review tests
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(skills): close SkillScan bypasses in the skill review gate
The public skill review gate re-materialized a package snapshot into a
temp directory for SkillScan, but copied only entries the reader had
decoded as text and skipped every file under any evals/fixtures/
directory. Executable binaries and nested archives never reached the
package rules, and a fixture-shaped path hid any script from the scan.
Readers now keep binary bytes as content_base64, the analyzer writes
every non-symlink file byte for byte, and only eval fixture SKILL.md
samples stay exempt. Files are created exclusively, so a duplicate
archive member or a case-folded name fails the scan closed instead of
overwriting an earlier file.
SkillScan itself skipped any file that was not NUL-free UTF-8. One
Latin-1 byte in a comment hid a reverse shell from the review gate, and
a NUL byte skipped static analysis at install. Code files that fail
strict decoding now raise package-undecodable-script (HIGH) and are
analyzed over a lossy decode, so CRITICAL matches keep blocking.
"Code file" and "executable magic" were defined separately in the
installer and SkillScan and had drifted: SkillScan missed 32-bit
little-endian and fat Mach-O variants the installer blocks. Both rules
now live in skills/package_files.py, shared by the installer, the export
guard, and SkillScan.
* docs(changelog): link the skill review gate fix to #5431
* fix(skills): fail closed on bytes-less snapshot entries and skip text rules for executables
The review analyzer skipped any snapshot entry it could not turn into
bytes. Readers only emit such entries for oversized files, and they also
mark the snapshot truncated, but content_base64 is optional in the
contract, so a reader regression or a hand-built snapshot would silently
drop a file from SkillScan. An entry without bytes now fails the scan
closed (not_assessed: skillscan) unless the snapshot is truncated, and a
text entry without content no longer materializes as an empty file.
A real executable under scripts/ is a code file, so SkillScan decoded it
lossily and ran the text rules over its string tables. An OpenSSH binary
produced a CRITICAL secret-private-key finding from the key-format
banner it embeds. An undecodable file with executable magic still
reports package-undecodable-script, and its CRITICAL
package-executable-binary finding already blocks it, so it now skips the
text rules. Decodable files keep full text analysis.
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(video): forward --aspect-ratio into the Gemini Veo request
The video-generation skill accepts --aspect-ratio and passes it into
generate_video(), but the Gemini branch drops the value:
_generate_video_gemini has no aspect_ratio parameter and builds the
predictLongRunning body with only instances, so every Veo request runs
at the default ratio regardless of the CLI flag. Forward the value as
parameters.aspectRatio and cover it with a monkeypatch regression test
that captures the outgoing request body.
* fix(skills): avoid false credential findings in video generation
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
Normalize YAML frontmatter keys in the shared parser so validation and review report malformed fields instead of failing while sorting mixed key types.
