From 6ed3a70f61c02612ac31070fb1deacb1e5dfd1c6 Mon Sep 17 00:00:00 2001 From: Lien Chen Date: Sun, 26 Jul 2026 19:45:19 +0900 Subject: [PATCH] Harden operation state writer parity --- CHANGELOG.md | 8 ++ core/state/layout.yaml | 20 ++- tests/bin/run-tests.sh | 5 +- tests/shell/test-layer-boundaries.sh | 73 +++++++++- tests/shell/test-run-tests.sh | 36 ++++- tests/shell/test-state-layout-parity.sh | 182 ++++++++++++++++++++++++ 6 files changed, 313 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 325c7676..04a4a305 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,14 @@ Versions follow [Semantic Versioning](https://semver.org/). ### Fixed +- **Operation state writes now participate in writer-boundary parity + (CC-508).** `layout.yaml` declares the three operation writer entry points + and the append-locked `children.jsonl` relation, while dropping the stale, + unimplemented `review_upsert` declaration. The production-domain ratchet now + recognizes operation projections and child relations as load-bearing state, + and the affected-test planner selects layout parity whenever either the + layout contract or state writer changes. + - **A detached gate launcher failure terminalizes its childless parent (CC-508).** `pmctl gate run` creates the parent operation before either lifecycle path starts, but the detached branch returned the launcher's status diff --git a/core/state/layout.yaml b/core/state/layout.yaml index 9b46a432..7bd2d727 100644 --- a/core/state/layout.yaml +++ b/core/state/layout.yaml @@ -11,7 +11,8 @@ # 3. store_root_default below (fallback) # $PROJECT := sha1(git rev-parse --show-toplevel) — partitioning key # $TASK_ID := pattern ^[A-Z]{1,4}-[0-9]+$ (task.schema.json) -# $TS_HEX := YYYYMMDDTHHMMSSZ-<6 hex> (run/event/review id suffix) +# $OPERATION_ID := pattern ^op-[0-9]{8}T[0-9]{6}Z-[a-f0-9]{6}$ +# $TS_HEX := YYYYMMDDTHHMMSSZ-<6 hex> (run/event/review/operation id suffix) # $YYYYMM := archive rotation period (UTC month) # $NNNN := zero-padded monotonic archive segment within $YYYYMM @@ -100,7 +101,14 @@ project_partition: file_pattern: "op-$TS_HEX.json" schema: ../schema/operation.schema.json description: "Parent-operation lifecycle records and append-only child relations." - write_strategy: write-temp-then-rename + write_strategies: + create: write-temp-then-hard-link-exclusive + update: write-temp-then-rename + child_relation: + path_pattern: "$OPERATION_ID/children.jsonl" + format: jsonl + write_strategy: append-locked + lock_base_pattern: "$OPERATION_ID/children" - path: "context-packs/" file_pattern: "$TASK_ID-$TS_HEX.json" @@ -117,13 +125,17 @@ project_partition: # Writer-boundary contract writer: - description: "Single designated write boundary. The runtime layer owns the concrete module binding; only the designated writer module may write to $STORE paths." + description: "Single designated write boundary. The runtime layer owns the concrete module binding; only the designated writer module may write to $STORE paths. entry_points enumerates every public mutating API in that module." entry_points: + - state_project_identity_write + - state_store_init - runs_append - events_append + - operation_upsert + - operation_create + - operation_child_append - task_upsert - task_delete - - review_upsert - decision_upsert - decision_delete diff --git a/tests/bin/run-tests.sh b/tests/bin/run-tests.sh index 55638dd5..aa4eca3b 100755 --- a/tests/bin/run-tests.sh +++ b/tests/bin/run-tests.sh @@ -203,7 +203,10 @@ map_path() { add_suite test-pmctl-adapter-generate; add_suite test-pmctl-artifacts add_suite test-pmctl-gate; add_suite test-pmctl-discovery; behavioral=1 ;; runtime/lib/state-writer.sh) - add_suite test-state-store; behavioral=1 ;; + add_suite test-state-store; add_suite test-state-layout-parity + add_suite test-pmctl-operation; behavioral=1 ;; + core/state/layout.yaml) + add_suite test-state-store; add_suite test-state-layout-parity; behavioral=1 ;; runtime/lib/state-paths.sh) add_suite test-state-paths; add_suite test-state-store-rotation; behavioral=1 ;; runtime/lib/pmctl-policy.sh) diff --git a/tests/shell/test-layer-boundaries.sh b/tests/shell/test-layer-boundaries.sh index cb572314..a3a7fe35 100755 --- a/tests/shell/test-layer-boundaries.sh +++ b/tests/shell/test-layer-boundaries.sh @@ -142,13 +142,15 @@ check_production_no_direct_state_writes() { || s ~ /(^|[^<])>>?/ } function load_bearing_target(s) { - return s ~ /(runs[.]jsonl|events[.]jsonl|repo[.]json|runs[.]lock|events[.]lock)/ \ - || s ~ /\/(tasks|reviews|decisions|context-packs|archive)(\/|["'\''${}[:space:]])/ \ + return s ~ /(runs[.]jsonl|events[.]jsonl|children[.](jsonl|lock|lockdir)|repo[.]json|runs[.]lock|events[.]lock)/ \ + || s ~ /(op-[0-9]{8}T[0-9]{6}Z-[a-f0-9]{6}|[$]operation_id|[$][{]operation_id[}])[.]json/ \ + || s ~ /(^|[\/"'\''[:space:]])(tasks|reviews|decisions|operations|context-packs|archive)(\/|["'\''${}[:space:]])/ \ || s ~ /\/VERSION(["'\''${}[:space:]]|$)/ } function state_scope(s) { return s ~ /(PM_DISPATCH_STATE_ROOT|_sw_store_root|_sw_project_dir|\/projects\/)/ \ - || s ~ /[$][{]?(store_root|proj_dir|version_file|runs_file|events_file|task_file|review_file|decision_file|context_pack[^}]*)[}]?/ + || s ~ /[$][{](store_root|proj_dir|version_file|runs_file|events_file|task_file|review_file|decision_file|operation_file|operation_dir|op_dir|operation_children_file|context_pack[A-Za-z0-9_]*)([^A-Za-z0-9_]|$)/ \ + || s ~ /[$](store_root|proj_dir|version_file|runs_file|events_file|task_file|review_file|decision_file|operation_file|operation_dir|op_dir|operation_children_file|context_pack[A-Za-z0-9_]*)([^A-Za-z0-9_]|$)/ } function inspect_line(raw, line_number, line, load_bearing, scoped) { line = raw @@ -340,6 +342,69 @@ if should_run "selftest/production_state_writer catches mv destination"; then rm -rf "$FIX" fi +if should_run "selftest/production_state_writer catches operation projection variable"; then + name="selftest/production_state_writer catches operation projection variable" + _fixture + printf '#!/usr/bin/env bash\nmv -f "$tmp" "$operation_file"\n' \ + > "$FIX/runtime/lib/direct-operation.sh" + _expect_fires "$name" "$(check_production_no_direct_state_writes "$FIX")" + rm -rf "$FIX" +fi + +if should_run "selftest/production_state_writer catches operation child append"; then + name="selftest/production_state_writer catches operation child append" + _fixture + printf '#!/usr/bin/env bash\nprintf "%%s\\n" "$child" >> "$entity_root/operations/$operation_id/children.jsonl"\n' \ + > "$FIX/runtime/lib/direct-operation-child.sh" + _expect_fires "$name" "$(check_production_no_direct_state_writes "$FIX")" + rm -rf "$FIX" +fi + +if should_run "selftest/production_state_writer catches split-line relative child append"; then + name="selftest/production_state_writer catches split-line relative child append" + _fixture + printf '#!/usr/bin/env bash\ncd "$op_dir"\nprintf "%%s\\n" "$child" >> children.jsonl\n' \ + > "$FIX/runtime/lib/direct-relative-operation-child.sh" + _expect_fires "$name" "$(check_production_no_direct_state_writes "$FIX")" + rm -rf "$FIX" +fi + +if should_run "selftest/production_state_writer catches relative operation projection"; then + name="selftest/production_state_writer catches relative operation projection" + _fixture + printf '#!/usr/bin/env bash\nmv -f "$tmp" "operations/$operation_id.json"\n' \ + > "$FIX/runtime/lib/direct-relative-operation.sh" + _expect_fires "$name" "$(check_production_no_direct_state_writes "$FIX")" + rm -rf "$FIX" +fi + +if should_run "selftest/production_state_writer catches split-line operation projection"; then + name="selftest/production_state_writer catches split-line operation projection" + _fixture + printf '#!/usr/bin/env bash\ncd "$op_dir"\nmv -f "$tmp" "$operation_id.json"\n' \ + > "$FIX/runtime/lib/direct-split-operation.sh" + _expect_fires "$name" "$(check_production_no_direct_state_writes "$FIX")" + rm -rf "$FIX" +fi + +if should_run "selftest/production_state_writer catches operation directory variable"; then + name="selftest/production_state_writer catches operation directory variable" + _fixture + printf '#!/usr/bin/env bash\nmkdir -p "$op_dir"\n' \ + > "$FIX/runtime/lib/direct-operation-dir.sh" + _expect_fires "$name" "$(check_production_no_direct_state_writes "$FIX")" + rm -rf "$FIX" +fi + +if should_run "selftest/production_state_writer does NOT match longer operation variable"; then + name="selftest/production_state_writer does NOT match longer operation variable" + _fixture + printf '#!/usr/bin/env bash\nmkdir -p "$op_directory_cache"\n' \ + > "$FIX/runtime/lib/unrelated-operation-cache.sh" + _expect_clean "$name" "$(check_production_no_direct_state_writes "$FIX")" + rm -rf "$FIX" +fi + if should_run "selftest/production_state_writer catches multiline mv destination"; then name="selftest/production_state_writer catches multiline mv destination" _fixture @@ -381,7 +446,7 @@ fi if should_run "selftest/production_state_writer allows state readers"; then name="selftest/production_state_writer allows state readers" _fixture - printf '#!/usr/bin/env bash\njq -c . "$proj_dir/tasks/CC-1.json"\n' \ + printf '#!/usr/bin/env bash\njq -c . "$operation_file"\n' \ > "$FIX/runtime/lib/state-reader.sh" _expect_clean "$name" "$(check_production_no_direct_state_writes "$FIX")" rm -rf "$FIX" diff --git a/tests/shell/test-run-tests.sh b/tests/shell/test-run-tests.sh index c0bc7b1f..84a7149b 100755 --- a/tests/shell/test-run-tests.sh +++ b/tests/shell/test-run-tests.sh @@ -18,7 +18,7 @@ make_fixture() { local name="$1" repo repo="$TMP_ROOT/$name" mkdir -p "$repo/scripts/lib" "$repo/runtime/lib" "$repo/tests/bin" "$repo/tests/lib" \ - "$repo/core/schema" + "$repo/core/schema" "$repo/core/state" cp "$REPO_ROOT/tests/bin/run-tests.sh" "$repo/tests/bin/run-tests.sh" cp "$REPO_ROOT/tests/bin/run-all-tests.sh" "$repo/tests/bin/run-all-tests.sh" cp "$SCRIPT_DIR/../lib/test-result.sh" "$repo/tests/lib/test-result.sh" @@ -28,7 +28,7 @@ make_fixture() { cat > "$repo/tests/lib/test-suite-runner.sh" <<'RUNNER' #!/usr/bin/env bash set -euo pipefail -suites=(lint-agents lint-scripts lint-script-domain-inventory lint-portable-repo-paths test-lint-shellcheck test-script-domain-inventory test-lint-portable-repo-paths test-lint-frontmatter test-commands test-check-docs-freshness test-guards test-migrate test-install test-doctor test-pmctl-dispatch test-pmctl-context test-pmctl-memory test-pr-gate test-host-manifest test-host-write-codex test-codex-dispatch-continuation test-host-write-parity test-core-schemas test-layer-boundaries test-pm-scripts test-run-tests) +suites=(lint-agents lint-scripts lint-script-domain-inventory lint-portable-repo-paths test-lint-shellcheck test-script-domain-inventory test-lint-portable-repo-paths test-lint-frontmatter test-commands test-check-docs-freshness test-guards test-migrate test-install test-doctor test-pmctl-dispatch test-pmctl-context test-pmctl-memory test-pr-gate test-pmctl-operation test-host-manifest test-host-write-codex test-codex-dispatch-continuation test-host-write-parity test-core-schemas test-layer-boundaries test-pm-scripts test-run-tests test-state-store test-state-layout-parity) for arg in "$@"; do if [[ "$arg" == --list ]]; then printf '%s\n' "${suites[@]}"; exit 0; fi done @@ -94,6 +94,36 @@ case_memory_config_mapping_has_no_gap() { fi } +case_state_writer_mapping_runs_operation_parity() { + local name=state-writer-mapping-runs-operation-parity repo out status=0 args + args="$TMP_ROOT/$name.args" + repo="$(make_fixture "$name")" + out=$(RUN_TESTS_ARGS_LOG="$args" "$repo/tests/bin/run-tests.sh" \ + --path runtime/lib/state-writer.sh --list 2>&1) || status=$? + if [[ "$status" -eq 0 && "$out" == *"test-state-store"* && + "$out" == *"test-state-layout-parity"* && "$out" == *"test-pmctl-operation"* && + "$out" != *"coverage gaps"* && ! -e "$args" ]]; then + pass "$name" + else + fail "$name" "status=$status out=$out executed=$([[ -e "$args" ]] && echo yes || echo no)" + fi +} + +case_state_layout_mapping_runs_parity() { + local name=state-layout-mapping-runs-parity repo out status=0 args + args="$TMP_ROOT/$name.args" + repo="$(make_fixture "$name")" + out=$(RUN_TESTS_ARGS_LOG="$args" "$repo/tests/bin/run-tests.sh" \ + --path core/state/layout.yaml --list 2>&1) || status=$? + if [[ "$status" -eq 0 && "$out" == *"test-state-store"* && + "$out" == *"test-state-layout-parity"* && "$out" != *"coverage gaps"* && + ! -e "$args" ]]; then + pass "$name" + else + fail "$name" "status=$status out=$out executed=$([[ -e "$args" ]] && echo yes || echo no)" + fi +} + case_docs_mapping_list_only() { local name=docs-mapping-list-only repo out status=0 args args="$TMP_ROOT/$name.args" @@ -362,6 +392,8 @@ case_invalid_structured_sink_fails_closed() { case_direct_library_mapping case_memory_config_mapping_has_no_gap +case_state_writer_mapping_runs_operation_parity +case_state_layout_mapping_runs_parity case_docs_mapping_list_only case_operational_docs_map_to_stale_reference_lint case_agent_mapping_uses_registered_frontmatter_suite diff --git a/tests/shell/test-state-layout-parity.sh b/tests/shell/test-state-layout-parity.sh index 51a73e2a..4bb35450 100755 --- a/tests/shell/test-state-layout-parity.sh +++ b/tests/shell/test-state-layout-parity.sh @@ -69,6 +69,73 @@ writer_lock_names() { sed -E 's/.*\$proj_dir\/([^"]+)".*/\1.lock/' } +layout_writer_entry_points() { + python3 -c ' +import sys, yaml +data = yaml.safe_load(open(sys.argv[1], encoding="utf-8")) +for name in data.get("writer", {}).get("entry_points", []): + print(name) +' "$LAYOUT" | sort +} + +writer_public_mutating_entry_points() { + awk ' + /^[a-z][a-zA-Z0-9_]*[[:space:]]*\(\)[[:space:]]*\{/ { + name = $1 + sub(/\(\).*/, "", name) + print name + } + ' "$STATE_WRITER" | + grep -Ev '^(run_transition_valid|sw_extract_task_id|sw_build_run_json)$' | + sort +} + +layout_operation_contract() { + python3 -c ' +import sys, yaml +data = yaml.safe_load(open(sys.argv[1], encoding="utf-8")) +operations = [ + item for item in data["project_partition"]["subdirs"] + if item.get("path") == "operations/" +] +if len(operations) != 1: + raise SystemExit(f"expected one operations/ declaration, found {len(operations)}") +item = operations[0] +relation = item.get("child_relation", {}) +strategies = item.get("write_strategies", {}) +print("\t".join([ + str(item.get("file_pattern", "")), + str(strategies.get("create", "")), + str(strategies.get("update", "")), + str(relation.get("path_pattern", "")), + str(relation.get("format", "")), + str(relation.get("lock_base_pattern", "")), + str(relation.get("write_strategy", "")), +])) +' "$LAYOUT" +} + +writer_operation_create_strategy() { + local body + body="$(sed -n '/^operation_create() {/,/^}/p' "$STATE_WRITER")" + if grep -Fq 'ln "$tmp" "$target"' <<< "$body"; then + printf 'write-temp-then-hard-link-exclusive\n' + fi +} + +writer_operation_update_strategy() { + local body + body="$(sed -n '/^operation_upsert() {/,/^}/p' "$STATE_WRITER")" + if grep -Fq 'mv -f "$tmp" "$proj_dir/operations/${operation_id}.json"' <<< "$body"; then + printf 'write-temp-then-rename\n' + fi +} + +writer_operation_child_lock_base_pattern() { + grep -E 'serialize_with_lock "\$op_dir/[a-zA-Z0-9_-]+"' "$STATE_WRITER" | + sed -E 's/.*"\$op_dir\/([^"]+)".*/$OPERATION_ID\/\1/' +} + writer_archive_paths() { local proj_dir="$1" entity path for entity in runs events; do @@ -211,6 +278,119 @@ case_archive_patterns_match_layout() { pass "$name" } +case_writer_entry_points_match_layout() { + # Verifies every public mutating API in state-writer.sh is declared by layout.yaml, and vice versa. + # + # Steps: + # 1. Read the machine-readable writer.entry_points list from layout.yaml. + # 2. Extract all public functions and explicitly exclude the three pure helpers. + # 3. Assert the two sorted multisets are identical, including duplicate detection. + local name="state-layout-parity: writer entry points match layout" + should_run "$name" || return 0 + local declared implemented + declared="$(layout_writer_entry_points)" + implemented="$(writer_public_mutating_entry_points)" + + if [[ "$declared" == "$implemented" ]]; then + pass "$name" + else + fail "$name" \ + "layout=$(printf '%s' "$declared" | tr '\n' ' ') writer=$(printf '%s' "$implemented" | tr '\n' ' ')" + fi +} + +case_operation_paths_match_layout() { + # Verifies operation projection strategies and their append-only child relation follow layout.yaml. + # + # Steps: + # 1. Read parent create/update strategies plus child path/format/lock-base declarations. + # 2. Exercise exclusive create, atomic update, and both child-lock implementations. + # 3. Assert concrete state and source-derived strategies match the machine contract. + local name="state-layout-parity: operation paths match layout" + should_run "$name" || return 0 + local store repo proj_dir operation_id ts_hex operation_json collision_json updated_json + local child_json fallback_child_json contract create_rc=0 + local parent_pattern create_strategy update_strategy child_pattern child_format + local child_lock_base_pattern child_strategy expected_parent expected_child + local expected_child_lock_base writer_create_strategy writer_update_strategy + local writer_child_lock_base operations_dir + operation_id="op-20260101T000000Z-abcdef" + ts_hex="${operation_id#op-}" + store="$tmp_root/operation-store" + repo="$tmp_root/operation-repo" + mkdir -p "$repo" + git -C "$repo" init -q + operation_json="$(jq -cn \ + --arg id "$operation_id" --arg dir "$repo" \ + '{schema_version:1,id:$id,kind:"gate",working_dir:$dir,executor:"codex",state:"running",created_ts:"2026-01-01T00:00:00Z"}')" + collision_json="$(jq -c '.executor = "claude"' <<< "$operation_json")" + updated_json="$(jq -c \ + '.state = "completed" | .terminal_ts = "2026-01-01T00:00:02Z"' <<< "$operation_json")" + child_json="$(jq -cn --arg dir "$repo" \ + '{run_id:"run-20260101T000001Z-fedcba",working_dir:$dir,attached_ts:"2026-01-01T00:00:01Z"}')" + fallback_child_json="$(jq -cn --arg dir "$repo" \ + '{run_id:"run-20260101T000002Z-a1b2c3",working_dir:$dir,attached_ts:"2026-01-01T00:00:02Z"}')" + + if ! PM_DISPATCH_STATE_ROOT="$store" _SW_REPO_ROOT="$repo" \ + operation_create "$operation_id" "$operation_json"; then + fail "$name" "operation_create failed" + return + fi + proj_dir="$(PM_DISPATCH_STATE_ROOT="$store" _SW_REPO_ROOT="$repo" _sw_project_dir)" + operations_dir="$proj_dir/operations" + PM_DISPATCH_STATE_ROOT="$store" _SW_REPO_ROOT="$repo" \ + operation_create "$operation_id" "$collision_json" >/dev/null 2>&1 || create_rc=$? + if [[ "$create_rc" -ne 3 || + "$(jq -r '.executor' "$operations_dir/$operation_id.json")" != "codex" ]]; then + fail "$name" "duplicate create rc=$create_rc or replaced the reserved record" + return + fi + if ! PM_DISPATCH_STATE_ROOT="$store" _SW_REPO_ROOT="$repo" \ + operation_upsert "$operation_id" "$updated_json"; then + fail "$name" "operation_upsert failed" + return + fi + if ! PM_DISPATCH_STATE_ROOT="$store" _SW_REPO_ROOT="$repo" \ + operation_child_append "$operation_id" "$child_json"; then + fail "$name" "operation_child_append failed" + return + fi + if ! PM_DISPATCH_STATE_ROOT="$store" _SW_REPO_ROOT="$repo" FAKE_FLOCK_MISSING=1 \ + operation_child_append "$operation_id" "$fallback_child_json"; then + fail "$name" "operation_child_append mkdir-lock fallback failed" + return + fi + contract="$(layout_operation_contract)" || { + fail "$name" "could not read operations/ contract" + return + } + IFS=$'\t' read -r parent_pattern create_strategy update_strategy child_pattern \ + child_format child_lock_base_pattern child_strategy <<< "$contract" + expected_parent="${parent_pattern//\$TS_HEX/$ts_hex}" + expected_child="${child_pattern//\$OPERATION_ID/$operation_id}" + expected_child_lock_base="${child_lock_base_pattern//\$OPERATION_ID/$operation_id}" + writer_create_strategy="$(writer_operation_create_strategy)" + writer_update_strategy="$(writer_operation_update_strategy)" + writer_child_lock_base="$(writer_operation_child_lock_base_pattern)" + writer_child_lock_base="${writer_child_lock_base//\$OPERATION_ID/$operation_id}" + + if [[ -f "$operations_dir/$expected_parent" && + -f "$operations_dir/$expected_child" && + "$(jq -r '.state' "$operations_dir/$expected_parent")" == "completed" && + "$create_strategy" == "$writer_create_strategy" && + "$update_strategy" == "$writer_update_strategy" && + "$expected_child_lock_base" == "$writer_child_lock_base" && + "$child_format" == "jsonl" && + "$child_strategy" == "append-locked" && + ! -e "$operations_dir/$expected_child_lock_base.lockdir" ]] && + jq -se 'length == 2' "$operations_dir/$expected_child" >/dev/null 2>&1; then + pass "$name" + else + fail "$name" \ + "parent=$expected_parent create=$create_strategy/$writer_create_strategy update=$update_strategy/$writer_update_strategy child=$expected_child format=$child_format lock_base=$expected_child_lock_base/$writer_child_lock_base strategy=$child_strategy" + fi +} + case_sw_build_run_json_emits_current_schema_version() { local name="sw_build_run_json emits schema_version matching run.schema.json" should_run "$name" || return 0 @@ -230,6 +410,8 @@ case_layout_is_valid_yaml case_project_subdirs_match_layout case_active_files_and_locks_match_layout case_archive_patterns_match_layout +case_writer_entry_points_match_layout +case_operation_paths_match_layout case_sw_build_run_json_emits_current_schema_version th_summary