Skip to content

[codex] Clarify runtime route roles #174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Pigbibi merged 1 commit into from
Jun 18, 2026
Merged

[codex] Clarify runtime route roles #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions .github/workflows/invoke-cloud-run.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ jobs:
if [[ "${raw_path}" != /* ]]; then
raw_path="/${raw_path}"
fi
if [ "${raw_path}" = "/" ] && [ "${{ inputs.allow_live_execution }}" != "true" ]; then
echo "Calling / can trigger the live execution entrypoint. Re-run with allow_live_execution=true if this is intentional." >&2
if { [ "${raw_path}" = "/" ] || [ "${raw_path}" = "/run" ]; } && [ "${{ inputs.allow_live_execution }}" != "true" ]; then

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Guard /run even when the path has a query string

When this workflow is used against a non-internal service, a path like /run?reason=manual will not satisfy this exact /run comparison, so it falls through to the direct curl invocation while Flask still routes it to the new /run live handler. That bypasses allow_live_execution=false and can trigger live trading; parse or normalize the path component before applying the live-entrypoint guard.

Useful? React with 👍 / 👎.

echo "Calling ${raw_path} can trigger the live execution entrypoint. Re-run with allow_live_execution=true if this is intentional." >&2
exit 1
fi

Expand Down Expand Up @@ -140,21 +140,25 @@ jobs:
invoke_method="direct"
scheduler_job=""
scheduler_location=""
scheduler_expected_path=""
if [ "${service_ingress}" = "internal" ]; then
scheduler_location="${CLOUD_SCHEDULER_LOCATION:-${CLOUD_RUN_REGION}}"
case "${raw_path}" in
/)
/|/run)
scheduler_job="${CLOUD_RUN_SERVICE}-scheduler"
scheduler_expected_path="/"
;;
/probe)
scheduler_job="${CLOUD_RUN_SERVICE}-probe-scheduler"
scheduler_expected_path="/probe"
;;
/precheck)
/precheck|/dry-run)
scheduler_job="${CLOUD_RUN_SERVICE}-precheck-scheduler"
scheduler_expected_path="/precheck"
;;
*)
echo "Cloud Run service ${CLOUD_RUN_SERVICE} has internal ingress, so GitHub-hosted runners cannot curl ${raw_path} directly." >&2
echo "Use one of the scheduler-backed paths: /, /probe, /precheck." >&2
echo "Use one of the scheduler-backed paths: /, /run, /probe, /precheck, /dry-run." >&2
exit 1
;;
esac
Expand All @@ -179,15 +183,15 @@ jobs:
print(normalize(urlparse(os.environ["SCHEDULER_URI"]).path))
PY
)"
requested_path="$(RAW_PATH="${raw_path}" python3 - <<'PY'
requested_path="$(RAW_PATH="${scheduler_expected_path}" python3 - <<'PY'
import os

clean = (os.environ["RAW_PATH"] or "/").rstrip("/")
print(clean or "/")
PY
)"
if [ "${scheduler_path}" != "${requested_path}" ]; then
echo "Cloud Scheduler job ${scheduler_job} targets ${scheduler_uri}, not ${raw_path}." >&2
echo "Cloud Scheduler job ${scheduler_job} targets ${scheduler_uri}, not ${scheduler_expected_path}." >&2
exit 1
fi
invoke_method="scheduler"
Expand Down
27 changes: 19 additions & 8 deletions main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,14 +601,6 @@ def run_probe(*, response_body: str = "Probe OK"):
composer = build_composer(dry_run_only_override=True)
reporting_adapters = composer.build_reporting_adapters()
log_context, report = reporting_adapters.start_run()
strategy_plugin_signals, strategy_plugin_error = composer.load_strategy_plugin_signals(
getattr(RUNTIME_SETTINGS, "strategy_plugin_mounts_json", None)
)
composer.attach_strategy_plugin_report(
report,
signals=strategy_plugin_signals,
error=strategy_plugin_error,
)
reporting_adapters.log_event(
log_context,
"health_probe_received",
Expand Down Expand Up @@ -681,6 +673,7 @@ def run_probe(*, response_body: str = "Probe OK"):


@app.route("/", methods=["POST", "GET"])
@app.route("/run", methods=["POST", "GET"])
def handle_trigger():
"""Entrypoint for Cloud Run / scheduler: run strategy and return 200."""
return _route_with_runtime_error_fallback(
Expand Down Expand Up @@ -715,6 +708,19 @@ def handle_precheck():
)


@app.route("/dry-run", methods=["POST", "GET"])
def handle_dry_run():
"""Strategy dry-run entrypoint; alias of precheck with clearer operator wording."""
return _route_with_runtime_error_fallback(
run_strategy,
force_run=True,
validation_only=True,
validation_label="precheck",
success_body="Dry Run OK",
route_label="POST /dry-run",
)


@app.route("/probe", methods=["POST", "GET"])
def handle_probe():
"""Post-open broker/account health probe; notify only on failure."""
Expand All @@ -725,5 +731,10 @@ def handle_probe():
)


@app.route("/health", methods=["GET"])
def health():
return "OK", 200


if __name__ == "__main__":
app.run(host='0.0.0.0', port=int(os.environ.get('PORT', 8080)))
43 changes: 39 additions & 4 deletions tests/test_request_handling.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ def test_cloud_run_route_contracts_are_registered(self):
module = load_module()

self.assertIs(module.app._routes[("/", ("POST", "GET"))], module.handle_trigger)
self.assertIs(module.app._routes[("/run", ("POST", "GET"))], module.handle_trigger)
self.assertIs(
module.app._routes[("/backfill", ("POST", "GET"))],
module.handle_backfill,
Expand All @@ -212,10 +213,24 @@ def test_cloud_run_route_contracts_are_registered(self):
module.app._routes[("/precheck", ("POST", "GET"))],
module.handle_precheck,
)
self.assertIs(
module.app._routes[("/dry-run", ("POST", "GET"))],
module.handle_dry_run,
)
self.assertIs(
module.app._routes[("/probe", ("POST", "GET"))],
module.handle_probe,
)
self.assertIs(module.app._routes[("/health", ("GET",))], module.health)

def test_health_route_returns_ok(self):
module = load_module()

with module.app.test_request_context("/health", method="GET"):
body, status = module.health()

self.assertEqual(status, 200)
self.assertEqual(body, "OK")

def test_handle_trigger_runs_strategy(self):
module = load_module()
Expand Down Expand Up @@ -356,6 +371,26 @@ def fake_run_strategy(*, force_run=False, validation_only=False, validation_labe
self.assertTrue(observed["validation_only"])
self.assertEqual(observed["validation_label"], "precheck")

def test_handle_dry_run_alias_forces_strategy_dry_run(self):
module = load_module()
observed = {"force_run": None, "validation_only": None}

def fake_run_strategy(*, force_run=False, validation_only=False, validation_label="backfill"):
observed["force_run"] = force_run
observed["validation_only"] = validation_only
observed["validation_label"] = validation_label

module.run_strategy = fake_run_strategy

with module.app.test_request_context("/dry-run", method="POST"):
body, status = module.handle_dry_run()

self.assertEqual(status, 200)
self.assertEqual(body, "Dry Run OK")
self.assertTrue(observed["force_run"])
self.assertTrue(observed["validation_only"])
self.assertEqual(observed["validation_label"], "precheck")

def test_handle_probe_checks_account_snapshot_without_success_notification(self):
module = load_module()
observed = {"override": None, "events": [], "notifications": []}
Expand Down Expand Up @@ -391,10 +426,10 @@ def build_notification_adapters(self):
raise AssertionError("probe success should stay silent")

def load_strategy_plugin_signals(self, *_args, **_kwargs):
return (), None
raise AssertionError("health probe should not load strategy plugins")

def attach_strategy_plugin_report(self, *_args, **_kwargs):
return None
raise AssertionError("health probe should not attach strategy plugin reports")

module.build_composer = lambda *, dry_run_only_override=None: observed.__setitem__("override", dry_run_only_override) or FakeComposer()

Expand Down Expand Up @@ -442,10 +477,10 @@ def build_notification_adapters(self):
return FakeNotifications()

def load_strategy_plugin_signals(self, *_args, **_kwargs):
return (), None
raise AssertionError("health probe should not load strategy plugins")

def attach_strategy_plugin_report(self, *_args, **_kwargs):
return None
raise AssertionError("health probe should not attach strategy plugin reports")

module.build_composer = lambda *, dry_run_only_override=None: FakeComposer()

Expand Down