feat(index): add opt-in discovery resource limits

[liuchong]

Related to #1347.

Problem

Indexing accepts whatever a repository contains. A tree carrying a vendored monorepo, a generated dump, or a runaway build directory is discovered in full, and the first sign of trouble is a host under memory pressure with nothing that attributes it to indexing. There is currently no way for an operator to say "not larger than this".

What this changes

Two opt-in limits, evaluated during discovery against accepted source files only, so ignored and excluded paths never count against an operator's budget:

Key	Meaning
index_max_files	Maximum number of accepted source files
index_max_source_mb	Maximum total size of accepted source files

Both default to off; without configuration this PR changes no behaviour.

Crossing a limit fails the whole attempt with a structured resource_limit_exceeded result naming the resource, the observed value and the limit. No partial graph is published, and an existing serving index keeps answering. A partial graph would be worse than no graph: it looks complete and is silently wrong.

Limits are read from the CLI-managed _config.db, not from MCP request arguments. A supervised parent replaces any caller-supplied internal policy before spawning its worker, and the worker rejects a missing or incomplete contract, so the CLI, the daemon and the supervised worker all enforce the same decision. A test pins the forged-override path.

Testing

make -f Makefile.cbm test and make -f Makefile.cbm lint-ci on macOS. New coverage: discovery counting and rejection, policy parsing and validation boundaries, worker-contract propagation and forged-override rejection, end-to-end preservation of the previously published index.

One pre-existing failure (tests/test_cli.c:6484, agent-client registry) is present on the unmodified base commit as well and is unrelated to this change.

Stack

This is the first of six PRs that replace #1348. Each is independently reviewable and mergeable in order; later ones build on earlier ones.

1. this PR — discovery limits
2. feat(index): add worker resource watchdogs #1724 — worker watchdogs
3. feat(index): enforce opt-in storage resource limits #1725 — storage limits
4. feat(index): add opt-in index resource profiles #1726 — resource profiles
5. feat(index): record and report the latest index attempt #1727 — attempt record and freshness
6. feat(mcp): warn on answers served from a failed rebuild #1728 — stale-index warning on answers

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[DeusData]

This fixes exactly what I blocked #1348 on, and I verified it rather than taking the claim. One small defect and I will merge.

Default-off is now true by construction, not by convention. I traced all four gates:

1. cbm_index_policy_init zeroes the struct, and cbm_index_limit_u64_t carries its own enabled flag — so zero means disabled. Critically, there is no ..._defaults() function anywhere in the diff; the shape that caused the problem is gone rather than corrected.
2. cbm_index_policy_default_value() returns "off" for every policy key.
3. cbm_config_load_index_policy passes that as cbm_config_get's default, and cbm_config_get returns the default when the row is absent — so an unconfigured install loads "off".
4. pipeline.c passes a policy pointer only when cbm_index_policy_enabled(), and fl_add guards the whole accounting block on a non-NULL policy plus each limit's own enabled.

discover_resource_policy_off_matches_legacy_discovery is the test I most wanted to see: it runs discovery twice, once with no policy and once with an initialised-but-off policy, and asserts identical counts, identical per-file rel_path and size, and violation.resource == CBM_INDEX_RESOURCE_NONE. That is the "nothing changes for people who did not ask" property stated as an assertion rather than a promise.

Two config keys instead of eighteen. index_max_files and index_max_source_mb, both defaulting to "off", both reachable through the existing config set/get/list/reset with no new subcommand — and reset works because "off" is declared in CONFIG_KEYS rather than implied. The CBM_INDEX_MAX_*_VALUE constants are ceilings on what an operator may configure, not defaults, which I checked because that is an easy thing to misread.

Three things you hardened beyond what I asked, which are worth naming:

• The caller-policy strip is now a while loop rather than a single remove_key, so a request carrying duplicate _cbm_index_policy keys cannot smuggle one past. That is a real attack shape against JSON objects and it was not in the original.
• The worker refuses anything short of the exact full key set, not merely a present object. A future key added to the policy cannot silently arrive unset in a worker.
• Discovery violations map to a distinct CBM_PIPELINE_RESOURCE_LIMIT at all four sites rather than collapsing into CBM_NOT_FOUND, so "we refused" is now distinguishable from "we failed".

And index_policy_mcp_rejects_forged_override_and_preserves_serving_index is the strongest test in the PR: it forges both keys to "off", proves server config still wins with observed: 2, limit: 1, and then proves the pre-existing database was untouched by comparing indexed_at byte-for-byte. Abort-not-truncate is pinned too, including the late-breach case via the before_final_manifest hook.

Worth noting: because every new test calls a symbol that does not exist on main, the suite would not even compile against the base. That is a compile-level guarantee against vacuous tests, which is stronger than the usual argument.

---

The one fix before merge — a silent failure.

config_index_policy_write prints only when validation fails:

if (!cbm_index_policy_set(&candidate, key, value, error, sizeof(error))) {
    (void)fprintf(stderr, "error: %s\n", error);
    return CLI_ERR;
}
return cbm_config_set(config, key, value);

but the caller now suppresses the generic message for policy keys:

if (!config_key_is_index_policy(argv[CLI_SKIP_ONE])) {
    (void)fprintf(stderr, "error: failed to set %s\n", argv[CLI_SKIP_ONE]);
}

So if validation passes and the cbm_config_set write then fails — a locked or corrupt _config.db — config set index_max_files 100 exits non-zero having printed nothing at all. The suppression assumes the helper always spoke, and it does not on that path. Either print in the write-failure branch of the helper, or narrow the suppression to the validation case.

Two things to confirm rather than change:

1. index_run_supervised_path returns NULL when policy load fails, so on a corrupt _config.db the auto-index attempt is dropped after logging index.policy. Fail-closed is the right default and consistent with the rest of the design — I just want it to be a decision for the unattended auto-index path specifically, where nobody is watching the log.
2. fl->limit_exceeded now carries two meanings — the pre-existing count_only file-cap early stop and a policy breach. No current caller passes a policy in count-only mode, so this is latent, but it is the kind of latent that bites the next person.

And one completeness check I would like your eye on, because it is the failure mode your own commit message warns about: the worker argv contract is now mandatory and fail-closed, so any harness that spawns --index-worker and hand-builds an index_repository payload must supply the full key set. You updated the two that do. I enumerated the other --index-worker sites — index_supervisor.c, daemon/bootstrap.c, and several test files — and they look like argv-plumbing that never constructs that payload, so I believe the change is complete. Please confirm, since a miss there produces exactly the quietly-broken guard you were guarding against.

Fix the CLI message and I will merge this. Thank you for taking the reshaping seriously instead of arguing for the original — this is a much better change than #1348 was, and the trust boundary is stronger than what I asked for.

[liuchong]

Fixed, and the three checks confirmed.

The silent write failure is fixed. config_index_policy_write now speaks in the write-failure branch, so the caller's suppression stays correct on every path the helper can return from:

int rc = cbm_config_set(config, key, value);
if (rc != 0) {
    (void)fprintf(stderr, "error: failed to set %s\n", key);
}
return rc;

index_policy_cli_set_reports_a_failed_write pins it. Making the write fail without corrupting anything needed a fixture rather than a mock: it creates _config.db with config as a view, so validation passes and the INSERT is what fails. The test asserts a non-zero exit with the key named on stderr. Amended into the original commit rather than stacked on top, so the slice stays one reviewable diff.

1. Fail-closed on the unattended path — deliberate, and specifically for that path. With no policy loadable there is no bound to enforce, and unattended is exactly where an unbounded index is least survivable. The cost is the one you named: nobody reads that log line. #1727 and #1728 are what close that gap — which is also why I understand judging them separately.

2. fl->limit_exceeded — confirmed, and mutually exclusive by accident rather than by construction. fl->max_files is -1 unless count_only, and the policy breach only fires when fl->resource_policy is non-NULL, so today the two producers cannot both run. Nothing enforces that, which is your point exactly. If you want it structural, the narrowest change is a second flag for the count-only stop:

    if (fl->max_files >= 0 && fl->count >= fl->max_files) {
        fl->capped = true;
        return;
    }

with file_list_should_stop returning fl->failed || fl->limit_exceeded || fl->capped. Behaviour is identical; the two meanings stop sharing a bit. Say the word and I will add it here or in a follow-up — I left it out because it changes a struct the later slices also touch, and you asked me to confirm this one rather than change it.

3. Worker argv contract — confirmed complete. Your enumeration matches mine. Every production spawn goes through the strip-and-inject in index_args_with_repo_path, so the payload is built in one place and cannot arrive short. index_supervisor.c and daemon/bootstrap.c plumb argv without ever constructing an index_repository payload, as you read them. The only hand-built payloads are the two shell harnesses, and both now build the full key set through a shared cbm_test_index_worker_policy_json helper in scripts/test-runtime.sh rather than each spelling it out — so a future key is a one-line change rather than a silent miss.

On the branch. Rebased onto current main and green. I also opened #1796 for the daemon start defect behind the Windows guard failures that were showing on this stack.