Repository navigation
pstack statechart: output replacement deletes foreign nested files #773
Description
Summary
The new pstack statechart tool can silently delete unrelated nested files when replacing an existing output folder. foreign() checks only immediate entry names, while publish() recursively removes the previous output directory. An allowed name can therefore hide foreign descendants or even be a directory where a generated file is expected.
This contradicts the preservation contract described in #741: a run should refuse files it did not write rather than overwrite/delete them.
Affected source
Verified at commit eb4db2b55b2b7dec3d9282754cddf0a56bf4ff10:
Also rechecked against the current main source returned by GitHub at reporting time, blob 6e2ebfccdafff7900016b5cf2ed5384c84181545: the same isolated reproduction below confirms all three cases.
Observed behavior
For newly created disposable output fixtures:
tlc/notes.md:foreign(output)returns[]; publication deletes the sentinel.chart.mmd/notes.md: a directory named like an expected generated file is accepted; publication deletes its sentinel..DS_Store/notes.md: the same type-confusion case is accepted and deleted.
This does not require overlapping runs. It affects the output-replacement path, including an explicitly supplied --out directory. A root-level foreign notes.md is correctly refused by the existing check; nested/type-confusion cases are missed.
A separate symlink probe removed an accepted tlc symlink but preserved its target contents. This report does not claim recursive deletion through symlinks into their targets.
Minimal isolated reproduction (no TLC required)
Save this as repro.cjs in a checkout, then run:
node repro.cjs pstack/skills/statechart/scripts/statechart.mjsThe probe extracts and executes the unchanged actual ownership/publication function bodies in a Node VM. It creates only fresh temporary fixtures; it does not run a full TLC invocation or touch an existing output directory. No user data is needed. The fixture tree is left in the temporary directory for inspection.
// Isolated reproduction: only newly created disposable fixtures are touched.
const fs = require('node:fs');
const path = require('node:path');
const os = require('node:os');
const vm = require('node:vm');
const assert = require('node:assert/strict');
const source = fs.readFileSync(process.argv[2] || 'pstack/skills/statechart/scripts/statechart.mjs', 'utf8');
const start = source.indexOf('const OWN =');
const end = source.indexOf('function secondModel(', start);
assert(start >= 0 && end > start, 'Unexpected upstream source layout');
const context = { fs, path, rel: String };
vm.createContext(context);
vm.runInContext('class CannotFinish extends Error {}\n' + source.slice(start, end) + '\nglobalThis.probe = { foreign, publish };', context);
const scratch = fs.mkdtempSync(path.join(os.tmpdir(), 'statechart-ownership-repro-'));
for (const [name, relative] of [['nested-tlc', 'tlc/notes.md'], ['wrong-type-chart', 'chart.mmd/notes.md'], ['wrong-type-dsstore', '.DS_Store/notes.md']]) {
const output = path.join(scratch, name, 'output');
const work = path.join(scratch, name, 'work');
const sentinel = path.join(output, relative);
fs.mkdirSync(path.dirname(sentinel), { recursive: true });
fs.mkdirSync(work, { recursive: true });
fs.writeFileSync(sentinel, 'disposable sentinel');
fs.writeFileSync(path.join(work, 'chart.mmd'), 'replacement');
const foreign = Array.from(context.probe.foreign(output));
context.probe.publish(work, output, new Map());
const preserved = fs.existsSync(sentinel);
console.log(JSON.stringify({ case: name, foreign, sentinelPreserved: preserved }));
assert.equal(foreign.length, 0);
assert.equal(preserved, false);
}Observed output:
{"case":"nested-tlc","foreign":[],"sentinelPreserved":false}
{"case":"wrong-type-chart","foreign":[],"sentinelPreserved":false}
{"case":"wrong-type-dsstore","foreign":[],"sentinelPreserved":false}Expected: reject the foreign nested content/wrong entry type, identify the offending path, and preserve the existing directory contents.
Suggested regression coverage / correction
- Type-check reserved generated leaves; a directory named
chart.mmdor.DS_Storemust not imply ownership of all its descendants. - Validate the permitted
tlcsubtree rather than accepting everything beneath its top-level name; reject unexpected descendants, symlinks and special/unreadable entries without following them. - Apply preservation checks at the existing initial-selection and publication/moved-aside boundaries, including foreign content appearing during a run.
- Keep safe rerun behavior for valid generated outputs, and ensure a failed replacement restores or retains the original contents with their location reported.
- Treat concurrent mutation separately: a recursive scan alone does not establish that a subsequent recursive deletion cannot race a writer.
Verification and limits
- Environment: Node
v22.22.2, Linux aarch64. - Existing statechart test suite: 27 passed, 31 skipped, 0 failed; TLC is unavailable here. Existing root-foreign-file cases do not exercise the nested/type cases above.
- The isolated ownership/publication reproduction above was executed successfully. It is not a claim of a full Java/TLC or PNG end-to-end run.
Activity
deanrie commentedon Oct 23, 2026
Hey @raiscan, reproduced on today's
main, after #776 and #779 moved the lines:OWNis still matched against immediate entry names only (foreign(), now L668), with no recursion and noisDirectorycheck, andpublish()(L757) ends withfs.rmSync(old, { recursive: true, force: true })afterrefuseForeign(out, foreign(old))has passed. Sotlc/notes.md, a directory namedchart.mmd, and a directory named.DS_Storeall survive the check and go with the folder; a root-levelnotes.mdis refused as you say. The symlink observation matches too: the link is removed, the target is left. The fix that keeps #741's contract is a recursiveforeign()that ownstlconly as a directory of the files the run writes there and treats an owned name of the wrong type as foreign; your VM probe is a ready-made test input for the three cases. @poteto has been landing statechart fixes daily, so this is his to take first; if it is still open at midday I will put up a PR with those tests.deanrie commentedon Oct 23, 2026
Hey @raiscan, #785 is up with the fix and your three fixtures plus two more (a symlink named
chart.mmd, refused with its target untouched, and a folder of only owned files passing to the next step).OWNnow covers top-level files only,OWN_TLCcovers what the script writes intotlc/(*.cfg,*.out,*.dot,regions-proof.mmd,regions-all.mmd),foreign()reads entry types and looks one level intotlc/, and a folder or link under an owned name is reported by its path. If your VM probe still has the fixtures around, running it against the branch is the one check I could not do with your exact setup.fix(pstack): statechart refuses foreign files nested under tlc/ or be…