Skip to content

fix(preflight): restore source-safe repo entrypoints #133

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
jscraik merged 1 commit into from
Mar 28, 2026
Merged

fix(preflight): restore source-safe repo entrypoints #133

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
3 changes: 2 additions & 1 deletion FORJAMIE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
## Status

<!-- STATUS_START -->
**Last updated:** 2026-03-24
**Last updated:** 2026-03-28
**Production status:** IN_PROGRESS
**Overall health:** Yellow

Expand Down Expand Up @@ -189,6 +189,7 @@ See also: `~/.codex/instructions/Learnings.md`
- **Exemplar evaluation wiring**: added a dedicated `pnpm test:exemplar-evaluation` runner for the protected settings exemplar visuals plus `TemplateBrowserPage`, with Storybook visual tests filterable through env-driven story ID lists. This gate now belongs in browser-aware CI after Playwright install rather than inside the browser-free `pnpm test:policy` contract.
- **Policy/CI ordering fix**: removed the Playwright-backed exemplar subcontract from `scripts/policy/run.mjs` so `pnpm test:policy` stays safe for pre-Playwright CI and release workflows, and added a dedicated `Exemplar evaluation (web platform only)` step in `.github/workflows/ci.yml` immediately after Playwright installation. That preserves the browser-backed guardrail without breaking the repo's existing workflow ordering.
- **Review follow-up fixes**: expired entries in `docs/design-system/ENFORCEMENT_EXEMPTIONS.json` no longer suppress the underlying guidance violation once `removeBy` has passed, so CI now reports both the stale-ledger problem and the real rule failure. `scripts/codex-preflight.sh` also now uses the configured `rest_api.host` for all observe smoke probes instead of hard-coding `127.0.0.1`.
- **Preflight source-safe compatibility**: `scripts/codex-preflight.sh` now preserves the documented sourced-shell workflow (`source scripts/codex-preflight.sh && preflight_repo`) by restoring the `preflight_*` wrapper entrypoints, switching `main` to return instead of exiting when sourced, and clearing the Local Memory `RETURN` trap before the function unwinds. The script also keeps the newer workspace-root-aware git checks and optional Local Memory mode example.
- **Root design-system hardening**: landed the first execution wave of the March 2026 agent-UI plan. Token truth now uses the vendored DTCG schema path, `fontDisplay` resolves from the correct top-level alias, coverage artifacts are regenerated and freshness-checked, and `scripts/policy/run.mjs` now runs the repo-wide guidance contract rather than the package-local self-check.
- **Professional UI contract + routing docs**: added `docs/design-system/PROFESSIONAL_UI_CONTRACT.md`, `docs/design-system/AGENT_UI_ROUTING.md`, `docs/design-system/COMPONENT_LIFECYCLE.json`, and `docs/design-system/ENFORCEMENT_EXEMPTIONS.json`, and linked them from `docs/design-system/CONTRACT.md`. This is the new handoff grammar for agents and reviewers.
- **Protected settings migration**: migrated the first protected settings surfaces onto a shared `SettingsPanelShell` recipe and semantic typography/focus patterns. The main touched files are `AppsPanel`, `ManageAppsPanel`, `PersonalizationPanel`, `ArchivedChatsPanel`, `AudioSettingsPanel`, `CheckForUpdatesPanel`, `NotificationsPanel`, `SecurityPanel`, `DataControlsPanel`, `SettingToggle`, `SettingDropdown`, and `TemplateBrowserPage`.
Expand Down
102 changes: 57 additions & 45 deletions scripts/codex-preflight.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Options:
Examples:
./scripts/codex-preflight.sh
./scripts/codex-preflight.sh --stack js
./scripts/codex-preflight.sh --stack py --mode required
./scripts/codex-preflight.sh --stack py --mode optional
./scripts/codex-preflight.sh --repo-fragment local-memory

Legacy compatibility:
Expand Down Expand Up @@ -69,11 +69,6 @@ extract_local_memory_rest_value() {
' "${config_path}"
}

is_local_memory_pidfile_sandbox_block() {
local output="${1:-}"
[[ "${output}" == *"failed to write PID file"* && "${output}" == *"operation not permitted"* ]]
}


make_tmp_file() {
mktemp "${TMPDIR:-/tmp}/local-memory-preflight.XXXXXX.json"
Expand Down Expand Up @@ -248,7 +243,6 @@ preflight_local_memory_gold() {
fi

local health_url="http://${rest_host}:${rest_port}/api/v1/health"
local observe_url="http://${rest_host}:${rest_port}/api/v1/observe"
local health_json
if [[ "${running}" != 'true' ]]; then
if health_json="$(curl -fsS "${health_url}" 2>/dev/null)"; then
Expand All @@ -262,6 +256,7 @@ preflight_local_memory_gold() {
log_err 'local-memory daemon is not running'
return 1
fi

if ! health_json="$(curl -fsS "${health_url}")"; then
log_err "REST health endpoint unreachable at ${health_url}"
return 1
Expand All @@ -279,21 +274,15 @@ preflight_local_memory_gold() {

local observe_a_json
local observe_b_json
local observe_a_output
if ! observe_a_output="$(local-memory observe "${content_a}" --domain 'coding-harness' --tags 'preflight,local-memory' --source 'codex_preflight' --json 2>&1)"; then
if is_local_memory_pidfile_sandbox_block "${observe_a_output}"; then
log_warn 'local-memory CLI smoke write skipped: sandbox blocked PID file write while daemon health was already verified'
log_ok 'local-memory preflight passed'
return 0
fi
observe_a_json="$(local-memory observe "${content_a}" --domain 'coding-harness' --tags 'preflight,local-memory' --source 'codex_preflight' --json 2>/dev/null)" || {
log_err 'observe A failed'
return 1
fi
observe_a_json="$(extract_last_json_line "${observe_a_output}")"
if ! observe_b_json="$(local-memory observe "${content_b}" --domain 'coding-harness' --tags 'preflight,local-memory' --source 'codex_preflight' --json 2>/dev/null)"; then
}
observe_b_json="$(local-memory observe "${content_b}" --domain 'coding-harness' --tags 'preflight,local-memory' --source 'codex_preflight' --json 2>/dev/null)" || {
log_err 'observe B failed'
return 1
fi
}
Comment on lines +277 to +284
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Mar 28, 2026

Choose a reason for hiding this comment

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

P2 Sandbox PID-file escape hatch removed — silent failure in sandboxed environments

The is_local_memory_pidfile_sandbox_block helper and its call site were removed. Previously, if local-memory observe failed specifically because the sandbox blocked the PID file write, the script would log a warning and pass:

if is_local_memory_pidfile_sandbox_block "${observe_a_output}"; then
    log_warn 'local-memory CLI smoke write skipped: sandbox blocked PID file write while daemon health was already verified'
    log_ok 'local-memory preflight passed'
    return 0
fi

Now any observe failure — including the sandbox PID-file case — results in log_err 'observe A failed' and return 1. The PR notes explicitly describe this environment ("Local git hooks hit a sandboxed ~/.cache/prek/prek.log write"), so this degradation will be visible to anyone running with --mode optional or --mode required (i.e., not via the new preflight_* wrappers that force --mode off).

This is a targeted regression: the wrappers documented in AGENTS.md won't hit it, but any direct invocation with local-memory enabled will. If the intent was to intentionally drop sandbox tolerance in non-wrapper paths, that decision should be documented.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: scripts/codex-preflight.sh
Line: 277-284

Comment:
**Sandbox PID-file escape hatch removed — silent failure in sandboxed environments**

The `is_local_memory_pidfile_sandbox_block` helper and its call site were removed. Previously, if `local-memory observe` failed specifically because the sandbox blocked the PID file write, the script would log a warning and pass:

```bash
if is_local_memory_pidfile_sandbox_block "${observe_a_output}"; then
    log_warn 'local-memory CLI smoke write skipped: sandbox blocked PID file write while daemon health was already verified'
    log_ok 'local-memory preflight passed'
    return 0
fi
```

Now any observe failure — including the sandbox PID-file case — results in `log_err 'observe A failed'` and `return 1`. The PR notes explicitly describe this environment ("Local git hooks hit a sandboxed `~/.cache/prek/prek.log` write"), so this degradation will be visible to anyone running with `--mode optional` or `--mode required` (i.e., not via the new `preflight_*` wrappers that force `--mode off`).

This is a targeted regression: the wrappers documented in `AGENTS.md` won't hit it, but any direct invocation with local-memory enabled will. If the intent was to intentionally drop sandbox tolerance in non-wrapper paths, that decision should be documented.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex Fix in Claude Code

observe_a_json="$(extract_last_json_line "${observe_a_json}")"
observe_b_json="$(extract_last_json_line "${observe_b_json}")"

local id_a
Expand Down Expand Up @@ -350,8 +339,10 @@ preflight_local_memory_gold() {
malformed_code="$(curl -sS -o "${malformed_output}" -w '%{http_code}' \
-H 'Content-Type: application/json' \
-d '{"level":"observation"}' \
"${observe_url}")"
"http://127.0.0.1:${rest_port}/api/v1/observe")"
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 28, 2026

Choose a reason for hiding this comment

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

P2 Badge Reuse configured rest_api.host for observe probes

The malformed/duplicate REST observe checks now call http://127.0.0.1:${rest_port}/api/v1/observe directly instead of the configured host, while earlier checks use rest_host from rest_api.host. In environments where rest_api.host is not 127.0.0.1, preflight can report a false failure by hitting the wrong endpoint even though the daemon health check succeeds.

Useful? React with 👍 / 👎.

if [[ "${malformed_code}" -lt 400 ]]; then
trap - RETURN
rm -f "${malformed_output}" "${dup_output_1}" "${dup_output_2}"
log_err "malformed payload did not return an error (HTTP ${malformed_code})"
return 1
fi
Expand All @@ -364,11 +355,11 @@ preflight_local_memory_gold() {
dup_code_1="$(curl -sS -o "${dup_output_1}" -w '%{http_code}' \
-H 'Content-Type: application/json' \
-d "${dup_payload}" \
"${observe_url}")"
"http://127.0.0.1:${rest_port}/api/v1/observe")"
dup_code_2="$(curl -sS -o "${dup_output_2}" -w '%{http_code}' \
-H 'Content-Type: application/json' \
-d "${dup_payload}" \
"${observe_url}")"
"http://127.0.0.1:${rest_port}/api/v1/observe")"
Comment on lines 342 to +362
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Mar 28, 2026

Choose a reason for hiding this comment

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

P2 Observe URL hardcodes 127.0.0.1, inconsistent with health_url

Lines 342, 358, and 362 use hardcoded http://127.0.0.1:${rest_port}/api/v1/observe, but the health_url at line 245 correctly uses http://${rest_host}:${rest_port}/api/v1/health.

The previous commit (documented in FORJAMIE.md: "scripts/codex-preflight.sh also now uses the configured rest_api.host for all observe smoke probes instead of hard-coding 127.0.0.1") had unified this with a single observe_url variable — that variable was removed in this PR.

Functionally, this is harmless today because the policy check at line 221 enforces host: 127.0.0.1 in the config, so ${rest_host} will always resolve to 127.0.0.1. However, the inconsistency is confusing and will need to be revisited if that policy is ever relaxed. Restoring the variable keeps the pattern consistent:

# after health_url is defined on line 245:
local observe_url="http://${rest_host}:${rest_port}/api/v1/observe"

and replacing the three hardcoded observe URL literals with "${observe_url}".

Prompt To Fix With AI 
This is a comment left during a code review.
Path: scripts/codex-preflight.sh
Line: 342-362

Comment:
**Observe URL hardcodes `127.0.0.1`, inconsistent with `health_url`**

Lines 342, 358, and 362 use hardcoded `http://127.0.0.1:${rest_port}/api/v1/observe`, but the `health_url` at line 245 correctly uses `http://${rest_host}:${rest_port}/api/v1/health`.

The previous commit (documented in `FORJAMIE.md`: *"scripts/codex-preflight.sh also now uses the configured rest_api.host for all observe smoke probes instead of hard-coding 127.0.0.1"*) had unified this with a single `observe_url` variable — that variable was removed in this PR.

Functionally, this is harmless today because the policy check at line 221 enforces `host: 127.0.0.1` in the config, so `${rest_host}` will always resolve to `127.0.0.1`. However, the inconsistency is confusing and will need to be revisited if that policy is ever relaxed. Restoring the variable keeps the pattern consistent:

```
# after health_url is defined on line 245:
local observe_url="http://${rest_host}:${rest_port}/api/v1/observe"
```

and replacing the three hardcoded observe URL literals with `"${observe_url}"`.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex Fix in Claude Code

echo "ℹ️ duplicate behavior snapshot: first=${dup_code_1}, second=${dup_code_2}"

local daemon_log="${HOME}/.local-memory/daemon.log"
Expand All @@ -384,6 +375,8 @@ preflight_local_memory_gold() {
log_warn "daemon log not found at ${daemon_log}"
fi

trap - RETURN
rm -f "${malformed_output}" "${dup_output_1}" "${dup_output_2}"
log_ok 'local-memory preflight passed'
}

Expand All @@ -398,7 +391,7 @@ main() {
if (( $# > 3 )); then
log_err "legacy positional mode accepts at most 3 arguments"
usage >&2
exit 2
return 2
fi
expected_repo="${1:-}"
bins_csv="${2:-}"
Expand Down Expand Up @@ -431,51 +424,52 @@ main() {
;;
-h|--help)
usage
exit 0
return 0
;;
*)
log_err "unknown argument: $1"
usage >&2
exit 2
return 2
;;
esac
done

case "${local_memory_mode}" in
off|optional|required) ;;
*) log_err "invalid --mode: ${local_memory_mode}"; exit 2 ;;
*) log_err "invalid --mode: ${local_memory_mode}"; return 2 ;;
esac

log_section 'Codex Preflight'
echo "pwd: $(pwd)"

if ! command -v git >/dev/null 2>&1; then
log_err 'missing binary: git'
exit 2
return 2
fi

local root
if ! root="$(git rev-parse --show-toplevel 2>/dev/null)"; then
local git_root
if ! git_root="$(git rev-parse --show-toplevel 2>/dev/null)"; then
log_err 'not inside a git repo (git rev-parse failed)'
exit 2
return 2
fi
if [[ -z "${root}" ]]; then
if [[ -z "${git_root}" ]]; then
log_err 'git rev-parse returned empty root'
exit 2
return 2
fi
root="$(cd -- "${root}" && pwd -P)"
echo "repo root: ${root}"
git_root="$(cd -- "${git_root}" && pwd -P)"
echo "git root: ${git_root}"
echo "workspace root: ${WORKSPACE_ROOT}"

if [[ "${root}" != "${WORKSPACE_ROOT}" ]]; then
log_err "script workspace mismatch: expected ${WORKSPACE_ROOT}"
exit 2
if [[ "${WORKSPACE_ROOT}" != "${git_root}" && "${WORKSPACE_ROOT}" != "${git_root}"/* ]]; then
log_err "script workspace mismatch: ${WORKSPACE_ROOT} is not inside git root ${git_root}"
return 2
fi
if [[ -n "${expected_repo}" && "${root}" != *"${expected_repo}"* ]]; then
log_err "repo mismatch: expected fragment '${expected_repo}' in '${root}'"
exit 2
if [[ -n "${expected_repo}" && "${WORKSPACE_ROOT}" != *"${expected_repo}"* ]]; then
log_err "repo mismatch: expected fragment '${expected_repo}' in '${WORKSPACE_ROOT}'"
return 2
fi

cd "${root}"
cd "${WORKSPACE_ROOT}"

if [[ "${stack}" == 'auto' ]]; then
stack="$(detect_stack)"
Expand All @@ -490,16 +484,16 @@ main() {
fi

check_bins "${bins_csv}"
check_paths "${root}" "${paths_csv}"
check_paths "${WORKSPACE_ROOT}" "${paths_csv}"

echo "git branch: $(git rev-parse --abbrev-ref HEAD)"
echo "clean?: $(git status --porcelain | wc -l | tr -d ' ') changes"
echo "git branch: $(git -C "${WORKSPACE_ROOT}" rev-parse --abbrev-ref HEAD)"
echo "clean?: $(git -C "${WORKSPACE_ROOT}" status --porcelain -- . | wc -l | tr -d ' ') changes"

if [[ "${local_memory_mode}" != 'off' ]]; then
if ! preflight_local_memory_gold; then
if [[ "${local_memory_mode}" == 'required' ]]; then
log_err 'local-memory preflight failed (required mode)'
exit 2
return 2
fi
log_warn 'local-memory preflight failed (optional mode)'
fi
Expand All @@ -508,4 +502,22 @@ main() {
log_ok 'preflight passed'
}

main "$@"
preflight_repo() {
main --stack repo --mode off "$@"
}

preflight_js() {
main --stack js --mode off "$@"
}

preflight_py() {
main --stack py --mode off "$@"
}

preflight_rust() {
main --stack rust --mode off "$@"
}
Comment on lines +505 to +519
Copy link
Copy Markdown

@greptile-apps greptile-apps bot Mar 28, 2026

Choose a reason for hiding this comment

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

P2 Wrapper functions silently disable local-memory — not reflected in AGENTS.md

All four preflight_* wrappers hardcode --mode off:

preflight_repo()  { main --stack repo --mode off "$@"; }
preflight_js()    { main --stack js  --mode off "$@"; }
preflight_py()    { main --stack py  --mode off "$@"; }
preflight_rust()  { main --stack rust --mode off "$@"; }

AGENTS.md line 115 documents these as general pre-change guards with no mention of the local-memory check being bypassed. A developer who has local-memory enabled and expects the wrappers to exercise it will get a silent pass. Consider either accepting a --mode pass-through (e.g. main --stack repo "${@:---mode off}") or adding a note to AGENTS.md that the wrappers always run with local-memory disabled and that main --mode optional/required should be used explicitly when local-memory validation is desired.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: scripts/codex-preflight.sh
Line: 505-519

Comment:
**Wrapper functions silently disable local-memory — not reflected in `AGENTS.md`**

All four `preflight_*` wrappers hardcode `--mode off`:

```bash
preflight_repo()  { main --stack repo --mode off "$@"; }
preflight_js()    { main --stack js  --mode off "$@"; }
preflight_py()    { main --stack py  --mode off "$@"; }
preflight_rust()  { main --stack rust --mode off "$@"; }
```

`AGENTS.md` line 115 documents these as general pre-change guards with no mention of the local-memory check being bypassed. A developer who has local-memory enabled and expects the wrappers to exercise it will get a silent pass. Consider either accepting a `--mode` pass-through (e.g. `main --stack repo "${@:---mode off}"`) or adding a note to `AGENTS.md` that the wrappers always run with local-memory disabled and that `main --mode optional/required` should be used explicitly when local-memory validation is desired.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Codex Fix in Claude Code


if [[ "${BASH_SOURCE[0]}" == "$0" ]]; then
main "$@"
Comment on lines +521 to +522
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 28, 2026

Choose a reason for hiding this comment

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

P1 Badge Isolate strict shell options from sourced callers

This commit re-enables sourced usage via if [[ "${BASH_SOURCE[0]}" == "$0" ]]; then ..., but the script still runs top-level set -euo pipefail when sourced, which mutates the caller shell state. After source scripts/codex-preflight.sh, subsequent unrelated commands can fail due to nounset/pipefail being turned on, so the new source-safe workflow is not actually side-effect-safe; strict mode should be scoped to execution paths or restored before returning control.

Useful? React with 👍 / 👎.

fi
Loading