Conversation
Both Alloy.Runner.Tests and Tlc.Runner.Tests walked up from `Directory.GetCurrentDirectory()` to find Zeta.sln. CWD is process-global mutable state; xUnit parallelizes test classes, so `WitnessDurableBackingStore canonicalises workDir under CWD churn` (tests/Tests.FSharp/Storage/Durability.Tests.fs) racing against the Alloy module's static initializer produced a TypeInitializationException on macOS-14 in PR #54's gate run — walking up from a churned CWD never finds Zeta.sln. TLC's module has identical code and passes, not because it's safe, but because discovery order happens to fire its cctor at a moment when CWD is settled. Switching both to `AppContext.BaseDirectory` (immutable for the AppDomain's lifetime) closes the race at the source rather than relying on xUnit scheduling luck. 12 local Alloy+TLC tests pass post-fix on darwin. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… (Copilot P0)
The doc claimed a "hand-verified script run `curl | bash`-style after
a careful read is safe" (line 71), but `curl | bash` pipes bytes to
the shell before any read is possible — contradicting step 1 of the
four-step protocol in the same section ("Download to disk, do not
execute") and Aaron's quoted standing policy ("validate them first").
The fix is twofold. First-contact ingest disallows `curl | bash`
because the pipe prevents validation. After SHA-256 pinning the
validated content, `curl <pinned-url> | bash` becomes acceptable in
automation because the hash verifies before the pipe executes — the
pin is the cached review, earned at first contact via the protocol.
Preserves the "unvalidated content is the risk" framing the doc
already had; removes the inconsistent example that made the pattern
look permissive at first contact. Copilot-pull-request-reviewer
comment on PR #54 flagged this; resolves the P0 inconsistency
without loosening the policy.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
AceHack
deleted the
sync/acehack-to-lfg-batch-4-code-fixes-AppContext-curlbash
branch
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f479c7461a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+100
to
+102
| - **After SHA-256-pinning, `curl <pinned-url> | bash` becomes | ||
| acceptable** in automation — the pin is the cached review, | ||
| and the hash is verified before the pipe executes. But the |
There was a problem hiding this comment.
curl | bash guidance
The new guidance says curl <pinned-url> | bash is acceptable because "the hash is verified before the pipe executes," but that command performs no digest verification at all; it streams network bytes directly into execution. In automation that follows this text literally, a compromised or repointed URL would still execute unvalidated content, which undermines the four-step protocol this section is defining. Please require an explicit verify-before-exec flow (download, hash check, then run) instead of documenting a bare pipe command as safe.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR syncs two missing fixes from AceHack→LFG: making repo-root discovery in formal test runners stable under xUnit parallelism, and removing a supply-chain guidance contradiction around pipe-to-shell.
Changes:
- Switch formal test repo-root discovery from
Directory.GetCurrentDirectory()toAppContext.BaseDirectoryto avoid CWD races during module static initialization. - Update supply-chain guidance to emphasize content review over hashes and clarify that
curl | bashis disallowed at first contact. - Add explanatory comments describing the underlying CI flake/race and the intended safe-ingress protocol.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/Tests.FSharp/Formal/Tlc.Runner.Tests.fs | Repo-root discovery now walks up from AppContext.BaseDirectory with added rationale comments. |
| tests/Tests.FSharp/Formal/Alloy.Runner.Tests.fs | Same repo-root discovery change for the Alloy runner tests with added rationale comments. |
| docs/security/SUPPLY-CHAIN-SAFE-PATTERNS.md | Rewords content-review guidance and adds explicit first-contact prohibition for `curl |
Comment on lines
+97
to
+104
| which makes step 2 impossible. Use `curl -o path && bash | ||
| path` (or equivalent split) so the content lands on disk | ||
| first. | ||
| - **After SHA-256-pinning, `curl <pinned-url> | bash` becomes | ||
| acceptable** in automation — the pin is the cached review, | ||
| and the hash is verified before the pipe executes. But the | ||
| pin must have been earned by the four-step protocol the | ||
| first time the content was admitted. |
There was a problem hiding this comment.
P0: This section reintroduces a pipe-to-shell recommendation (curl <pinned-url> | bash). Even with a pinned hash, piping directly to bash makes it impossible to verify the bytes on disk before execution, and it conflicts with the factory rule of never using pipe-to-shell patterns in guidance. Suggest rewriting the "after SHA-256-pinning" guidance to still download to disk and verify the digest before executing, without showing any curl | bash-style example as acceptable.
Suggested change
| which makes step 2 impossible. Use `curl -o path && bash | |
| path` (or equivalent split) so the content lands on disk | |
| first. | |
| - **After SHA-256-pinning, `curl <pinned-url> | bash` becomes | |
| acceptable** in automation — the pin is the cached review, | |
| and the hash is verified before the pipe executes. But the | |
| pin must have been earned by the four-step protocol the | |
| first time the content was admitted. | |
| which makes step 2 impossible. Download to disk first, then | |
| review and execute the saved file (`curl -o path` followed | |
| by `bash path`, or equivalent split). | |
| - **After SHA-256-pinning, automation must still download to | |
| disk, verify the pinned digest, and only then execute the | |
| saved file** — the pin is the cached review, not permission | |
| to stream code directly into a shell. The pin must have been | |
| earned by the four-step protocol the first time the content | |
| was admitted. |
Comment on lines
+49
to
+50
| // of the AppDomain and always points at | ||
| // `<repo>/tests/Tests.FSharp/bin/Release/net10.0/` under |
There was a problem hiding this comment.
P2: The comment claims AppContext.BaseDirectory "always" points at .../bin/Release/net10.0/ under dotnet test. That’s not generally true (Debug vs Release, different TFMs, IDE runners). Suggest rewording to something configuration-agnostic (e.g., bin/<Configuration>/<TFM>/) or just describing it as the test assembly output directory.
Suggested change
| // of the AppDomain and always points at | |
| // `<repo>/tests/Tests.FSharp/bin/Release/net10.0/` under | |
| // of the AppDomain and points at the test assembly output | |
| // directory (for example `bin/<Configuration>/<TFM>/`) under |
AceHack
added a commit
that referenced
this pull request
Apr 27, 2026
…on (#636) * tick-history: 18:02Z sync option-c COMPLETE + substrate transition Sync work done. Task #284 marked completed. Forward-sync PRs landed: #592 batch-1, #633 batch-2 (23 per-row-files), #634 batch-3 in CI, #635 batch-4. Batches 5+6 ALL SUPERSEDED-DISCARD per Otto-347 2nd-agent verify. Reverse-sync via Option C (close PR #15, UPSTREAM-RHYTHM cadence going forward). Substrate yield exceeded sync output: ~12 memory files + 3 lineage anchors (harbor+blade=Radical Candor, SRE-as-Substrate-RE, Rodney's-Razor=Occam+Brooks+Kolmogorov+Bennett+Gell-Mann) emerged during the sync work. Otto-347 2nd-agent verify load-bearing in both directions (caught real loss on #618 + confirmed 7 SUPERSEDED calls). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: standardize "Option C" casing in tick-history row 2026-04-26T18:02Z (Copilot P2) Copilot P2 nit on PR #636: row mixed "option-c" (twice) and "Option C" (once). Standardize on "Option C" across the row so later searches over tick-history don't miss matches. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
AceHack
added a commit
to AceHack/Zeta
that referenced
this pull request
Apr 28, 2026
…rip plan (task Lucent-Financial-Group#302) (#31) * adr(sync-drain): canonicalize AceHack <-> LFG option-c 7-step round-trip plan (task Lucent-Financial-Group#302) Why: - Aaron 2026-04-26 asked "do you have the 7 step plan?" and Otto had to reconstruct from git history because the plan lived as commits + tick-history rows, not a single doc. - The plan ITSELF is reusable — every sync drain cycle going forward follows the same shape — but without a canonical doc each future agent has to re-reconstruct. - Aaron offered "if you want it as a permanent ADR / canonical-plan doc, that's a small follow-up I can ship". Picking it up. - Per Aaron's "we got tons to do" framing: this is real shippable work that doesn't queue noise. What: - New file docs/DECISIONS/2026-04-26-sync-drain-plan-acehack-lfg-roundtrip-option-c.md (~150 lines) - Archive-header §33 compliance (Scope / Attribution / Operational status / Non-fusion disclaimer) - Decision: option-c (cherry-pick-with-rewrites) chosen over options a/b/d with rejection rationale per option - 7-step round-trip plan documented with commit refs, PRs, and tick-history anchors: 1. batch-1: foundation files (PR Lucent-Financial-Group#592, commit 1c1bd95) 2. batch-2: BACKLOG row migration (PR Lucent-Financial-Group#633, commits a3b7e24/fecd8d0) 3. batch-3: terminology canonicalization (PR Lucent-Financial-Group#634, commits ff4ee39/a1d781c) 4. batch-4: bug fixes + tooling hygiene (PR Lucent-Financial-Group#635, commit 05d274f) 5. closure: tick-history + substrate transition (commit e4b1fa2) 6. reverse leg: LFG -> AceHack via 7-parallel-subagent merge (PR #26) 7. steady state: UPSTREAM-RHYTHM batched cadence (every ~10 PRs) - Consequences (positive + negative + mitigations) documented - Convergence test: if next sync drain cycle modifies <= 1 step, the template is stable; if 3+ modifications, overfit and needs revision - Composes with 8 sibling memories + 2 prior ADRs + tasks Lucent-Financial-Group#284 and Lucent-Financial-Group#302 Closes the documentation gap surfaced when Aaron asked the 7-step-plan question. Future sync drain cycles can adopt this template directly. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(pr-31): address review threads (3 form-1 + 2 form-2 + 1 form-1 prose-fix) - **3 feedback files brought in-repo** per the 2026-04-24 in-repo-canonicalization shift (`feedback_natural_home_of_memories_is_in_repo_now_all_types_glass_halo_full_git_native_2026_04_24.md`): - feedback_fork_pr_cost_model_prs_land_on_acehack_sync_to_lfg_in_bulk.md - feedback_parallel_subagent_dispatch_for_content_preserving_merge_pattern_2026_04_26.md - feedback_dont_invent_when_existing_vocabulary_exists.md Resolves Codex P2 thread (line 124) + Copilot threads (line 125, 60) about non-existent feedback citations. The files existed in user-scope memory; the in-repo copy makes the cross-references resolvable per Otto-339 + the directional shift toward in-repo canonicalization. - **Citation-convention preamble** added to "## 7-step round-trip structure" before Step 1 documenting the repo-default for bare SHAs/PR numbers (Lucent-Financial-Group/Zeta for steps 1-6, AceHack/Zeta for step 7 with explicit inline deviations). Step 1's citations qualified explicitly as a reference example. Resolves Codex P2 thread (line 47) about bare-short-SHA verification ambiguity. Threads being closed as form-2 (already-addressed) with replies: - Thread 3 (Copilot, line 8): ADR header IS structured — uses the GOVERNANCE §33 archive-header format (Scope/Attribution/Operational status/Non-fusion disclaimer) which is the post-2026-04-26 standard for cross-substrate ADRs. The Date/Status/Deciders convention from earlier ADRs (2026-04-20-tools-scripting-language.md) is the pre-§33 format; both encode the same metadata in different keys. - Thread 4 (Copilot, line 12): GOVERNANCE.md §33 DOES exist at line 765 ("Archived external conversations require boundary headers") — verified via `grep -n '^33\.' GOVERNANCE.md`. The "archive-header" terminology is informal but the section IS about archive boundary headers. Form-2 closure with line citation in the thread reply. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Batch-4 of task #284 AceHack→LFG cherry-pick sync (option-c). Two genuine code/doc fixes that LFG was missing — verified by Otto-347 2nd-agent audit.
What this PR fixes
Commit #23 (
17f38fb) — repoRoot discovery uses AppContext.BaseDirectory, not CWDtests/Tests.FSharp/Formal/Alloy.Runner.Tests.fs+Tlc.Runner.Tests.fsDirectory.GetCurrentDirectory()walk-up — racy because xUnit parallelizes test classes and CWD is process-global mutable state (WitnessDurableBackingStoretest churns CWD)AppContext.BaseDirectorywalk-up — immutable for process lifetime, race-freeCommit #29 (
177a981) — SUPPLY-CHAIN-SAFE-PATTERNS curl|bash self-contradictiondocs/security/SUPPLY-CHAIN-SAFE-PATTERNS.mdOtto-347 2nd-agent verification trail
Test plan
Sync progress