Skip to content

Commit 41cb477

Browse files
nickandersonclaude
nickanderson
and
claude
committed
CFE-4681: Complete timer_policy support for classes: promises
- Flip default timer_policy to "absolute" (PRESERVE) for backward compatibility: classes: promises historically skip re-evaluation when the class is already defined, so the timer was never reset. - Bypass the ExpandDeRefPromise skip when timer_policy is explicitly set to "reset" with persistence > 0, allowing the promise to reach VerifyClassPromise so the DB timer can be updated. - Add second-layer bypass in VerifyClassPromise for the case where EvalClassExpression returns false (class already in context) but timer_policy is "reset" — write the DB entry to reset the timer. - Clean up acceptance tests: use imported bodies (in_shell, always), regline() instead of returnszero+grep, $(G.testdir), files promises with delete => tidy, and add test metadata. - Add timer_policy_reset acceptance test verifying the timer resets across agent runs. Ticket: CFE-4681 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 981c175 commit 41cb477

11 files changed

Lines changed: 291 additions & 10 deletions

‎libpromises/attributes.c‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1132,6 +1132,21 @@ ContextConstraint GetContextConstraints(const EvalContext *ctx, const Promise *p
11321132
a.expression = NULL;
11331133
a.persistent = PromiseGetConstraintAsInt(ctx, "persistence", pp);
11341134

1135+
{
1136+
const char *tp = PromiseGetConstraintAsRval(pp, "timer_policy", RVAL_TYPE_SCALAR);
1137+
if (tp != NULL && strcmp(tp, "reset") == 0)
1138+
{
1139+
a.timer = CONTEXT_STATE_POLICY_RESET;
1140+
}
1141+
else
1142+
{
1143+
/* Default to PRESERVE (absolute) for backward compatibility:
1144+
* classes: promises historically skip re-evaluation when the
1145+
* class is already defined, so the timer was never reset. */
1146+
a.timer = CONTEXT_STATE_POLICY_PRESERVE;
1147+
}
1148+
}
1149+
11351150
{
11361151
const char *context_scope = PromiseGetConstraintAsRval(pp, "scope", RVAL_TYPE_SCALAR);
11371152
a.scope = ContextScopeFromString(context_scope);
@@ -1143,7 +1158,9 @@ ContextConstraint GetContextConstraints(const EvalContext *ctx, const Promise *p
11431158

11441159
for (int k = 0; CF_CLASSBODY[k].lval != NULL; k++)
11451160
{
1146-
if (strcmp(cp->lval, "persistence") == 0 || strcmp(cp->lval, "scope") == 0)
1161+
if (strcmp(cp->lval, "persistence") == 0 ||
1162+
strcmp(cp->lval, "scope") == 0 ||
1163+
strcmp(cp->lval, "timer_policy") == 0)
11471164
{
11481165
continue;
11491166
}

‎libpromises/cf3.defs.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,6 +1210,7 @@ typedef struct
12101210
ContextScope scope;
12111211
int nconstraints;
12121212
int persistent;
1213+
PersistentClassPolicy timer;
12131214
} ContextConstraint;
12141215

12151216
/*************************************************************************/

‎libpromises/eval_context.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -735,7 +735,7 @@ void EvalContextHeapPersistentSave(EvalContext *ctx, const char *name, unsigned
735735

736736
// first see if we have an existing record, and if we should bother to update
737737
{
738-
int existing_info_size = ValueSizeDB(dbp, key, strlen(key));
738+
int existing_info_size = ValueSizeDB(dbp, key, strlen(key) + 1);
739739
if (existing_info_size > 0)
740740
{
741741
PersistentClassInfo *existing_info = xcalloc(existing_info_size, 1);

‎libpromises/mod_common.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,7 @@ const ConstraintSyntax CF_CLASSBODY[] =
226226
ConstraintSyntaxNewContext("not", "Evaluate the negation of string expression in normal form", SYNTAX_STATUS_NORMAL),
227227
ConstraintSyntaxNewContextList("select_class", "Select one of the named list of classes to define based on host identity. Default value: random_selection", SYNTAX_STATUS_NORMAL),
228228
ConstraintSyntaxNewContextList("xor", "Combine class sources with XOR", SYNTAX_STATUS_NORMAL),
229+
ConstraintSyntaxNewOption("timer_policy", "absolute,reset", "Whether a persistent class restarts its counter when rediscovered. Default value: absolute", SYNTAX_STATUS_NORMAL),
229230
ConstraintSyntaxNewNull()
230231
};
231232

‎libpromises/promises.c‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -705,16 +705,31 @@ Promise *ExpandDeRefPromise(EvalContext *ctx, const Promise *pp, bool *excluded)
705705
pcopy->org_pp = pp->org_pp;
706706

707707
// if this is a class promise, check if it is already set, if so, skip
708+
// Exception: persistent classes with timer_policy => "reset" must not
709+
// be skipped — the promise needs to fire so the timer gets reset.
708710
if (strcmp("classes", PromiseGetPromiseType(pp)) == 0)
709711
{
710712
if (IsDefinedClass(ctx, CanonifyName(pcopy->promiser)))
711713
{
712-
Log(LOG_LEVEL_DEBUG,
713-
"Skipping evaluation of classes promise as class '%s' is already set",
714-
CanonifyName(pcopy->promiser));
714+
const char *tp = PromiseGetConstraintAsRval(pp, "timer_policy", RVAL_TYPE_SCALAR);
715+
int persistence = PromiseGetConstraintAsInt(ctx, "persistence", pp);
715716

716-
*excluded = true;
717-
return pcopy;
717+
if (tp != NULL && strcmp(tp, "reset") == 0 && persistence > 0)
718+
{
719+
Log(LOG_LEVEL_DEBUG,
720+
"Class '%s' is already set but timer_policy is reset"
721+
" — allowing promise evaluation to reset persistence timer",
722+
CanonifyName(pcopy->promiser));
723+
}
724+
else
725+
{
726+
Log(LOG_LEVEL_DEBUG,
727+
"Skipping evaluation of classes promise as class '%s' is already set",
728+
CanonifyName(pcopy->promiser));
729+
730+
*excluded = true;
731+
return pcopy;
732+
}
718733
}
719734
}
720735

‎libpromises/verify_classes.c‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,37 @@ PromiseResult VerifyClassPromise(EvalContext *ctx, const Promise *pp, ARG_UNUSED
7373
return PROMISE_RESULT_FAIL;
7474
}
7575

76-
if (a.context.expression == NULL ||
77-
EvalClassExpression(ctx, a.context.expression, pp))
76+
bool class_expression_true =
77+
(a.context.expression == NULL ||
78+
EvalClassExpression(ctx, a.context.expression, pp));
79+
80+
/* When a persistent class is already defined (loaded from DB),
81+
* EvalClassExpression short-circuits and returns false. If the
82+
* timer_policy is "reset", we still need to reset the persistence
83+
* timer in the DB even though the class is already in the context. */
84+
if (!class_expression_true &&
85+
a.context.persistent > 0 &&
86+
a.context.timer == CONTEXT_STATE_POLICY_RESET &&
87+
IsDefinedClass(ctx, pp->promiser))
88+
{
89+
StringSet *tags = StringSetNew();
90+
StringSetAdd(tags, xstrdup("source=promise"));
91+
for (const Rlist *rp = PromiseGetConstraintAsList(ctx, "meta", pp); rp; rp = rp->next)
92+
{
93+
StringSetAdd(tags, xstrdup(RlistScalarValue(rp)));
94+
}
95+
Log(LOG_LEVEL_VERBOSE,
96+
"C: + Resetting persistent class timer: '%s' (%d minutes)",
97+
pp->promiser, a.context.persistent);
98+
Buffer *buf = StringSetToBuffer(tags, ',');
99+
EvalContextHeapPersistentSave(ctx, pp->promiser, a.context.persistent,
100+
CONTEXT_STATE_POLICY_RESET, BufferData(buf));
101+
BufferDestroy(buf);
102+
StringSetDestroy(tags);
103+
return PROMISE_RESULT_NOOP;
104+
}
105+
106+
if (class_expression_true)
78107
{
79108
if (a.context.expression == NULL)
80109
{
@@ -131,7 +160,7 @@ PromiseResult VerifyClassPromise(EvalContext *ctx, const Promise *pp, ARG_UNUSED
131160
pp->promiser, a.context.persistent);
132161
Buffer *buf = StringSetToBuffer(tags, ',');
133162
EvalContextHeapPersistentSave(ctx, pp->promiser, a.context.persistent,
134-
CONTEXT_STATE_POLICY_RESET, BufferData(buf));
163+
a.context.timer, BufferData(buf));
135164
BufferDestroy(buf);
136165
}
137166
if (inserted && (comment != NULL))

‎tests/acceptance/02_classes/01_basic/persistent_timer_policy.cf‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
#######################################################
2+
#
3+
# CFE-4681: classes: promises should support timer_policy
4+
#
5+
# Verify that timer_policy => "absolute" on a classes: promise
6+
# is parsed correctly and passed to EvalContextHeapPersistentSave
7+
# as CONTEXT_STATE_POLICY_PRESERVE. The verbose log message
8+
# should contain "policy preserve".
9+
#
10+
#######################################################
11+
12+
body common control
13+
{
14+
inputs => { "../../default.sub.cf" };
15+
bundlesequence => { default("$(this.promise_filename)") };
16+
version => "1.0";
17+
}
18+
19+
bundle agent init
20+
{
21+
# Remove the persistent class DB to ensure a clean state.
22+
files:
23+
"$(sys.workdir)/state/cf_state.lmdb"
24+
delete => tidy;
25+
"$(sys.workdir)/state/cf_state.lmdb-lock"
26+
delete => tidy;
27+
"$(sys.workdir)/state/cf_state.lmdb.lock"
28+
delete => tidy;
29+
}
30+
31+
bundle agent test
32+
{
33+
meta:
34+
"description" -> { "CFE-4681" }
35+
string => "timer_policy => absolute on classes: promises stores CONTEXT_STATE_POLICY_PRESERVE";
36+
37+
commands:
38+
# Run sub-policy that defines a persistent class with
39+
# timer_policy => "absolute"
40+
"$(sys.cf_agent) -Kv -f $(this.promise_filename).sub > $(G.testdir)/timer_policy_run1.log 2>&1"
41+
contain => in_shell,
42+
classes => always("done");
43+
}
44+
45+
bundle agent check
46+
{
47+
classes:
48+
done::
49+
# Verify the log contains "policy preserve" (not "policy reset")
50+
"ok" expression => regline(".*Creating persistent class.*timer_policy_test_class.*policy preserve.*",
51+
"$(G.testdir)/timer_policy_run1.log");
52+
53+
reports:
54+
DEBUG.done.!ok::
55+
"FAIL: log did not contain 'policy preserve' for timer_policy_test_class";
56+
!done::
57+
"$(this.promise_filename) FAIL (sub-agent run did not complete)";
58+
done.ok::
59+
"$(this.promise_filename) Pass";
60+
done.!ok::
61+
"$(this.promise_filename) FAIL";
62+
}

‎tests/acceptance/02_classes/01_basic/persistent_timer_policy.cf.sub‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
body common control
2+
{
3+
bundlesequence => { run };
4+
}
5+
6+
bundle agent run
7+
{
8+
classes:
9+
# Define persistent class with timer_policy => "absolute"
10+
# This stores CONTEXT_STATE_POLICY_PRESERVE in the DB
11+
"timer_policy_test_class"
12+
expression => "any",
13+
persistence => "120",
14+
timer_policy => "absolute";
15+
}

‎tests/acceptance/02_classes/01_basic/persistent_timer_policy_reset.cf‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
#######################################################
2+
#
3+
# CFE-4681: classes: promises with timer_policy => "reset"
4+
#
5+
# Verify that timer_policy => "reset" on a classes: promise
6+
# causes the persistence timer to be reset on subsequent
7+
# agent runs, even though the class is already defined
8+
# (loaded from the persistent DB).
9+
#
10+
# First run: expect "Creating persistent class"
11+
# Second run: expect "Resetting persistent class" (not skipped)
12+
#
13+
#######################################################
14+
15+
body common control
16+
{
17+
inputs => { "../../default.sub.cf" };
18+
bundlesequence => { default("$(this.promise_filename)") };
19+
version => "1.0";
20+
}
21+
22+
bundle agent init
23+
{
24+
# Remove the persistent class DB to ensure a clean state.
25+
files:
26+
"$(sys.workdir)/state/cf_state.lmdb"
27+
delete => tidy;
28+
"$(sys.workdir)/state/cf_state.lmdb-lock"
29+
delete => tidy;
30+
"$(sys.workdir)/state/cf_state.lmdb.lock"
31+
delete => tidy;
32+
}
33+
34+
bundle agent test
35+
{
36+
meta:
37+
"description" -> { "CFE-4681" }
38+
string => "timer_policy => reset on classes: promises resets the persistence timer on subsequent runs";
39+
40+
commands:
41+
# First run: define the persistent class
42+
"$(sys.cf_agent) -Kv -f $(this.promise_filename).sub > $(G.testdir)/timer_reset_run1.log 2>&1"
43+
contain => in_shell,
44+
classes => always("first_done");
45+
}
46+
47+
bundle agent check
48+
{
49+
commands:
50+
first_done::
51+
# Second run: class already exists in DB, timer_policy=reset
52+
# should cause the timer to be reset
53+
"$(sys.cf_agent) -Kv -f $(this.promise_filename).sub > $(G.testdir)/timer_reset_run2.log 2>&1"
54+
contain => in_shell,
55+
classes => always("second_done");
56+
57+
classes:
58+
second_done::
59+
"first_ok" expression => regline(".*Creating persistent class.*timer_reset_test_class.*",
60+
"$(G.testdir)/timer_reset_run1.log");
61+
"second_ok" expression => regline(".*Resetting persistent class.*timer_reset_test_class.*",
62+
"$(G.testdir)/timer_reset_run2.log");
63+
"ok" and => { "first_ok", "second_ok" };
64+
65+
reports:
66+
DEBUG.second_done.!first_ok::
67+
"FAIL: first run did not log 'Creating persistent class'";
68+
DEBUG.second_done.!second_ok::
69+
"FAIL: second run did not log 'Resetting persistent class' (short-circuit not bypassed)";
70+
!first_done::
71+
"$(this.promise_filename) FAIL (first sub-agent run did not complete)";
72+
first_done.!second_done::
73+
"$(this.promise_filename) FAIL (second sub-agent run did not complete)";
74+
second_done.ok::
75+
"$(this.promise_filename) Pass";
76+
second_done.!ok::
77+
"$(this.promise_filename) FAIL";
78+
}
79+

‎tests/acceptance/02_classes/01_basic/persistent_timer_policy_reset.cf.sub‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
body common control
2+
{
3+
bundlesequence => { run };
4+
}
5+
6+
bundle agent run
7+
{
8+
classes:
9+
# Define persistent class with timer_policy => "reset"
10+
# On second run, the timer should be reset even though
11+
# the class is already defined from the persistent DB.
12+
"timer_reset_test_class"
13+
expression => "any",
14+
persistence => "120",
15+
timer_policy => "reset";
16+
}

0 commit comments

Comments
 (0)