fix(core): state perms, contract validation, cleanup + unit-test coverage#420
Merged
Conversation
…rage - #72: write guard state files with mode 0600 (owner-only); they hold goal text/contract/evidence and were landing at the default umask (world-readable). - #151: validate/coerce goal_contract string-array fields (acceptanceCriteria, requirements, inferred, nonGoals) via coerceStringList so a non-string element is dropped instead of stored as "[object Object]". - #218: add persistence cleanup()/remove() to prune orphaned per-worktree state files by on-disk TTL (DEFAULT_PERSIST_TTL_MS = 7d), preserving the current worktree and fresh siblings; sweep stray .tmp files; ignore unrelated files. - #244: prefix every blocked-completion rewrite with an unambiguous guard sentinel ("⛔ [Goal Guard · blocked completion]") so an intervention can't be confused with a benign "I am not done yet" message; marker flip is preserved. - #262: deepen logger.js coverage (synthetic-turn edge cases, guardPrompt error path/return values, emitGoalCompleted format, best-effort info/warn/toast). - #291: add direct unit tests for completion/events/system/summary/agents. - #373: make plugin.test.mjs assert loudly with an ERR_MODULE_NOT_FOUND hint instead of a vague falsy check when the tool hook is absent. - #374: add tests/concurrency.test.mjs proving concurrent goal sessions complete independently with no cross-contamination. - #375: add case-insensitive last-wins parseVerdict coverage (no source change). - #67: clean up temp dirs in install.test.mjs and persistence.test.mjs (t.after). Note: guard.js is owned elsewhere; the persistence cleanup primitive is added and unit-tested here, to be wired into the guard dispose/flush path by that owner.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
CodeQL js/incomplete-sanitization: the assertion escaped only [ and ] before new RegExp(); escape all regex metacharacters so the anchored match is correct regardless of GUARD_PREFIX content.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the remaining core-runtime follow-up gaps from the PR #416 hardening.
What changed
Security & correctness
persistence.jsnow writes state files withmode 0o600(owner read/write only). The snapshot carries the verbatim goal text, contract, changed-file paths, evidence and reviewer findings, all of which previously landed at the default umask (typically0644, world-readable on a shared host). The mode is set on the temp file before the atomic rename, so the target is never momentarily world-readable.goal_contractnow validates/coerces every string-array field (acceptanceCriteria,requirements,inferred,nonGoals) through a new exportedcoerceStringList: strings/finite-numbers/booleans are stringified+normalized, objects/arrays/null/NaN are dropped, so a malformed element can never be stored as"[object Object]"and render a garbled sidebar todo.cleanup()andremove()tocreatePersistence.cleanup()prunes orphaned OTHER-worktree<hash>.jsonfiles that are stale by mtime OR by their embeddedupdatedAt(default on-disk TTLDEFAULT_PERSIST_TTL_MS= 7 days, deliberately longer than the 24h in-memory TTL), always preserving the current worktree and fresh siblings, sweeping stray.tmpfiles, and never touching unrelated files. Best-effort/degrade-safe. (Note:guard.jsis owned elsewhere; the primitive is added and unit-tested here for that owner to wire into dispose/flush.)⛔ [Goal Guard · blocked completion]) so a guard intervention is searchable and visibly distinct from a benign "I am not done yet" agent message. The existing marker flip (Goal Completed→Goal Not Completed, markdown prefix preserved) and detection contract are unchanged.Test coverage
logger.jstests: synthetic-turn edge cases (mixed/tool-only/empty/null parts),guardPromptmissing-arg/throwing/return-value paths,emitGoalCompletedformat + cycle-count coercion + custom marker, and best-effortinfo/warn/toast(never throw, correct payload shape).tests/completion.test.mjs,tests/events.test.mjs,tests/system.test.mjs,tests/summary.test.mjs,tests/agents-unit.test.mjs.plugin.test.mjs"registers the goal_* tools" now fails loudly with anERR_MODULE_NOT_FOUND/npm install @opencode-ai/pluginhint instead of a vague falsy assert.tests/concurrency.test.mjs: two concurrent goal sessions complete independently with their own verdicts/gates/cycles and no cross-contamination (including per-session contextual gates).parseVerdictcoverage for mixed-case verdicts in one body (test-only;verdicts.jsalready correct).install.test.mjsandpersistence.test.mjsnow track everymkdtempSyncdir and remove them in atest.afterhook (no more leaked/tmpdirs).Verification
node --test "tests/*.test.mjs"→ 619 pass, 0 fail (was 516).node scripts/validate-opencode-config.mjs→ passes.Closes #72
Closes #151
Closes #218
Closes #244
Closes #262
Closes #291
Closes #373
Closes #374
Closes #375
Closes #67