You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Surfaced by nightly regression triage (lane nightly-triage-f7ec48e1).
Observed failure
Reproduced locally with ./scripts/pytest-clean.sh <node>:
tests/unit/test_update_reflections_callables.py:295: in test_read_only_probe_does_not_clear_a_concurrent_failing_sentinel
assert result.success is True, result.detail
E AssertionError: FAIL: /Users/valorengels/src/ai/config/reflections.yaml: reflection 'improvement-evidence-collect' callable 'reflections.improvement_collect.run_improvement_collect' did not resolve with agent.sustainability banned: ModuleNotFoundError("No module named 'reflections.improvement_collect'")
E FAIL: 1 of 2 registry copy(ies) did not resolve: /Users/valorengels/src/ai/config/reflections.yaml
E assert False is True
E + where False = RegistryProbeResult(success=False, detail='FAIL: ...', sentinel_recorded=False, nothing_probed=False, sentinel_skipped=True).success
The test fails at its first assertion (result.success is True, line 295) and so never reaches the behavior it actually exists to guard — that record_sentinel=False leaves a concurrently-stamped failing sentinel intact (lines 296-298). Note sentinel_skipped=True in the result, so the suppression itself did behave correctly; only the precondition is unmet.
Likely cause
Real broken behavior, not a stale test. The reflections registry declares three entries whose Python modules do not exist anywhere in the repo — and never have.
config/reflections.yaml (gitignored, materialized from the vault copy at ~/Desktop/Valor/reflections.yaml) declares:
No — reflections/improvement_collect.py does not exist
side-effect-drain
reflections.housekeeping.side_effect_drain.run
config/reflections.yaml:456
No — not in reflections/housekeeping/
dead-letter-replay
reflections.housekeeping.dead_letter_replay.run
config/reflections.yaml:464
No — not in reflections/housekeeping/
git log -- <path> returns nothing for all three module paths: they were never committed, so this is not a deletion or a rename — the registry was written ahead of the code.
All three entries are enabled: true, with every: 900s / 60s / 300s respectively, so the scheduler is attempting to run three reflections that cannot resolve. side-effect-drain at every: 60s is the loudest: its description says it runs due SideEffectJob rows (post-session memory extraction) and dead-letters exhausted attempts, which means post-session side effects are very likely not draining at all on this machine.
Note reflections/housekeeping/ does contain analytics_rollup.py, disk_reclaim.py, disk_space_check.py, merged_branch_cleanup.py, and redis_ttl_cleanup.py — so the package itself resolves; only these two module names are missing.
Secondary cause specific to this node: the probe is not isolated from the machine's real registry
This test writes a clean registry to tmp_path and points REFLECTIONS_YAML at it (tests/unit/test_update_reflections_callables.py:282-286), which reads as an intent to be hermetic — and the module docstring (tests/unit/test_update_reflections_callables.py:4-7) states that every test passes explicit targets precisely so the real vault registry is never touched.
But run_registry_probe does not take targets. scripts/verify_registry_without_shim.py:116-131 builds a candidate list and probes all of them:
REFLECTIONS_YAML (the test's clean tmp file)
~/Desktop/Valor/reflections.yaml
_REPO_ROOT/config/reflections.yaml ← leaks in here
config/reflections.yaml in the checkout owning this worktree
Hence 1 of 2 registry copy(ies) did not resolve. This matters more here than for its red/green partner: this test is the guard against a documented fail-open (a passing --verify unlinking another run's failing verdict and handing remote-update.sh a green light onto an unresolvable registry, per the docstring at lines 268-280). A guard against a fail-open that itself goes red for unrelated environmental reasons is a guard that will get muted.
Suggested next steps
Decide per entry whether the code or the registry is wrong:
Apply the fix to the vault registry (~/Desktop/Valor/reflections.yaml), not just the materialized config/reflections.yaml copy — the repo copy is gitignored and regenerated, so a fix applied only there will be overwritten.
Check whether side-effect-drain failing to resolve has left SideEffectJob rows undrained on this machine, and whether the failure is visible anywhere other than this test.
Close the gap that let this land: a registry entry whose callable does not resolve should be caught before it reaches the vault file, not by a nightly test on one machine. scripts/verify_registry_without_shim.py already does the resolution check — the question is why nothing ran it at the point the vault registry was edited.
Related
Same root cause, filed separately in this triage batch:
Additionally, for this node specifically: give run_registry_probe a way to be scoped to explicit targets (mirroring run_reflections_callables_migration(..., targets=[...])), so this fail-open guard and its red/green partner test_probe_success_clears_a_stale_sentinel both run against a controlled registry instead of whatever this machine happens to have.
Failing node
Surfaced by nightly regression triage (lane
nightly-triage-f7ec48e1).Observed failure
Reproduced locally with
./scripts/pytest-clean.sh <node>:The test fails at its first assertion (
result.success is True, line 295) and so never reaches the behavior it actually exists to guard — thatrecord_sentinel=Falseleaves a concurrently-stamped failing sentinel intact (lines 296-298). Notesentinel_skipped=Truein the result, so the suppression itself did behave correctly; only the precondition is unmet.Likely cause
Real broken behavior, not a stale test. The reflections registry declares three entries whose Python modules do not exist anywhere in the repo — and never have.
config/reflections.yaml(gitignored, materialized from the vault copy at~/Desktop/Valor/reflections.yaml) declares:improvement-evidence-collectreflections.improvement_collect.run_improvement_collectconfig/reflections.yaml:448reflections/improvement_collect.pydoes not existside-effect-drainreflections.housekeeping.side_effect_drain.runconfig/reflections.yaml:456reflections/housekeeping/dead-letter-replayreflections.housekeeping.dead_letter_replay.runconfig/reflections.yaml:464reflections/housekeeping/git log -- <path>returns nothing for all three module paths: they were never committed, so this is not a deletion or a rename — the registry was written ahead of the code.All three entries are
enabled: true, withevery: 900s/60s/300srespectively, so the scheduler is attempting to run three reflections that cannot resolve.side-effect-drainatevery: 60sis the loudest: its description says it runs dueSideEffectJobrows (post-session memory extraction) and dead-letters exhausted attempts, which means post-session side effects are very likely not draining at all on this machine.Provenance from the registry descriptions:
improvement-evidence-collectcites Recursive self-improvement controller: Valor owns the discover, investigate, experiment, evaluate, release loop #3177 ("Recursive self-improvement controller"), which is still OPEN — its code has not landed, so the registry entry is premature.side-effect-drainanddead-letter-replayboth cite ETL-grade pipeline hardening: queued side effects, one dead-letter model, wire schemas, execution lease #3183 ("ETL-grade pipeline hardening"), which is CLOSED as COMPLETED — either those two reflection entry points were dropped from the implementation while the registry entry stayed, or they live under names the registry does not match.Note
reflections/housekeeping/does containanalytics_rollup.py,disk_reclaim.py,disk_space_check.py,merged_branch_cleanup.py, andredis_ttl_cleanup.py— so the package itself resolves; only these two module names are missing.Secondary cause specific to this node: the probe is not isolated from the machine's real registry
This test writes a clean registry to
tmp_pathand pointsREFLECTIONS_YAMLat it (tests/unit/test_update_reflections_callables.py:282-286), which reads as an intent to be hermetic — and the module docstring (tests/unit/test_update_reflections_callables.py:4-7) states that every test passes explicit targets precisely so the real vault registry is never touched.But
run_registry_probedoes not taketargets.scripts/verify_registry_without_shim.py:116-131builds a candidate list and probes all of them:REFLECTIONS_YAML(the test's clean tmp file)~/Desktop/Valor/reflections.yaml_REPO_ROOT/config/reflections.yaml← leaks in hereconfig/reflections.yamlin the checkout owning this worktreeHence
1 of 2 registry copy(ies) did not resolve. This matters more here than for its red/green partner: this test is the guard against a documented fail-open (a passing--verifyunlinking another run's failing verdict and handingremote-update.sha green light onto an unresolvable registry, per the docstring at lines 268-280). A guard against a fail-open that itself goes red for unrelated environmental reasons is a guard that will get muted.Suggested next steps
side_effect_drain,dead_letter_replay): confirm against the merged ETL-grade pipeline hardening: queued side effects, one dead-letter model, wire schemas, execution lease #3183 work whether these entry points exist under other names. If they do, repoint the registry callables. If they were never implemented, either implement them or remove the entries — but do not leave anenabled: true60s reflection pointing at a missing module.improvement-evidence-collect: Recursive self-improvement controller: Valor owns the discover, investigate, experiment, evaluate, release loop #3177 is still open, so set the entryenabled: falseor drop it until the controller lands.~/Desktop/Valor/reflections.yaml), not just the materializedconfig/reflections.yamlcopy — the repo copy is gitignored and regenerated, so a fix applied only there will be overwritten.side-effect-drainfailing to resolve has leftSideEffectJobrows undrained on this machine, and whether the failure is visible anywhere other than this test.scripts/verify_registry_without_shim.pyalready does the resolution check — the question is why nothing ran it at the point the vault registry was edited.Related
Same root cause, filed separately in this triage batch:
tests/unit/test_reflection_scheduler.py::TestRegistryIntegrity::test_all_callables_resolvetests/unit/test_update_reflections_callables.py::test_probe_success_clears_a_stale_sentineltests/unit/test_update_reflections_callables.py::test_read_only_probe_does_not_clear_a_concurrent_failing_sentinelAdditionally, for this node specifically: give
run_registry_probea way to be scoped to explicit targets (mirroringrun_reflections_callables_migration(..., targets=[...])), so this fail-open guard and its red/green partnertest_probe_success_clears_a_stale_sentinelboth run against a controlled registry instead of whatever this machine happens to have.Cluster siblings already filed: #3327, #3328.