diff --git a/include/naab/governance.h b/include/naab/governance.h index d372cec2..53937095 100644 --- a/include/naab/governance.h +++ b/include/naab/governance.h @@ -2643,7 +2643,7 @@ class GovernanceEngine { // --- Mid-run reload (Governance Under Survivability) --- bool reloadIfChanged(); // check mtime, reload if tightened, return true if reloaded std::vector getAndClearNotices(); // retrieve + clear pending reload notices - int getReloadCount() const { return reload_count_; } + int getReloadCount() const { return reload_count_.load(std::memory_order_relaxed); } // --- State --- bool isActive() const { return active_; } @@ -3475,7 +3475,10 @@ class GovernanceEngine { // --- Mid-run reload state (Governance Under Survivability) --- int64_t loaded_mtime_ns_ = 0; // govern.json mtime as nanoseconds (fs clock epoch) int64_t last_sig_fail_mtime_ = 0; // mtime of last signature-failed reload (suppress duplicate logs) - int reload_count_ = 0; // reloads applied this run + // Atomic because agent worker threads read it via getReloadCount() while + // reloadIfChanged() writes it under reload_mutex_ — a different lock, so the + // mutex does not order the read. Mirrors governance_epoch_ above. + std::atomic reload_count_{0}; // reloads applied this run std::vector pending_notices_; // notices from last reload mutable std::mutex notices_mutex_; // guards pending_notices_ mutable std::mutex reload_mutex_; // serializes reload attempts diff --git a/src/runtime/governance_config.cpp b/src/runtime/governance_config.cpp index bd6fb80d..2d86fcbb 100644 --- a/src/runtime/governance_config.cpp +++ b/src/runtime/governance_config.cpp @@ -4005,7 +4005,7 @@ bool GovernanceEngine::reloadIfChanged() { } writeAgentTelemetry("CONFIG_ADJUSTMENT", { {"accepted", "true"}, - {"reload_count", std::to_string(reload_count_)}, + {"reload_count", std::to_string(reload_count_.load(std::memory_order_relaxed))}, {"governance_epoch", std::to_string(governance_epoch_.load())}, {"update_reason", update_reason}, {"changed_agents", joinStrings(changed_agents)}, diff --git a/src/runtime/governance_engine.cpp b/src/runtime/governance_engine.cpp index e95c27c0..828fda8f 100644 --- a/src/runtime/governance_engine.cpp +++ b/src/runtime/governance_engine.cpp @@ -2845,8 +2845,9 @@ void GovernanceEngine::printDashboard() const { if (!rules().sandbox_level_config.empty() && rules().sandbox_level_config != "unrestricted") config_line += " | Sandbox: " + rules().sandbox_level_config; fprintf(stderr, "%s\n", config_line.c_str()); - if (reload_count_ > 0) - fprintf(stderr, "Reloads: %d mid-run reload(s) applied\n", reload_count_); + int reload_count_snapshot = reload_count_.load(std::memory_order_relaxed); + if (reload_count_snapshot > 0) + fprintf(stderr, "Reloads: %d mid-run reload(s) applied\n", reload_count_snapshot); if (warned > 0) fprintf(stderr, "Checks: %d passed, %d blocked, %d advisory\n", passed, blocked, warned); else diff --git a/src/stdlib/agent_impl.cpp b/src/stdlib/agent_impl.cpp index afb2c1ae..63c64347 100644 --- a/src/stdlib/agent_impl.cpp +++ b/src/stdlib/agent_impl.cpp @@ -118,6 +118,16 @@ struct PendingProposal { bool scan_ok = true; // response scan + contract passed during propose double score = 1.0; // read-only admissibility score at propose time bool admissible = true; + // Accepted-config generation this candidate was produced under. commit() + // deliberately does NOT reloadIfChanged() — an accepted reload re-baselines + // mandate_keywords and would score an already-generated candidate against a + // mandate it never received, besides dangling `config` through the atomic + // rules_ptr_ swap. Refusing across a reload is the correct substitute: it + // declines to commit under conditions the candidate was not evaluated under, + // rather than re-scoring it under them. reload_count_ and not + // governance_epoch_ — the epoch also moves on pulse verdict transitions, and + // the pulse sawtooths, so proposals would be voided during normal operation. + int reload_count = 0; }; static std::unordered_map> s_pending_proposals; @@ -4777,22 +4787,50 @@ static NaabVal agentPropose(std::vector& args) { // weakening. CRITICAL still denies outright regardless of lease. if (gov_engine && gov_engine->isActive()) { const auto& cb = gov_engine->getRules().circuit_breaker; + const bool lease_configured = (config->standing_lease_turns > 0 || + config->standing_lease_seconds > 0); + bool lease_expired = false; + if (lease_configured) { + std::lock_guard lock(s_agent_mutex); + auto it = s_trackers.find(handle_id); + if (it != s_trackers.end()) { + lease_expired = leaseExpiredLocked(it->second, config); + } + } + + // The LEASE test is unconditional. It used to sit inside the + // step_up_enabled guard, so with step-up off (the default — + // governance.h step_up_enabled = false) propose consulted the lease not + // at all, while agent.send() hard-throws on an expired lease and + // agent.commit() checks it unconditionally. Propose was the only entry + // point where expired authority passed silently. + if (lease_expired) { + if (cb.step_up_enabled) { + throw std::runtime_error( + "Agent error: agent.propose denied — step-up challenge required\n\n" + " Help:\n" + " - Governance level or lease state requires re-authorization\n" + " - Call agent.send() first so the step-up challenge can run\n"); + } + // No re-auth path exists with step-up disabled, so point at the + // remedy that does — same wording as send() and commit(). + throw std::runtime_error( + "Agent error: Standing lease expired before propose\n\n" + " Help:\n" + " - Create a new agent handle for a fresh conversation\n" + " - Or enable step_up challenges to allow lease renewal\n"); + } + + // The LEVEL test stays inside the guard: required_level is derived from + // step_up_at_level, so it is intrinsically about step-up and means + // nothing when step-up is off. CRITICAL denies a leased agent outright; + // an agent with no lease has no authorization state to consult, so it + // keeps the level test rather than being loosened to nothing. if (cb.step_up_enabled) { int required_level = (cb.step_up_at_level == "high") ? 2 : 1; int level = static_cast(gov_engine->getGovernanceLevel()); - const bool lease_configured = (config->standing_lease_turns > 0 || - config->standing_lease_seconds > 0); - bool lease_expired = false; - { - std::lock_guard lock(s_agent_mutex); - auto it = s_trackers.find(handle_id); - if (it != s_trackers.end()) { - lease_expired = leaseExpiredLocked(it->second, config); - } - } const bool deny = lease_configured - ? (lease_expired || - level >= static_cast(governance::GovernanceLevel::CRITICAL)) + ? (level >= static_cast(governance::GovernanceLevel::CRITICAL)) : (level >= required_level); if (deny) { throw std::runtime_error( @@ -5008,6 +5046,10 @@ static NaabVal agentPropose(std::vector& args) { pp.scan_ok = scan_ok; pp.score = score; pp.admissible = admissible; + // Stamp the config generation this candidate was evaluated under. + // propose() calls reloadIfChanged() above, so this is the post-reload + // value — the generation the candidate actually saw. + pp.reload_count = gov_engine ? gov_engine->getReloadCount() : 0; pending.push_back(std::move(pp)); cand["success"] = NaabVal::makeBool(true); @@ -5180,6 +5222,24 @@ static NaabVal agentCommit(std::vector& args) { auto* gov_engine = governance::GovernanceEngine::getCurrent(); + // Governance configuration changed between propose and commit. The candidate + // was generated and scored under the previous configuration, so committing it + // would advance conversation state under conditions it was never evaluated + // against. Refuse rather than re-score — re-scoring is what calling + // reloadIfChanged() here would amount to, and it would judge the candidate + // against a mandate it never received. Checked alongside the lease below: + // both are conditions whose truth changes with the passage of time, which is + // why they belong on the commit half rather than the proposal half. + if (gov_engine && gov_engine->isActive() && + gov_engine->getReloadCount() != selected.reload_count) { + throw std::runtime_error( + "Agent error: Governance configuration changed after this proposal\n\n" + " Help:\n" + " - The candidate was generated under the previous configuration\n" + " - Call agent.propose() again so candidates are evaluated" + " under the current one\n"); + } + if (lease_expired) { const auto& cb_cfg = (gov_engine && gov_engine->isActive()) ? gov_engine->getRules().circuit_breaker diff --git a/tests/governance_v4/test_propose_commit.sh b/tests/governance_v4/test_propose_commit.sh index 703b4e51..a90b477d 100755 --- a/tests/governance_v4/test_propose_commit.sh +++ b/tests/governance_v4/test_propose_commit.sh @@ -519,6 +519,174 @@ else fail "G-03" "Wrong refusal wording with step-up disabled" "got: $R_G3" fi +# ============================================================ +# Group H: a proposal cannot be committed across a config reload +# +# commit() deliberately does NOT call reloadIfChanged() — an accepted reload +# re-baselines mandate_keywords via onAgentConfigChanged() and would score an +# already-generated candidate against a mandate it never received, besides +# dangling `config` through the atomic rules_ptr_ swap. Refusing across a reload +# is the substitute: decline to commit under conditions the candidate was never +# evaluated against, rather than re-scoring it under them. +# +# Two mechanics this test depends on, both verified the hard way: +# - reloadIfChanged() compares mtime at SECOND granularity, so the rewrite +# must land in a later second than the initial load or no reload occurs. +# - subprocess env is scrubbed, so the re-sign must be passed --signing-key +# explicitly; NAAB_SIGNING_KEY does not survive into the child. +# ============================================================ +echo "" +echo -e "${CYAN}--- Group H: proposal invalidated by a config reload ---${NC}" + +write_govern_reload() { # $1=workdir $2=port + cat > "$1/govern.json" << GOVEOF +{ + "mode": "enforce", + "security": { "sandbox_level": "elevated" }, + "languages": { "allowed": ["python"] }, + "telemetry": { "enabled": true, "output_file": "telemetry.jsonl" }, + "circuit_breaker": { "enabled": true, "step_up_enabled": true }, + "agents": { + "proposer": { + "provider": "gemini", + "model": "stub-model", + "api_base": "http://127.0.0.1:$2", + "api_key_env": "FAKE_KEY_PROPOSE_TEST", + "max_tokens": 100, + "max_turns": 30, + "propose_candidates_max": 2 + } + } +} +GOVEOF + sign_govern "$1" +} + +run_reload_case() { # $1=name $2=do_reload(0|1) + local d="$TEST_TMP/reload-$1"; mkdir -p "$d" + cp "$WDIR/fixture.json" "$d/fixture.json" + start_stub "$d/fixture.json" "$d" >/dev/null 2>&1 || { echo "STUB_FAIL"; return; } + write_govern_reload "$d" "$STUB_PORT" + cat > "$d/t.naab" << 'EOF' +use agent +use file +use json +use process +main { + let h = agent.create("proposer") + let p = agent.propose(h, "summarize quarterly revenue", 1) + if env.get("DO_RELOAD", "") == "1" { + // mtime is second-granular; without this the rewrite is invisible. + time.sleep(2) + let raw = process.run("cat", ["govern.json"]) + let cfg = json.parse(raw.stdout) + cfg["agents"]["proposer"]["max_tokens"] = 80 // tightening: ratchet-safe + file.write("_pending.json", json.stringify(cfg, 2)) + let mv = process.run("mv", ["_pending.json", "govern.json"]) + // Subprocess env is scrubbed — pass the key explicitly. + let sg = process.run(env.get("NAAB_BIN", "naab-lang"), + ["--signing-key", env.get("NAAB_KEY", ""), "--sign-governance"]) + // A polyglot block calls reloadIfChanged(), bumping reload_count_. + let r = <> + } + try { + let c = agent.commit(h, (p.get("candidates") ?? [])[0]) + print("COMMIT_OK|turns=" + string(agent.usage(h).get("turns") ?? 0)) + } catch (e) { + print("COMMIT_DENIED|config_changed=" + string(string(e).contains("configuration changed"))) + } +} +EOF + local out + out=$( (cd "$d" && DO_RELOAD="$2" NAAB_BIN="$NAAB" NAAB_KEY="$NAAB_SIGNING_KEY" \ + timeout 90s "$NAAB" t.naab 2>/dev/null) \ + | grep -oE 'COMMIT_OK\|turns=[0-9]+|COMMIT_DENIED\|config_changed=(true|false)' | tail -1 ) + echo "${out:-}" + stop_stub +} + +# H-01: reload lands between propose and commit -> commit must refuse. +R_H1=$(run_reload_case changed 1) +if echo "$R_H1" | grep -q 'COMMIT_DENIED|config_changed=true'; then + pass "H-01" "Config reload between propose and commit invalidates the proposal" +else + fail "H-01" "Proposal committed across a config reload" "got: $R_H1" +fi + +# H-02: no reload -> commit must still succeed. Without this the stamp could be +# refusing every commit and H-01 would still pass. +R_H2=$(run_reload_case unchanged 0) +if echo "$R_H2" | grep -q 'COMMIT_OK|turns=1'; then + pass "H-02" "Commit still succeeds when the config did not change" +else + fail "H-02" "Commit refused with no config change — stamp is over-broad" "got: $R_H2" +fi + +# ============================================================ +# Group I: propose consults the lease even with step-up disabled +# +# The lease test used to sit inside `if (cb.step_up_enabled)`, and +# step_up_enabled defaults to FALSE — so by default propose did not consult the +# lease at all, while send() hard-throws and commit() (Group G) checks it +# unconditionally. Propose was the only entry point where expired authority +# passed silently. +# ============================================================ +echo "" +echo -e "${CYAN}--- Group I: propose lease gate independent of step-up ---${NC}" + +run_propose_lease_case() { # $1=name $2=lease_seconds $3=step_up + local d="$TEST_TMP/please-$1"; mkdir -p "$d" + cp "$WDIR/fixture.json" "$d/fixture.json" + start_stub "$d/fixture.json" "$d" >/dev/null 2>&1 || { echo "STUB_FAIL"; return; } + write_govern_commit_lease "$d" "$STUB_PORT" "$2" 0 "$3" + cat > "$d/t.naab" << 'EOF' +use agent +main { + let h = agent.create("proposer") + time.sleep(4) + try { + let p = agent.propose(h, "summarize quarterly revenue", 1) + print("PROPOSE_OK|candidates=" + string((p.get("candidates") ?? []).length())) + } catch (e) { + print("PROPOSE_DENIED|stepup=" + string(string(e).contains("step-up"))) + } +} +EOF + local out + out=$( (cd "$d" && timeout 90s "$NAAB" t.naab 2>/dev/null) \ + | grep -oE 'PROPOSE_OK\|candidates=[0-9]+|PROPOSE_DENIED\|stepup=(true|false)' | tail -1 ) + echo "${out:-}" + stop_stub +} + +# I-01: the hole. Expired lease + step-up disabled must now deny, with the hard +# wording since there is no challenge to point the caller at. +R_I1=$(run_propose_lease_case nostepup 2 false) +if echo "$R_I1" | grep -q 'PROPOSE_DENIED|stepup=false'; then + pass "I-01" "Expired lease denies propose with step-up disabled" +else + fail "I-01" "Expired lease passed propose with step-up disabled" "got: $R_I1" +fi + +# I-02: no lease configured + step-up disabled -> propose must still work. This +# is the shape two existing test configs use, and guards the no-op claim. +R_I2=$(run_propose_lease_case nolease 0 false) +if echo "$R_I2" | grep -q 'PROPOSE_OK'; then + pass "I-02" "No lease + step-up disabled still permits propose (not over-broad)" +else + fail "I-02" "Unleased config denied — hoisted check is over-broad" "got: $R_I2" +fi + +# I-03: anti-regression on Group F — with step-up on the renewable wording stays. +R_I3=$(run_propose_lease_case stepup 2 true) +if echo "$R_I3" | grep -q 'PROPOSE_DENIED|stepup=true'; then + pass "I-03" "Expired lease keeps the step-up wording when step-up is enabled" +else + fail "I-03" "Refusal wording changed with step-up enabled" "got: $R_I3" +fi + # ============================================================ echo "" echo -e "${CYAN}==============================================${NC}"