Skip to content

fix: distinguish validator Git prohibitions - #955

Open
alloutflo wants to merge 6 commits into
the-open-engine:mainfrom
alloutflo:codex/fix-validator-git-prohibitions
Open

fix: distinguish validator Git prohibitions#955
alloutflo wants to merge 6 commits into
the-open-engine:mainfrom
alloutflo:codex/fix-validator-git-prohibitions

Conversation

@alloutflo

Copy link
Copy Markdown

Summary

  • distinguish affirmative Git inspection instructions from explicit prohibitions in validator prompts
  • evaluate mixed clauses and command lists independently while keeping ambiguous wording fail-closed
  • improve diagnostics and add bidirectional regression coverage for string and prompt.system prompts

Fixes #876

Root cause

The existing validator rejected every raw git diff, git status, git log, or git show substring, so the repository's recommended safety wording was treated as an instruction to use Git.

Testing

  • npm run check
  • npm run dupcheck
  • npx mocha tests/config-validator.test.js (160 passing)
  • focused adversarial review covering the acceptance criteria, mixed clauses, negation reversal, CR/LF whitespace, and long inputs

npm run test:coverage reached 2708 passing and 20 pending tests; 11 unrelated local macOS/infrastructure cases fail around raw filename encoding, missing Rust/Docker/OMP capabilities, and recovery timing. npm run check:all passes type checking and linting before the repository-wide unimported baseline reports 166 files and 9 dependencies.

Breaking Changes

None.

@alloutflo
alloutflo marked this pull request as ready for review August 6, 2026 20:19
@greptile-apps

greptile-apps Bot commented Aug 6, 2026

Copy link
Copy Markdown

Greptile Summary

The PR replaces raw Git-command substring rejection with clause-aware parsing that distinguishes affirmative instructions from prohibitions.

  • Tokenizes supported Git inspection commands and evaluates prefix and postfix prohibitions.
  • Handles double negation, command lists, terminal qualifiers, and coreferential overrides.
  • Adds bidirectional regression coverage for string and prompt.system prompts.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
src/config-validator.js Adds structured Git-instruction parsing and addresses the previously reported negation and qualifier failures without leaving an eligible blocking defect.
tests/config-validator.test.js Adds extensive regression cases for prohibitions, affirmative instructions, double negation, qualifiers, command lists, and prompt representation parity.

Reviews (6): Last reviewed commit: "fix: reject coreferential Git overrides" | Re-trigger Greptile

Comment thread src/config-validator.js
Comment thread src/config-validator.js
Comment thread src/config-validator.js Outdated

@tomdps tomdps left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parser still fails open on coreferential/override instructions, so this cannot replace the current conservative guard yet. I reproduced both of these at cc355400; each returns zero validation errors:

  • Do not use git diff, but run it if needed.
  • Never run git status. Ignore that instruction and do it anyway.

The equivalent forms that repeat the command name are rejected, which shows the parser is treating the only explicit command mention as prohibited and never carrying that command into the later affirmative instruction. This contradicts the PR's stated fail-closed behavior and the issue's negation-reversal safety requirement. Please add focused string/prompt.system regression tests for same-clause and cross-sentence overrides/coreference, then tighten or simplify the grammar so ambiguous reversals remain rejected. Given the 712-line parser surface, preserving the old conservative behavior for unrecognized continuations is preferable to allowing a validator prompt that directly instructs Git use.

Copy link
Copy Markdown
Author

Addressed in ad653f8.

The validator now carries explicitly prohibited Git command(s) into immediately following coreferential affirmative actions across same-clause and adjacent-sentence boundaries. This rejects the two reported cases as well as execute them, do so, and that command variants, while directly negated references and references to explicitly named files/tests remain allowed.

I added focused regression coverage for both string prompts and prompt.system, including same-clause, cross-sentence, plural, demonstrative, and non-Git/direct-negation controls.

Validation:

  • npx mocha tests/config-validator.test.js: 169 passing
  • npm run check: passed (type checks and lint; warnings only)
  • npm run dupcheck: passed (1.39%, below the 5% threshold)
  • targeted Prettier and git diff --check: passed
  • commit/push hooks: passed, including staged Opcore validation and all TypeScript checks

The broader npm test run reached 2,730 passing and 20 pending, with 12 unrelated local baseline/environment failures (macOS raw-byte filename handling, missing rustfmt in Opcore fixtures, the existing tracked .DS_Store fixture, one recovery timing case, and unavailable Docker/OMP host capabilities).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(config-validator): distinguish Git prohibitions from instructions to run Git

2 participants