diff --git a/editor/CMakeLists.txt b/editor/CMakeLists.txt index 3b931a4..3f68f4c 100644 --- a/editor/CMakeLists.txt +++ b/editor/CMakeLists.txt @@ -3640,4 +3640,13 @@ target_link_libraries(step528_test PRIVATE tree_sitter_javascript tree_sitter_typescript tree_sitter_java tree_sitter_rust tree_sitter_go) +add_executable(step529_test tests/step529_test.cpp) +target_include_directories(step529_test PRIVATE src) +target_link_libraries(step529_test PRIVATE + nlohmann_json::nlohmann_json + unofficial::tree-sitter::tree-sitter + tree_sitter_python tree_sitter_cpp tree_sitter_elisp + tree_sitter_javascript tree_sitter_typescript + tree_sitter_java tree_sitter_rust tree_sitter_go) + # Step 12: Dear ImGui shell scaffolding created (main.cpp exists but not built due to dependencies) diff --git a/editor/src/PreApplyValidationGate.h b/editor/src/PreApplyValidationGate.h new file mode 100644 index 0000000..d1c3ee7 --- /dev/null +++ b/editor/src/PreApplyValidationGate.h @@ -0,0 +1,130 @@ +#pragma once +// Step 529: Pre-Apply Validation Gate + +#include +#include +#include +#include + +#include + +#include "ConstraintViolationDiagnostics.h" + +using json = nlohmann::json; + +struct CandidateEdit { + std::string nodeKind; + std::string operation; + std::vector referencedSymbols; +}; + +struct PreApplyValidationResult { + bool allowed = false; + bool mayMutateBuffer = false; + TaskitemContract contract; + ScopeSnapshot scopeSnapshot; + ConstraintDiagnosticPacket diagnostics; +}; + +class PreApplyValidationGate { +public: + static PreApplyValidationResult validate(const json& taskitemJson, + const CandidateEdit& edit, + const std::vector& visibleSymbols, + const LegalOperationGraph& graph, + const SymbolScopeExtractor& scopeExtractor) { + PreApplyValidationResult result; + + auto schema = TypedTaskitemContractSchema::parseAndValidate(taskitemJson); + if (!schema.valid) { + result.diagnostics = schemaFailurePacket(schema.errors); + return result; + } + result.contract = schema.contract; + + std::set forbidden(result.contract.forbiddenSymbols.begin(), + result.contract.forbiddenSymbols.end()); + result.scopeSnapshot = scopeExtractor.buildSnapshot(result.contract.language, + edit.nodeKind, + visibleSymbols, + forbidden); + + ConstraintCheckInput input; + input.contract = result.contract; + input.language = result.contract.language; + input.nodeKind = edit.nodeKind; + input.requestedOp = edit.operation; + input.requestedSymbols = edit.referencedSymbols; + input.scopeSnapshot = result.scopeSnapshot; + + result.diagnostics = ConstraintViolationDiagnostics::evaluate(input, graph, scopeExtractor); + + if (!targetAllowed(result.contract, edit.nodeKind)) { + result.diagnostics.violations.push_back({ + "target_out_of_contract", + "Target node kind is outside contract allowedTargets", + edit.nodeKind, + false + }); + result.diagnostics.ok = false; + result.diagnostics.recommendedAction = "escalate"; + } + + dedupe(result.diagnostics.violations); + result.allowed = result.diagnostics.violations.empty(); + result.mayMutateBuffer = result.allowed; + result.diagnostics.ok = result.allowed; + if (result.allowed) { + result.diagnostics.recommendedAction = "proceed"; + } else if (result.diagnostics.recommendedAction == "proceed") { + result.diagnostics.recommendedAction = deriveAction(result.diagnostics.violations); + } + return result; + } + +private: + static bool targetAllowed(const TaskitemContract& contract, + const std::string& nodeKind) { + return std::find(contract.allowedTargets.begin(), + contract.allowedTargets.end(), + nodeKind) != contract.allowedTargets.end(); + } + + static void dedupe(std::vector& violations) { + std::vector out; + for (const auto& v : violations) { + bool exists = false; + for (const auto& current : out) { + if (current.code == v.code && current.field == v.field) { + exists = true; + break; + } + } + if (!exists) out.push_back(v); + } + violations.swap(out); + } + + static std::string deriveAction(const std::vector& violations) { + for (const auto& v : violations) { + if (!v.retryable) return "escalate"; + } + return violations.empty() ? "proceed" : "retry"; + } + + static ConstraintDiagnosticPacket schemaFailurePacket( + const std::vector& errors) { + ConstraintDiagnosticPacket packet; + packet.ok = false; + packet.recommendedAction = "escalate"; + for (const auto& error : errors) { + packet.violations.push_back({ + "under_constrained_contract", + "Taskitem contract failed schema validation", + error, + false + }); + } + return packet; + } +}; diff --git a/editor/tests/step529_test.cpp b/editor/tests/step529_test.cpp new file mode 100644 index 0000000..27b4d42 --- /dev/null +++ b/editor/tests/step529_test.cpp @@ -0,0 +1,202 @@ +// Step 529: Pre-Apply Validation Gate (12 tests) + +#include "PreApplyValidationGate.h" + +#include + +static int passed = 0, failed = 0; +#define TEST(name) { std::cout << " " << #name << "... "; } +#define PASS() { std::cout << "PASS\n"; ++passed; } +#define FAIL(msg) { std::cout << "FAIL: " << msg << "\n"; ++failed; } +#define CHECK(cond, msg) if (!(cond)) { FAIL(msg); return; } else {} + +static json validTaskitem() { + return { + {"id", "ti-529"}, + {"nodeId", "n-1"}, + {"language", "cpp"}, + {"allowedTargets", json::array({"Function"})}, + {"allowedOps", json::array({"rename", "update"})}, + {"allowedSymbols", json::array({"cursor", "count", "EditorState", "render", "ghost"})}, + {"forbiddenSymbols", json::array({"unsafeGlobal"})}, + {"expectedDiagnosticsAdd", json::array({"none"})}, + {"expectedDiagnosticsRemove", json::array()} + }; +} + +static std::vector visibleSymbols() { + return { + {"cursor", "Variable", 3}, + {"count", "Parameter", 3}, + {"EditorState", "Type", 0}, + {"render", "Function", 1}, + {"unsafeGlobal", "Variable", 0} + }; +} + +static bool hasCode(const PreApplyValidationResult& result, + const std::string& code) { + for (const auto& v : result.diagnostics.violations) { + if (v.code == code) return true; + } + return false; +} + +void test_valid_edit_is_allowed_before_apply() { + TEST(valid_edit_is_allowed_before_apply); + LegalOperationGraph graph; + SymbolScopeExtractor extractor; + CandidateEdit edit{"Function", "rename", {"cursor", "count"}}; + auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor); + CHECK(result.allowed, "valid edit should be allowed"); + CHECK(result.mayMutateBuffer, "valid edit may mutate buffer"); + CHECK(result.diagnostics.recommendedAction == "proceed", "valid edit should proceed"); + PASS(); +} + +void test_illegal_operation_rejected_before_mutation() { + TEST(illegal_operation_rejected_before_mutation); + LegalOperationGraph graph; + SymbolScopeExtractor extractor; + CandidateEdit edit{"Function", "delete", {"cursor"}}; + auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor); + CHECK(!result.allowed, "illegal op should be rejected"); + CHECK(!result.mayMutateBuffer, "illegal op should block mutation"); + CHECK(hasCode(result, "illegal_operation"), "missing illegal operation diagnostic"); + PASS(); +} + +void test_out_of_scope_symbol_rejected_before_mutation() { + TEST(out_of_scope_symbol_rejected_before_mutation); + LegalOperationGraph graph; + SymbolScopeExtractor extractor; + CandidateEdit edit{"Function", "rename", {"ghost"}}; + auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor); + CHECK(!result.allowed, "out-of-scope symbol should be rejected"); + CHECK(!result.mayMutateBuffer, "out-of-scope symbol should block mutation"); + CHECK(hasCode(result, "out_of_scope_symbol"), "missing out-of-scope diagnostic"); + PASS(); +} + +void test_unsupported_context_rejected_before_mutation() { + TEST(unsupported_context_rejected_before_mutation); + LegalOperationGraph graph; + SymbolScopeExtractor extractor; + CandidateEdit edit{"MacroBlock", "rename", {"cursor"}}; + auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor); + CHECK(!result.allowed, "unsupported context should be rejected"); + CHECK(hasCode(result, "unsupported_context"), "missing unsupported context diagnostic"); + PASS(); +} + +void test_target_outside_allowed_targets_rejected() { + TEST(target_outside_allowed_targets_rejected); + LegalOperationGraph graph; + SymbolScopeExtractor extractor; + CandidateEdit edit{"Class", "rename", {"cursor"}}; + auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor); + CHECK(!result.allowed, "target outside contract should be rejected"); + CHECK(hasCode(result, "target_out_of_contract"), "missing target contract diagnostic"); + PASS(); +} + +void test_forbidden_symbol_rejected() { + TEST(forbidden_symbol_rejected); + LegalOperationGraph graph; + SymbolScopeExtractor extractor; + CandidateEdit edit{"Function", "rename", {"unsafeGlobal"}}; + auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor); + CHECK(!result.allowed, "forbidden symbol should be rejected"); + CHECK(hasCode(result, "forbidden_symbol"), "missing forbidden symbol diagnostic"); + PASS(); +} + +void test_missing_schema_field_rejected() { + TEST(missing_schema_field_rejected); + LegalOperationGraph graph; + SymbolScopeExtractor extractor; + auto item = validTaskitem(); + item.erase("allowedOps"); + CandidateEdit edit{"Function", "rename", {"cursor"}}; + auto result = PreApplyValidationGate::validate(item, edit, visibleSymbols(), graph, extractor); + CHECK(!result.allowed, "schema-invalid taskitem should be rejected"); + CHECK(hasCode(result, "under_constrained_contract"), "missing schema violation code"); + PASS(); +} + +void test_duplicate_allowed_ops_rejected_by_schema() { + TEST(duplicate_allowed_ops_rejected_by_schema); + LegalOperationGraph graph; + SymbolScopeExtractor extractor; + auto item = validTaskitem(); + item["allowedOps"] = json::array({"rename", "rename"}); + CandidateEdit edit{"Function", "rename", {"cursor"}}; + auto result = PreApplyValidationGate::validate(item, edit, visibleSymbols(), graph, extractor); + CHECK(!result.allowed, "duplicate allowedOps should fail schema gate"); + CHECK(hasCode(result, "under_constrained_contract"), "expected schema rejection diagnostic"); + PASS(); +} + +void test_retry_action_for_retryable_only_failure() { + TEST(retry_action_for_retryable_only_failure); + LegalOperationGraph graph; + SymbolScopeExtractor extractor; + CandidateEdit edit{"Function", "rename", {"ghost"}}; + auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor); + CHECK(result.diagnostics.recommendedAction == "retry", "retryable-only failure should retry"); + PASS(); +} + +void test_multiple_failures_are_aggregated() { + TEST(multiple_failures_are_aggregated); + LegalOperationGraph graph; + SymbolScopeExtractor extractor; + CandidateEdit edit{"Class", "delete", {"unsafeGlobal"}}; + auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor); + CHECK(hasCode(result, "target_out_of_contract"), "missing target violation"); + CHECK(hasCode(result, "unsupported_context"), "missing unsupported context violation"); + CHECK(hasCode(result, "forbidden_symbol"), "missing forbidden symbol violation"); + PASS(); +} + +void test_empty_symbol_list_can_still_validate() { + TEST(empty_symbol_list_can_still_validate); + LegalOperationGraph graph; + SymbolScopeExtractor extractor; + CandidateEdit edit{"Function", "rename", {}}; + auto result = PreApplyValidationGate::validate(validTaskitem(), edit, visibleSymbols(), graph, extractor); + CHECK(result.allowed, "empty symbol list should be valid for rename"); + PASS(); +} + +void test_forbidden_symbol_not_in_snapshot_still_reports_violation() { + TEST(forbidden_symbol_not_in_snapshot_still_reports_violation); + LegalOperationGraph graph; + SymbolScopeExtractor extractor; + CandidateEdit edit{"Function", "rename", {"unsafeGlobal"}}; + auto result = PreApplyValidationGate::validate(validTaskitem(), edit, { + {"cursor", "Variable", 3} + }, graph, extractor); + CHECK(hasCode(result, "forbidden_symbol"), "forbidden symbol should still be diagnosed"); + PASS(); +} + +int main() { + std::cout << "Step 529: Pre-Apply Validation Gate\n"; + + test_valid_edit_is_allowed_before_apply(); // 1 + test_illegal_operation_rejected_before_mutation(); // 2 + test_out_of_scope_symbol_rejected_before_mutation(); // 3 + test_unsupported_context_rejected_before_mutation(); // 4 + test_target_outside_allowed_targets_rejected(); // 5 + test_forbidden_symbol_rejected(); // 6 + test_missing_schema_field_rejected(); // 7 + test_duplicate_allowed_ops_rejected_by_schema(); // 8 + test_retry_action_for_retryable_only_failure(); // 9 + test_multiple_failures_are_aggregated(); // 10 + test_empty_symbol_list_can_still_validate(); // 11 + test_forbidden_symbol_not_in_snapshot_still_reports_violation();// 12 + + std::cout << "\nResults: " << passed << "/" << (passed + failed) << " passed\n"; + return failed == 0 ? 0 : 1; +} diff --git a/progress.md b/progress.md index f2ee5f7..2709ff2 100644 --- a/progress.md +++ b/progress.md @@ -8738,3 +8738,41 @@ constraint diagnostics before execution is allowed. - **Steps completed:** 5 - **New tests in phase plan:** 56/56 passing - **Constraint model + diagnostics + integration:** PASS + +### Step 529: Pre-Apply Validation Gate +**Status:** PASS (12/12 tests) + +Adds a deterministic pre-apply gate that validates candidate edits against +contract schema, allowed targets, legal operation graph, and in-scope symbol +snapshot before any mutation is allowed. + +**Files added:** +- `editor/src/PreApplyValidationGate.h` - pre-apply validation module: + - schema-first taskitem validation gate + - forbidden-aware scope snapshot build for candidate edit context + - operation/symbol constraint evaluation via structured diagnostics + - explicit `allowed` and `mayMutateBuffer` decision flags + - target-kind contract enforcement (`target_out_of_contract`) +- `editor/tests/step529_test.cpp` - 12 tests covering: + - valid pre-apply allow path + - illegal op, unsupported context, and out-of-scope symbol rejections + - target contract mismatch rejection + - forbidden symbol rejection + - schema-failure rejection paths + - retry vs escalate action semantics + - multi-violation aggregation behavior + - empty symbol-list edge case + +**Files modified:** +- `editor/CMakeLists.txt` - `step529_test` target + +**Verification run:** +- `cmake -S editor -B editor/build-native` - PASS +- `cmake --build editor/build-native --target step529_test step528_test` - PASS +- `./editor/build-native/step529_test` - PASS (12/12) +- `./editor/build-native/step528_test` - PASS (8/8) regression coverage + +**Architecture gate check:** +- `editor/src/PreApplyValidationGate.h` within header-size limit (`130` <= `600`) +- `editor/tests/step529_test.cpp` within test-file size guidance (`202` lines) +- Header-only architecture and naming conventions remain aligned with `ARCHITECTURE.md`