fix(lib): a malformed select in a resreq fails the parse instead of being dropped (bug-1887)

The primary_resreq action ignored parse_resreq_primary()'s result, so a
primary whose select (or rusage) clause failed to parse was accepted
without that clause: select[type==] or select[mem>] matched every host.
A malformed clause now fails the whole resreq, and parse_resreq() returns
the clause's own message.

A select naming an unregistered resource is still dropped, not an
error: parse_select() reports it as -ENOENT (new select_context_t
.unknown_resource) and the action lets it pass, because jobs rely on it
-- the scheduler's own "type == any" default among them when no "type"
resource is registered (making it fatal failed 36 scheduler suites).

Test: test_resreq_primary_select_error_fails_parse RED before (all
accepted), GREEN after.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

src/include/resreq.h

@@ -191,6 +191,7 @@ struct select_context_t_ {
     int                err_pos;
     int                has_error;
     char               errmsg[256]; /* fixed-size — no alloc/free in grammar actions */
+    int                unknown_resource; /* bug-1887: the error was an unknown resource name */
     int                depth;       /* current nesting depth — guard vs SELECT_MAX_DEPTH */
     xce_select_node_t* root;        /* result root set by grammar top-level action */
 };

src/lib/resreq.c

@@ -325,7 +325,9 @@ static int parse_select(xce_primary_resreq_t* primary, char* selectString,
             *errmsg = strdup(ctx.errmsg);
         else
             get_parse_errmsg(errmsg, selectString, ctx.err_pos);
-        rc = -EINVAL;
+        /* bug-1887: -ENOENT tells a select naming an unregistered resource
+         * apart from a malformed one (-EINVAL) */
+        rc = ctx.unknown_resource ? -ENOENT : -EINVAL;
         goto cleanup_parser;
     }
 
@@ -535,7 +537,14 @@ int parse_resreq(xce_resreq_t* resreq, char* data, var resmgr, char** errmsg)
     xce_resreq_parse(p, NULL);
 
     if (ctx.has_error) {
-        get_parse_errmsg(errmsg, ctx.input, ctx.err_pos);
+        /* bug-1887: a primary's own message (select/rusage) names the
+         * failing clause; otherwise report the grammar error position */
+        if (ctx.errmsg != NULL) {
+            *errmsg    = ctx.errmsg;
+            ctx.errmsg = NULL;
+        } else {
+            get_parse_errmsg(errmsg, ctx.input, ctx.err_pos);
+        }
         rc = -EINVAL;
         /* FIXME: need to freeup partial data in resreqs */
     }

src/lib/testsuite/test_resreq.c

@@ -6,6 +6,12 @@
 
 #include <unity/unity.h>
 
+#include <rtti.h>
+
+#include "kvmgr.h"
+#include "resobj.h"
+#include "xstring.h"
+
 #include "resreq.h"
 #include "utils.h"
 
@@ -519,6 +525,86 @@ void test_check_rusage_syntax(void)
     }
 }
 
+/* bug-1887: a primary whose select clause is malformed fails the whole
+ * resreq with that clause's message; it used to be accepted with no select
+ * at all, matching every host. */
+static void expect_resreq_error(var resmgr, char* str, const char* needle)
+{
+    xce_resreq_t resreq;
+    char*        errmsg = NULL;
+    int          rc;
+
+    memset(&resreq, 0, sizeof(resreq));
+    rc = parse_resreq(&resreq, str, resmgr, &errmsg);
+    TEST_ASSERT_TRUE_MESSAGE(rc < 0, str);
+    TEST_ASSERT_NOT_NULL_MESSAGE(errmsg, str);
+    TEST_ASSERT_NOT_NULL_MESSAGE(strstr(errmsg, needle), errmsg);
+    free(errmsg);
+    kill_resreq(&resreq);
+}
+
+/* a resource manager of the kind the scheduler parses against (an xkvmgr
+ * of resobjs); make_test_resmgr()'s plain kvtab does not resolve selects */
+static var make_select_resmgr(void)
+{
+    kvmgr_params_t params = { .key_type   = KVMGR_KEY_STRING,
+                              .nslots     = 64,
+                              .delta      = 8,
+                              .datakiller = (destroy_fn_t)dereference };
+    xce_resource_t tmpl;
+    var            mgr = makeInstance(&xkvmgr_type, &params);
+
+    TEST_ASSERT_NOT_NULL(mgr);
+    memset(&tmpl, 0, sizeof(tmpl));
+    tmpl.name    = (char*)"type";
+    tmpl.valtype = XCE_VAL_STRING;
+    TEST_ASSERT_NOT_NULL(makeResource(mgr, &tmpl));
+    tmpl.name    = (char*)"mem";
+    tmpl.valtype = XCE_VAL_NUMERIC;
+    TEST_ASSERT_NOT_NULL(makeResource(mgr, &tmpl));
+
+    return mgr;
+}
+
+void test_resreq_primary_select_error_fails_parse(void)
+{
+    var          resmgr;
+    xce_resreq_t resreq;
+    char*        errmsg = NULL;
+    int          rc;
+
+    TEST_ASSERT_TRUE(xstring_table_init(64) >= 0);
+    resreq_init(1); /* as the scheduler core does: select node table */
+    resmgr = make_select_resmgr();
+
+    expect_resreq_error(resmgr, "select[type==] rusage[mem=1]", "Error");
+    expect_resreq_error(resmgr, "select[mem>]", "Error");
+    expect_resreq_error(resmgr, "2*{select[mem>1 &&]}", "Error");
+
+    /* an unregistered resource name is still dropped with the select, not
+     * an error (jobs rely on it; see xce_resreq.peg) */
+    memset(&resreq, 0, sizeof(resreq));
+    rc = parse_resreq(&resreq, "select[nosuch>1]", resmgr, &errmsg);
+    TEST_ASSERT_TRUE(rc >= 0);
+    TEST_ASSERT_NULL(errmsg);
+    TEST_ASSERT_NULL(resreq.u.primary->select);
+    kill_resreq(&resreq);
+
+    /* a valid select still parses, and keeps its select */
+    memset(&resreq, 0, sizeof(resreq));
+    rc = parse_resreq(&resreq, "select[type==any] rusage[mem=1]", resmgr,
+                      &errmsg);
+    TEST_ASSERT_TRUE_MESSAGE(rc >= 0, (errmsg != NULL) ? errmsg : "(null)");
+    TEST_ASSERT_NULL(errmsg);
+    TEST_ASSERT_EQUAL_INT(XCE_RESREQ_PRIMARY, resreq.tag);
+    TEST_ASSERT_NOT_NULL(resreq.u.primary->select);
+    kill_resreq(&resreq);
+
+    destroy(resmgr);
+    resreq_fini();
+    xstring_table_fini();
+}
+
 void test_resreq(void)
 {
     printf("\n>> resreq test\n\n");
@@ -538,4 +624,5 @@ void test_resreq(void)
     RUN_TEST(test_parse_section_explicit_select_with_span);
     RUN_TEST(test_parse_section_explicit_select_rusage_span);
     RUN_TEST(test_parse_section_span_not_in_select);
+    RUN_TEST(test_resreq_primary_select_error_fails_parse);
 }

src/lib/xce_resreq.peg

@@ -2,6 +2,7 @@
 %value "char *"
 %auxil "resreq_context_t *"
 %source {
+    #include <errno.h>
     #include <stdio.h>
     #include <stdlib.h>
     #include <string.h>
@@ -108,7 +109,18 @@ primary_resreq <- <  (! ('{' / '}' ) .)* > {
       auxil->primary = primary;
       free(auxil->errmsg); /* bug-1884: an earlier primary's message */
       auxil->errmsg = NULL;
-      parse_resreq_primary(primary, (char *)$0, auxil->resmgr, &(auxil->errmsg));
+      /* bug-1887: a primary whose select/rusage is malformed fails the
+       * whole resreq (it was kept without that clause). A select naming
+       * an unregistered resource (-ENOENT) is still dropped, as before --
+       * jobs rely on it, the scheduler's own "type == any" default among
+       * them when no "type" resource is registered. */
+      {
+        int prc = parse_resreq_primary(primary, (char *)$0, auxil->resmgr,
+                                       &(auxil->errmsg));
+        if ((prc < 0) && (prc != -ENOENT)) {
+          auxil->has_error = 1;
+        }
+      }
     }
   }
 

src/lib/xce_select.peg

@@ -48,6 +48,7 @@
                 snprintf(auxil->errmsg, sizeof(auxil->errmsg),
                     "unknown resource '%s'", name);
             }
+            auxil->unknown_resource = 1;
             auxil->has_error = 1;
             return NULL;
         }

src/scheduler/testsuite/test_snapshot_save_load.c

@@ -1335,12 +1335,10 @@ test_snapshot_load_leaves_schedobj_null_when_resreq_fails_to_parse(void)
     /* An unbalanced/unterminated compound-resreq brace: the top-level PEG
      * grammar (xce_resreq.peg) requires a full input match through EOF; a
      * lone unterminated '{' cannot complete as alter/compound/primary and
-     * leaves trailing unconsumed input, so parse_resreq() genuinely fails
-     * (unlike plain non-brace garbage, which the grammar's permissive
-     * primary_resreq rule accepts as a literal select expression and only
-     * fails later, silently, inside parse_resreq_primary() without
-     * propagating an error -- confirmed empirically while writing this
-     * test). */
+     * leaves trailing unconsumed input, so parse_resreq() genuinely fails.
+     * (Plain non-brace garbage is taken as a select expression; before
+     * bug-1887 its failure inside parse_resreq_primary() was dropped
+     * silently, now a malformed select fails the parse too.) */
     bad_job = make_job_with_resreq(404, JOB_STAT_PEND, "normal", "alice",
                                    "proj1", "{");