test: stabilize OPA timeout regression

[dpaul0501]

Summary

• increase the fake OPA timeout test budget from 30 ms to 500 ms
• avoid racing the fake OPA script before it writes the PID file on slower local environments

Repro

Before this change, running the core package tests on this machine failed in opa_timeout_kills_child_process_without_lingering because core/tests/opa.rs attempted to read a PID file that had not been written yet.

Test

• cargo test -p agent_control_specification_core --test opa --quiet
• cargo test -p agent_control_specification_core --quiet

Signed-off-by: Debjyoti Paul debjyotipaul@users.noreply.github.com

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[github-actions]

Welcome to the Agent Governance Toolkit! Thanks for your first pull request.
Please ensure tests pass, code follows style (ruff check), and you have signed the CLA.
See our Contributing Guide.

[github-actions]

PR Review Summary

Check	Status	Details
🔍 Code Review	⚠️ Missing	No current-run comment
🛡️ Security Scan	⚠️ Missing	No current-run comment
🔄 Breaking Changes	⚠️ Missing	No current-run comment
📝 Docs Sync	⚠️ Missing	No current-run comment
🧪 Test Coverage	⚠️ Missing	No current-run comment

Verdict: ⚠️ AI review incomplete; ready for human review

AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims.

[Imran Siddique (imran-siddique)]

Read the test rather than the number, and the diagnosis in your PR body is exactly right. Worth
spelling out on the thread, because a one-line timeout bump looks like the usual "make the flake go
away" change and this one is not.

The fake OPA script is:

echo $$ > "$ACS_FAKE_OPA_PID_FILE"
exec sleep 30

and the assertion that actually matters is the last one, that
/proc/<pid> no longer exists after the timeout. That assertion needs the PID file to have been
written. With a 30 ms budget the runner can reach its timeout and kill the process group before
/bin/sh has spawned, written the file and reached exec, at which point
fs::read_to_string(&pid_file).unwrap() panics on a missing file. So the failure was never in the
timeout behaviour being tested, it was the harness racing its own fixture, which is why it presents
as a mysterious flake on slower machines and never on a warm CI runner.

500 ms is the right value for the same reason. It is far enough above shell startup to make the race
disappear and still two orders of magnitude below the fixture's sleep 30, so
error.detail().contains("OPA eval exceeded timeout") still asserts a real timeout rather than a
completed run. Nothing about what the test proves changes.

Nothing blocking. The cost is up to half a second on one #[cfg(unix)] test, which is a good trade
for an assertion that is currently unreliable.

[dpaul0501]

Thanks, that framing is exactly what I was trying to preserve here.

To make the intent explicit: the timeout value is not the behavior under test. The behavior under test is that an OPA child process which has entered the fake long-running command is killed after the evaluator timeout, and the final assertion verifies that /proc/<pid> no longer exists.

The old 30 ms value let the harness race its own fixture: on a slower host, the timeout could fire before /bin/sh had written ACS_FAKE_OPA_PID_FILE and reached exec sleep 30, so the test could panic while trying to read the missing PID file. In that failure mode, the test has not actually reached the condition it is supposed to verify.

The 500 ms budget gives the fixture enough time to publish the PID deterministically, while remaining far below the fake command's sleep 30. So the test still exercises the timeout path and still verifies child-process cleanup; it just removes the setup race before the real assertion.

[MohammadHaroonAbuomar]

Approved after group integration review (tests, gates and adversarial pass on the combined change).