knowledge: guard insertion vs execution order (new change-impact page); coordinator reset-before-send race; Bash-hook guards miss native Edit/Write (3 insights: 1 new page, 2 merged) - #110
Open
choiyounggi wants to merge 1 commit into
Conversation
…n order (new change-impact page), coordinator reset-before-send race, Bash-hook guards vs native Edit/Write tools
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.
Knowledge flush — 3 insight(s)
Verified best-practice
1. Guard placement vs. actual execution order (dev-loop t106).
Claim: when a plan inserts a precondition guard into an existing script, verify the
insertion point against the target file's real line order — an earlier unconditional
default/auto-create side effect makes a later guard dead code, even when the plan's
underlying decision is correct.
Verification: mechanism is precondition doctrine — checked against the Eiffel Design
by Contract documentation (https://www.eiffel.org/doc/eiffel/ET-_Design_by_Contract_(tm)%2C_Assertions_and_Exceptions),
which states preconditions are monitored "on routine entry", i.e. before any body
statement runs. Session evidence is a concrete reproduction:
status-update.sh:19unconditionally created the status file before the plan's proposed guard position;
the revised placement (between lines 18–19) made the BATS case "rework with no
status file → exit 4, no file created" pass. Confidence: field-tested
(doctrine-backed mechanism, field reproduction; no single official doc states the
whole directive).
2. Coordinator status reset must precede prompt re-delivery (linkly iss0817 t60).
Claim: in multi-session orchestration, resetting a task's status file after
re-sending the worker prompt races the worker's first progress signal; the file is
last-write-wins, so the coordinator's late reset erases
plan_readyand deadlocksthe watch.
Verification: the mechanism is an uncoordinated-writers file race — checked against
https://en.wikipedia.org/wiki/Race_condition (programs colliding on a shared file
produce order-dependent results; coordination or a single writer is required).
Session evidence: timestamps show worker
plan_ready(12:53:0x) overwritten by thecoordinator's
pendingre-seed (12:53:07); the tmux pane recorded "status set toplan_ready" while the file read
pending. Confidence: field-tested.3. Bash-hook write guards do not cover native Edit/Write tools (linkly run).
Claim: a worktree-isolation guard implemented as a Bash-command hook is silently
bypassed when the agent edits files via its native Edit/Write tools; isolation
needs a relative-paths-only brief instruction plus a pre-merge
git statusof theprotected tree.
Verification: checked against the official Claude Code hooks documentation
(https://code.claude.com/docs/en/hooks) — tool-event matchers filter on the tool
name; "
Bashmatches only the Bash tool", so a Bash-matcher hook never fires onEdit/Write calls. Session evidence: a worker under a
worktree_escapeguardmodified two
examples/*.lnplfiles in the main checkout via the Edit tool withno block and no log; found only when
git pullfailed. Confidence: verified(doc-backed mechanism + field observation).
Existing-layer check
Pages read: infrastructure-agent-orchestration-shared-run-state, infrastructure-agent-orchestration-worktree-isolated-workers, infrastructure-agent-orchestration-control-signals-vs-primary-artifacts, testing-quality-guard-shape-vs-consequence, qa-exploratory-guard-true-path-coverage, backend-common-change-impact-call-site-enumeration
Also read: root
INDEX.md,wiki/infrastructure/index.md,wiki/debugging/index.md,wiki/backend/index.md(routing tables).guard-shape-vs-consequence(repo-wide guard tests) andguard-true-path-coverage(branch coverage of guarded steps) share the "guardthat never actually protects" theme but have different triggers → new page,
cross-linked to
call-site-enumeration(same category) andguard-true-path-coverage(both ways).shared-run-stateowns file-based coordination but had nothing oncoordinator/worker write ordering during re-delivery → merged (+1 edge case,
+1 Instead-of, +2 sources, related += pane-delivery-confirmation). No conflict
with existing directives.
control-signals-vs-primary-artifactsalready has the worker-sideedge ("An editor/Write tool succeeds where Bash was refused — do not route
around, report it"). The candidate adds the coordinator/guard-designer side:
the escape happens innocently with no log, and the mitigations (brief wording +
pre-merge
git status) were absent → merged intoworktree-isolated-workers(the brief-authoring page), related +=control-signals-vs-primary-artifacts so both sides link. Flagged, not a
conflict: the two pages now cover the same mechanism from opposite roles.
(One-way link only — control-signals'
related:line is concurrently edited byopen PRs knowledge: caplog getMessage() assertions; probe-path vs operation-path auth divergence (2 verified, 1 folded into #80) #101/knowledge: an unattended worker's in-band question reaches nobody (1 ingested, 2 dropped as in-flight dups of #47/#51) #64/knowledge: bats/bash-3.2 assertion trap + 4 orchestration/guard edges; 5 duplicates retired #47; adding a third edit there would guarantee a conflict.)
Open-PR check
Listed 30 open
knowledge/*heads (#47–#104). Diffed the ones touchingoverlapping pages against merge-base (three-dot semantics; two-dot lists were
polluted by main-side #108 additions): #103 (shared-run-state — related-link only),
#101 (control-signals — related-link only), #92 (worktree-isolated-workers —
gitignored-path edge), #80 (pane-delivery — pasted-text delivery), #79
(dispatching-after-a-completion-report), #64 (control-signals/shared-run-state/
pane-delivery — related-links + new pages), #51 (worktree-isolated-workers —
cross-worktree read edges), #47 (control-signals usage-limit edge,
worktree-isolated-workers version-dependent read escalation, guard-shape widening
edge), #86 via
gh pr diff(session-completion-gates). Fork PRs #91/#104 touchunrelated Java/DB/QA pages.
Per-candidate verdicts:
knowledge: plugin MCP server registration + retiring a replaced auth gate (2 ingested, 1 merged into pane-delivery-confirmation, 1 corrected) #80's pane-delivery change is about paste-submission confirmation, a different
failure in the same re-delivery flow → new.
different guardrail edges (gitignored paths, cross-worktree reads, read
escalations); none covers the native-tool bypass or the pre-merge git-status
check → new (merge conflicts among sibling amendments are line-adjacent but
content-disjoint).
Routing decision
inserting-a-guard-before-an-existing-side-effect. The harvesteddomain: debugginghint was re-routed: debugging is scoped to diagnosing failures,while this is pre-change impact verification — exactly what change-impact holds
(precedent: call-site-enumeration, and in-flight knowledge: 9 insights — closed value table widening, guardrail read-vs-write correction, dispatch binding taxonomy #51/knowledge: bound a new rejection rule with a corpus sweep before writing it (1 ingested, 2 dropped as in-flight dups of #51) #58 additions to the same
category). backend/index.md gained the row.
page owns coordination through shared files). infrastructure/index.md load-when
extended with the reset-during-re-delivery trigger.
(merge; the page owns brief authoring + guardrail direction). The harvested
domain: securityhint was re-routed: the wiki's security domain isapplication trust boundaries; agent-guardrail semantics live in
agent-orchestration, where the sibling worker-side edge already sits.
infrastructure/index.md load-when extended with the Bash-hook-vs-native-tools
trigger. Sources gained the official hooks doc.
No new categories. log.md updated with the ingest entry.