fix: close truth and portability review gaps

This commit is contained in:
haelyra
2026-08-29 14:55:14 -04:00
parent d08331f14e
commit 30c41a9bde
12 changed files with 216 additions and 47 deletions
+64 -28
View File
@@ -68,9 +68,41 @@ function extractFrontmatter(content) {
* @param {string[]} lines
* @returns {{values: Record<string,string>, descriptionIndicator: string|null}}
*/
function stripUnquotedYamlComment(rawValue) {
let inSingleQuote = false;
let inDoubleQuote = false;
for (let index = 0; index < rawValue.length; index++) {
const character = rawValue[index];
if (inDoubleQuote && character === '\\') {
index += 1;
continue;
}
if (!inDoubleQuote && character === "'") {
if (inSingleQuote && rawValue[index + 1] === "'") {
index += 1;
} else {
inSingleQuote = !inSingleQuote;
}
continue;
}
if (!inSingleQuote && character === '"') {
inDoubleQuote = !inDoubleQuote;
continue;
}
if (!inSingleQuote && !inDoubleQuote && character === '#'
&& (index === 0 || /\s/.test(rawValue[index - 1]))) {
return rawValue.slice(0, index).trim();
}
}
return rawValue.trim();
}
function inspectFrontmatter(lines) {
const values = Object.create(null);
const syntaxErrors = [];
let syntaxErrors = [];
let descriptionIndicator = null;
let inBlockScalar = false;
let blockScalarIndent = -1;
@@ -92,13 +124,8 @@ function inspectFrontmatter(lines) {
const key = match[1];
const rawValue = match[2];
// Strip unquoted comments for value/indicator inspection. Handles both
// trailing comments (`foo: bar # note`) and comment-only values
// (`foo: # todo`) so the latter is treated as empty.
const valueNoComment = rawValue
.replace(/^\s*#.*$/, '')
.replace(/\s+#.*$/, '')
.trim();
// Strip YAML comments only when # appears outside a quoted scalar.
const valueNoComment = stripUnquotedYamlComment(rawValue);
values[key] = valueNoComment;
const isQuoted = /^"(?:[^"\\]|\\.)*"$/.test(valueNoComment) || /^'(?:[^']|'')*'$/.test(valueNoComment);
@@ -109,16 +136,19 @@ function inspectFrontmatter(lines) {
// drops a value's quoting, or glues the next frontmatter key onto
// the end of a value, this is exactly what shows up (see #2630).
if (valueNoComment.includes(': ')) {
syntaxErrors.push(
syntaxErrors = [...syntaxErrors,
`${key}: unquoted value contains ': ' — invalid YAML; ` + `quote the value or the next key was likely glued onto this line`
);
];
}
// '@' and '`' are reserved YAML indicators and cannot start a
// plain scalar (see #2630 — a reordering during translation moved
// '@' into the first column of an unquoted description).
if (/^[@`]/.test(valueNoComment)) {
syntaxErrors.push(`${key}: unquoted value starts with reserved character '${valueNoComment[0]}' — quote the value`);
syntaxErrors = [
...syntaxErrors,
`${key}: unquoted value starts with reserved character '${valueNoComment[0]}' — quote the value`
];
}
}
@@ -246,30 +276,31 @@ function validateSkillFile(skillMd, label, reportFrontmatterFinding, opts = {})
function findDocsSkillFiles(docsDir) {
if (!fs.existsSync(docsDir)) return [];
const files = [];
const locales = fs
.readdirSync(docsDir, { withFileTypes: true })
const readDirectories = (directory, label) => {
try {
return fs.readdirSync(directory, { withFileTypes: true });
} catch {
throw new Error(`unable to read ${label}`);
}
};
const locales = readDirectories(docsDir, 'docs directory')
.filter(e => e.isDirectory() && !e.name.startsWith('.'))
.map(e => e.name);
for (const locale of locales) {
return locales.flatMap(locale => {
const localeSkillsDir = path.join(docsDir, locale, 'skills');
if (!fs.existsSync(localeSkillsDir)) continue;
if (!fs.existsSync(localeSkillsDir)) return [];
const skillDirs = fs
.readdirSync(localeSkillsDir, { withFileTypes: true })
const skillDirs = readDirectories(localeSkillsDir, `docs/${locale}/skills directory`)
.filter(e => e.isDirectory() && !e.name.startsWith('.'))
.map(e => e.name);
for (const skillDir of skillDirs) {
files.push({
skillMd: path.join(localeSkillsDir, skillDir, 'SKILL.md'),
label: `docs/${locale}/skills/${skillDir}/SKILL.md`
});
}
}
return files;
return skillDirs.map(skillDir => ({
skillMd: path.join(localeSkillsDir, skillDir, 'SKILL.md'),
label: `docs/${locale}/skills/${skillDir}/SKILL.md`
}));
});
}
function validateSkills() {
@@ -329,4 +360,9 @@ function validateSkills() {
console.log(msg);
}
validateSkills();
try {
validateSkills();
} catch (error) {
console.error(`ERROR: ${error.message}`);
process.exit(1);
}
+6 -5
View File
@@ -162,10 +162,11 @@ function readLatestContextTokens(transcriptPath, options = {}) {
* positively detected or merely assumed.
*
* `inferred: false` means the size came from evidence — an explicit env
* override, the `[1m]` marker, a known large-window family, or an observed
* token count that already exceeds the standard window. `inferred: true` means
* every check fell through and the standard 200k default was assumed; the
* window may actually be larger and callers must not present it as fact.
* override, the `[1m]` marker, or a known large-window family. An observed
* token count above the standard window selects the safer large-window
* thresholds, but remains inferred because the true denominator could be an
* unmarked intermediate size such as 400k. Callers must not present inferred
* windows as fact.
*
* @returns {{ windowTokens: number, inferred: boolean }}
*/
@@ -193,7 +194,7 @@ function resolveContextWindow(tokens, model) {
}
if (Number.isFinite(tokens) && tokens > STANDARD_CONTEXT_WINDOW_TOKENS) {
return { windowTokens: LARGE_CONTEXT_WINDOW_TOKENS, inferred: false };
return { windowTokens: LARGE_CONTEXT_WINDOW_TOKENS, inferred: true };
}
return { windowTokens: STANDARD_CONTEXT_WINDOW_TOKENS, inferred: true };
+1
View File
@@ -8,6 +8,7 @@ dependencies = ["pyyaml>=6.0"]
[tool.pytest.ini_options]
testpaths = ["tests"]
pythonpath = ["."]
markers = ["unit: isolated tests without external services"]
[dependency-groups]
dev = [
+16 -2
View File
@@ -149,11 +149,25 @@ def _redact_home_path(text: str) -> str:
def _redact_home_paths(value: object) -> object:
"""Return a copy with home paths redacted from every string leaf."""
"""Return a copy with home paths redacted from string keys and leaves.
Redacted mapping keys receive a stable numeric suffix when two original
keys collapse to the same portable value. This preserves every observation
without leaking the original home path or silently dropping data.
"""
if isinstance(value, str):
return _redact_home_path(value)
if isinstance(value, dict):
return {key: _redact_home_paths(item) for key, item in value.items()}
redacted: dict[object, object] = {}
for key, item in value.items():
redacted_key = _redact_home_path(key) if isinstance(key, str) else key
candidate = redacted_key
suffix = 2
while candidate in redacted:
candidate = f"{redacted_key}#{suffix}"
suffix += 1
redacted[candidate] = _redact_home_paths(item)
return redacted
if isinstance(value, list):
return [_redact_home_paths(item) for item in value]
return value
+22
View File
@@ -144,6 +144,7 @@ class TestRunScenarioMaxTurnsTermination:
run_scenario(scenario, model="haiku")
@pytest.mark.unit
class TestParseStreamJsonRedactsHomePath:
"""Observations feed grade() and then a written report (results/<skill>.md) —
a raw absolute path bakes the operator's username into every tool call
@@ -209,6 +210,27 @@ class TestParseStreamJsonRedactsHomePath:
"~/資料.txt",
]
def test_mapping_keys_are_redacted_without_silent_collision(
self, monkeypatch: pytest.MonkeyPatch
) -> None:
home = r"C:\Users\Zoë"
self._set_home(monkeypatch, home)
stdout = self._stream_json_for(
{
home + r"\private.txt": "first",
r"c:\users\zoë\private.txt": "second",
},
"irrelevant output",
)
events = _parse_stream_json(stdout)
parsed_input = json.loads(events[0].input)
assert home not in events[0].input
assert parsed_input == {
r"~\private.txt": "first",
r"~\private.txt#2": "second",
}
def test_sibling_and_embedded_prefix_paths_untouched(
self, monkeypatch: pytest.MonkeyPatch
) -> None:
+23 -2
View File
@@ -13,6 +13,27 @@
set -euo pipefail
sort_nul_file() {
local input_file="$1"
local sorted_file="${input_file}.sorted"
node -e '
const fs = require("fs");
const input = fs.readFileSync(0);
const records = [];
let start = 0;
for (let index = 0; index < input.length; index += 1) {
if (input[index] === 0) {
records.push(input.subarray(start, index + 1));
start = index + 1;
}
}
if (start < input.length) records.push(input.subarray(start));
records.sort(Buffer.compare);
process.stdout.write(Buffer.concat(records));
' <"$input_file" >"$sorted_file"
mv "$sorted_file" "$input_file"
}
RESULTS_JSON="${1:-}"
CWD_SKILLS_DIR="${SKILL_STOCKTAKE_PROJECT_DIR:-${2:-$PWD/.claude/skills}}"
GLOBAL_DIR="${SKILL_STOCKTAKE_GLOBAL_DIR:-$HOME/.claude/skills}"
@@ -56,13 +77,13 @@ process_dir() {
# Capture find's exit status and stderr instead of discarding them: with -L,
# a broken symlink or unreadable directory makes find skip that entry AND
# exit non-zero, which would otherwise silently under-count skills.
# NUL-delimited (-print0 / sort -z / read -d '') so a path containing a
# NUL-delimited (-print0 / sort_nul_file / read -d '') so a path containing a
# literal newline can't desync record boundaries — paths here are untrusted.
if ! find -L "$dir" -name "SKILL.md" -type f -print0 >"$find_out" 2>"$find_err"; then
echo "Warning: find encountered errors while scanning $dir (broken symlinks or permission issues may cause skills to be missed):" >&2
cat "$find_err" >&2
fi
sort -z -o "$find_out" "$find_out"
sort_nul_file "$find_out"
while IFS= read -r -d '' file; do
local mtime dp is_new
+23 -2
View File
@@ -13,6 +13,27 @@
set -euo pipefail
sort_nul_file() {
local input_file="$1"
local sorted_file="${input_file}.sorted"
node -e '
const fs = require("fs");
const input = fs.readFileSync(0);
const records = [];
let start = 0;
for (let index = 0; index < input.length; index += 1) {
if (input[index] === 0) {
records.push(input.subarray(start, index + 1));
start = index + 1;
}
}
if (start < input.length) records.push(input.subarray(start));
records.sort(Buffer.compare);
process.stdout.write(Buffer.concat(records));
' <"$input_file" >"$sorted_file"
mv "$sorted_file" "$input_file"
}
GLOBAL_DIR="${SKILL_STOCKTAKE_GLOBAL_DIR:-$HOME/.claude/skills}"
CWD_SKILLS_DIR="${SKILL_STOCKTAKE_PROJECT_DIR:-${1:-$PWD/.claude/skills}}"
# Path to JSONL file containing tool-use observations (optional; used for usage frequency counts).
@@ -100,13 +121,13 @@ scan_dir_to_json() {
# Capture find's exit status and stderr instead of discarding them: with -L,
# a broken symlink or unreadable directory makes find skip that entry AND
# exit non-zero, which would otherwise silently under-count skills.
# NUL-delimited (-print0 / sort -z / read -d '') so a path containing a
# NUL-delimited (-print0 / sort_nul_file / read -d '') so a path containing a
# literal newline can't desync record boundaries — paths here are untrusted.
if ! find -L "$dir" -name "SKILL.md" -type f -print0 >"$find_out" 2>"$find_err"; then
echo "Warning: find encountered errors while scanning $dir (broken symlinks or permission issues may cause skills to be missed):" >&2
cat "$find_err" >&2
fi
sort -z -o "$find_out" "$find_out"
sort_nul_file "$find_out"
while IFS= read -r -d '' file; do
local name desc mtime u7 u30 dp
+15 -1
View File
@@ -52,6 +52,15 @@ function test(name, fn) {
const REGEX_CAN_FOLLOW = new Set([
'', '(', ',', '=', ':', '[', '!', '&', '|', '?', '{', '}', ';', '+', '-', '*', '%', '~', '^', '<', '>',
]);
const REGEX_CAN_FOLLOW_KEYWORD = new Set([
'await', 'case', 'delete', 'do', 'else', 'in', 'instanceof', 'new', 'of',
'return', 'throw', 'typeof', 'void', 'yield',
]);
function regexFollowsKeyword(source, slashIndex) {
const match = source.slice(0, slashIndex).match(/([A-Za-z_$][\w$]*)\s*$/);
return Boolean(match && REGEX_CAN_FOLLOW_KEYWORD.has(match[1]));
}
/**
* Blank out comments and literal text, preserving length and line breaks so
@@ -102,7 +111,7 @@ function blankCommentsAndLiterals(source) {
continue;
}
if (ch === '/' && REGEX_CAN_FOLLOW.has(prev)) {
if (ch === '/' && (REGEX_CAN_FOLLOW.has(prev) || regexFollowsKeyword(source, i))) {
emit(ch); i += 1;
let inClass = false;
while (i < source.length) {
@@ -291,6 +300,11 @@ if (test('a regex literal containing a slash does not swallow the code after it'
assert.deepStrictEqual([...readGateguardEnvNames(fixture)], ['GATEGUARD_AFTER_REGEX']);
})) passed++; else failed++;
if (test('a regex literal after a statement keyword is ignored', () => {
const fixture = 'function matches() { return /process\\.env\\.GATEGUARD_IN_RETURN_REGEX/; }';
assert.deepStrictEqual([...readGateguardEnvNames(fixture)], []);
})) passed++; else failed++;
if (test('the access guard rejects every form the parser cannot follow', () => {
const cases = [
['destructuring', 'const { GATEGUARD_HIDDEN } = process.env;'],
+25
View File
@@ -2856,6 +2856,31 @@ function runTests() {
cleanupTestDir(testDir);
})) passed++; else failed++;
if (test('preserves # inside a quoted frontmatter value', () => {
const testDir = createTestDir();
const docsDir = path.join(testDir, 'docs-root');
const skillDir = path.join(docsDir, 'ja-JP', 'skills', 'example');
fs.mkdirSync(skillDir, { recursive: true });
fs.writeFileSync(path.join(skillDir, 'SKILL.md'),
'---\nname: example\ndescription: "Fix: details #tag" # translation note\n---\n# Example');
const result = runSkillsValidator('/nonexistent/skills-dir', ['--strict'], {}, docsDir);
assert.strictEqual(result.code, 0,
`Quoted # content must remain valid, got stderr: ${result.stderr}`);
cleanupTestDir(testDir);
})) passed++; else failed++;
if (test('reports an unreadable docs root deterministically', () => {
const testDir = createTestDir();
const docsPath = path.join(testDir, 'docs-file');
fs.writeFileSync(docsPath, 'not a directory');
const result = runSkillsValidator('/nonexistent/skills-dir', ['--strict'], {}, docsPath);
assert.strictEqual(result.code, 1, 'Should fail when the docs root cannot be read');
assert.strictEqual(result.stderr.trim(), 'ERROR: unable to read docs directory');
cleanupTestDir(testDir);
})) passed++; else failed++;
if (test('flags a docs mirror SKILL.md with no frontmatter block at all', () => {
const testDir = createTestDir();
const docsDir = path.join(testDir, 'docs-root');
+4 -1
View File
@@ -698,7 +698,10 @@ function runTests() {
const ctx = createContextContext();
const transcript = writeTranscriptFixture(170000);
try {
const result = runCompactWithInput({ session_id: ctx.sessionId, transcript_path: transcript });
const result = runCompactWithInput(
{ session_id: ctx.sessionId, transcript_path: transcript },
{ ECC_CONTEXT_WINDOW_TOKENS: '', CLAUDE_CODE_AUTO_COMPACT_WINDOW: '' },
);
assert.strictEqual(result.code, 0, 'Should exit 0');
assert.ok(result.stdout.trim().length > 0, `Expected stdout payload. Got: "${result.stdout}"`);
const parsed = JSON.parse(result.stdout);
+11 -5
View File
@@ -139,6 +139,10 @@ console.log('\nresolveContextWindowTokens:');
// Isolation: an env-set window override (either knob) otherwise leaks into the
// default-window assertions below and fails them (#2290).
const originalContextWindowEnv = {
ECC_CONTEXT_WINDOW_TOKENS: process.env.ECC_CONTEXT_WINDOW_TOKENS,
CLAUDE_CODE_AUTO_COMPACT_WINDOW: process.env.CLAUDE_CODE_AUTO_COMPACT_WINDOW,
};
delete process.env.ECC_CONTEXT_WINDOW_TOKENS;
delete process.env.CLAUDE_CODE_AUTO_COMPACT_WINDOW;
@@ -222,9 +226,6 @@ test('treats an empty model id as standard window', () => {
// ── isContextWindowInferred ──
console.log('\nisContextWindowInferred:');
delete process.env.ECC_CONTEXT_WINDOW_TOKENS;
delete process.env.CLAUDE_CODE_AUTO_COMPACT_WINDOW;
test('flags the assumed 200k default as inferred', () => {
assert.strictEqual(isContextWindowInferred(187000, 'claude-opus-9'), true);
});
@@ -246,10 +247,15 @@ test('a known large-window family is a detected window, not inferred', () => {
assert.strictEqual(isContextWindowInferred(187000, 'claude-fable-5'), false);
});
test('tokens above the standard window make the size detected, not inferred', () => {
assert.strictEqual(isContextWindowInferred(220000, 'claude-opus-9'), false);
test('tokens above the standard window still leave the exact size inferred', () => {
assert.strictEqual(isContextWindowInferred(220000, 'claude-opus-9'), true);
});
for (const [name, value] of Object.entries(originalContextWindowEnv)) {
if (value === undefined) delete process.env[name];
else process.env[name] = value;
}
// ── resolveContextThreshold ──
console.log('\nresolveContextThreshold:');
@@ -47,7 +47,9 @@ test('both scanners use canonical, error-visible, NUL-delimited discovery', () =
for (const scriptPath of [scanScript, quickDiffScript]) {
const source = fs.readFileSync(scriptPath, 'utf8');
assert.match(source, /find -L "\$dir" -name "SKILL\.md" -type f -print0/);
assert.match(source, /sort -z -o "\$find_out" "\$find_out"/);
assert.match(source, /sort_nul_file "\$find_out"/);
assert.match(source, /records\.sort\(Buffer\.compare\)/);
assert.doesNotMatch(source, /sort -z/, `${path.basename(scriptPath)} still requires GNU sort`);
assert.match(source, /read -r -d '' file/);
assert.doesNotMatch(source, /find [^\n]*2>\/dev\/null/, `${path.basename(scriptPath)} still hides find errors`);
}
@@ -103,6 +105,9 @@ if (process.platform === 'win32') {
);
assert.ok(output.every(entry => entry.is_new === true));
});
} catch (error) {
console.log(` ✗ fixture setup: ${error.message}`);
failed++;
} finally {
fs.rmSync(tempRoot, { recursive: true, force: true });
}