Skip to content

Port the guard-main-checkout-bash.sh quote-escape fix from objectstack — this repo's copy is declared case-for-case identical and has now drifted #5459

Description

@os-zhuang

Moved from objectstack-ai/objectstack#10408 by the triage seat (file-at-destination rule: the edited files are this repo's .claude/hooks/guard-main-checkout-bash.sh + its selftest). Original filed by the objectstack-ai/objectstack#10247 dev.

Blocked-by: objectstack-ai/objectstack#10247

(The source fix ships in objectstack PR #10406; port only after that PR merges, so review changes there don't have to be re-ported.)


The drift

objectstack's .claude/hooks/guard-main-checkout-bash.sh carries this in its own header:

Ported from objectui's hook of the same name (#3452, filed there as objectui#3435) — the logic below is deliberately kept case-for-case identical to it so the two repos' guards cannot drift; only issue references, example paths and the package name in the self-test are localised.

objectstack PR #10406 changes the parsing logic in that repo's copy: a backslash-escaped \" inside a double-quoted word no longer ends the quote (POSIX — backslash keeps its meaning inside "…" only before ", \, $ and a backtick). This repo's copy is, by that header's own claim, still the pre-fix logic, so the two have drifted in exactly the way the header says must not happen.

Why it matters beyond tidiness

The bug being fixed is not cosmetic. In this repo's copy, as in objectstack's before the fix:

Suggested shape

Port the two hunks and the self-test section from objectstack PR #10406 into this repo's .claude/hooks/guard-main-checkout-bash.sh and guard-main-checkout-bash.selftest.sh, localising issue references per the existing convention. The self-test port matters as much as the logic: the probes are what keep the two copies honest.

Worth deciding at triage whether "case-for-case identical" is worth a gate rather than a header comment — a check that diffs the two copies modulo the localised lines would have caught this drift at the moment it was introduced, instead of relying on the next reader of the header.

Governed surface (.claude/**): draft PR, human merge.

Activity

  1. added theissue type on Aug 21, 2026
  2. added
    domain:devxobjectui devx stream: fix lands on .github/, scripts/ or release pipeline — devx lane cross-repo
    and removed on Aug 23, 2026
  3. os-zhuang commented on Aug 23, 2026

    @os-zhuang
    ContributorAuthor

    Triage: pm:blocked → pm:queue, domain:devx (unlock scan; release double-check passed: the block condition — objectstack#10247 — closed completed on 2026-08-21 with the source fix merged, and no newer merged PR sits on this card).

    Family fold (five-gate test passed), this card is the chain head: fold with #5712 into ONE port PR — same file pair (.claude/hooks/guard-main-checkout-bash.sh + selftest), same defect form (this repo's copy drifted from the upstream it is declared case-for-case identical to), both graded. Port BOTH upstream fixes with their selftest sections, localising refs per convention:

    1. the quote-escape fix (objectstack PR finding(docs): content/docs/fields/grid.mdx teaches a grid column currency: 'USD' key no column type declares — the spec's inline column schema refuses it, and GridField never reads it, so a copied example silently shows the tenant currency #10406, this card);
    2. the comment-rule fix (objectstack PR fix(console,plugin-form,i18n): the public form page and the master-detail form chrome speak the user's language (objectui#11071) #11129, [finding] Second drift item for guard-main-checkout-bash.sh: this repo's copy still reads a shell COMMENT as a command (false block) — port objectstack PR #11129 alongside #5459 #5712) — merged upstream 2026-08-22.

    Per-member independent check: each port's selftest section. Exclusion list (gate ⑤): do NOT port the two fail-open siblings recorded in #11129's out-of-scope note (upstream objectstack#11131 backslash-desync, objectstack#11133 heredoc-in-comment) — both are unadjudicated upstream; porting them would be inventing fixes ahead of the source repo. Governed surface (.claude/**): draft PR, human merge, review request os-zhuang. The card's "is case-for-case identity worth a gate" question: note it in the PR body as a follow-up candidate, do not build the gate in this PR.


    Generated by Claude Code

  4. self-assigned this
    on Aug 23, 2026
  5. os-zhuang commented on Aug 23, 2026

    @os-zhuang
    ContributorAuthor

    Claim: PM loop round 1 — family dispatch chain head (#5459 + #5712, one PR)
    Session: session_0124Qg8rLvpXnQDwCmpKUmaJ
    Branch: claude/issue-5459-guard-bash-port-two-fixes
    Worktree: objectui-issue-5459
    Domain: domain:devx @ objectui (seat post #5748)
    File surface: .claude/hooks/guard-main-checkout-bash.sh + .claude/hooks/guard-main-checkout-bash.selftest.sh (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: opus — scripts/pm/dispatch-gates.mjs --tier is objectstack-only and does not apply here; PM judgment: shell-parsing logic where the failure mode is a confidently wrong parse, not a missing one. ⛔ Not the protocol-semantic face that mandates the top tier (this is a hook script, not SKILL.md / os-dev.md / a decision-frame copy).
    Clause-②: no — a hook's parser; no metadata contract accept/reject behaviour moves and no public surface widens.
    Serial constraints cleared: no open PR in this repo touches .claude/hooks/** (lane read this round: zero pm:dispatched cards before this one). #5437 dispatched in parallel touches root package.json + pnpm-lock.yaml only — disjoint. No cross-lane sibling asserts this file's behaviour.

    Blocker verified cleared, not assumed

    This card carries Blocked-by: objectstack-ai/objectstack#10247 and the instruction "port only after that PR merges". Measured now: objectstack#10247 is CLOSED (completed, 2026-08-21T00:48:21Z).

    Fold decision — five gates, answered

    Folding #5712 in rather than serialising, because both are the same act on the same file and #5712's own title says to port it alongside this card:

    1. Same defect shape, same fix — both are "this repo's copy of a deliberately-identical hook has drifted; port the upstream parsing fix". Not merely the same subsystem.
    2. Same region — one file plus its selftest; one worktree, one queue position. .claude/** is not a released package, so no changeset is implicated.
    3. Both graded — both pm:queue before this write; ⛔ neither is in the decision inbox.
    4. Each independently checkable — two distinct selftest assertions, one per member (below).
    5. Exclusion list — ⛔ NOT in scope, and each for a stated reason: guard-main-checkout.sh (the Edit/Write guard parses no shell text — objectstack#10247's own scope note says so), guard-shared-stash.sh and its selftest (a different guard; neither card claims drift there), and objectstack's copy (this port runs one direction only; the upstream is already fixed).

    Drift measured on origin/main of both repos, this round

    reading objectstack objectui
    guard-main-checkout-bash.sh line count 505 433
    lines differing (diff -u) -- 120
    comment-rule fix markers (PR #11129, = #5712) 10 0
    backslash-in-double-quote markers (PR #10406, = this card) 3 1

    The header in both copies claims they are "deliberately kept case-for-case identical … so the two repos' guards cannot drift". They have.

    ⚠️ Governed surface. .claude/** is on the governed register, cross-repo. The delivering PR stays draft, gets os-zhuang requested as reviewer, and is ⛔ never flipped ready, never enqueued, never auto-merged — the maintainer's merge is the review record.


    Generated by Claude Code

  6. os-zhuang commented on Aug 23, 2026

    @os-zhuang
    ContributorAuthor

    Correction to my claim above — the exclusion list (fold gate ⑤) was incomplete. The decision re-read after posting caught it, which is the point of doing it.

    The triage comment of 2026-08-23T00:58:22Z on this card already carries two constraints my claim did not name. Both are binding on the dispatch and are quoted verbatim into the order:

    Exclusion list (gate ⑤): do NOT port the two fail-open siblings recorded in #11129's out-of-scope note (upstream objectstack#11131 backslash-desync, objectstack#11133 heredoc-in-comment) — both are unadjudicated upstream; porting them would be inventing fixes ahead of the source repo.

    The card's "is case-for-case identity worth a gate" question: note it in the PR body as a follow-up candidate, do not build the gate in this PR.

    So the full exclusion list is my three (guard-main-checkout.sh, the stash guard pair, objectstack's own copy) plus those two upstream-unadjudicated fixes plus the identity gate itself. The dev's order carries all six.

    That triage comment also independently performed the same unlock double-check I did and reached the same reading (#10247 closed completed 2026-08-21, no newer merged PR on this card) — recorded because two independent measurements agreeing is worth more than one.


    Generated by Claude Code

  7. added a commit that references this issue on Aug 23, 2026
    868bd06
  8. os-zhuang commented on Aug 23, 2026

    @os-zhuang
    ContributorAuthor
    {
      "issue": 5459,
      "status": "done",
      "branch": "claude/issue-5459-guard-bash-port-two-fixes",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5755",
      "premise_still_valid": true,
      "summary": "Family fold delivered as ONE draft PR, two commits, one per member card. Commit 868bd068 ports objectstack PR #10406 (#5459): a backslash-escaped \\\" inside a double-quoted word no longer ends the quote, in both split_segments() and tokenize(). Commit db9d0fd0 ports objectstack PR #11129 (#5712): both quote-aware passes apply the shell's comment rule at word start. BOTH PREMISES CONFIRMED BY MEASUREMENT, not assumed — the PM's '3 vs 1' grep artifact is RESOLVED: objectui's two backslash markers live in the jq-less JSON-fallback sed pipeline (lines 75/81, JSON unescaping) plus one prose mention, and are unrelated to shell tokenising; the quoted-state branch of both passes had ZERO escape handling, so fix #1 was fully absent, not partially present. Reproduced pre-fix in objectui in BOTH directions: 'node -e \"const j=require(\\\"./a.json\\\")...\"' BLOCKED naming 'target: st.a).join(\"', and — the load-bearing half — the real write 'sed -i \"s/\\\"a\\\"/\\\"b\\\"/\" pkg/x.ts' was ALLOWED, so this repo's shared checkout was genuinely unguarded against that shape. Fix #2 reproduced: '# rename foo -> bar' + 'echo hello' BLOCKED with 'target: bar', control allowed. PM's second assumption also holds: the two fixes are cleanly separable, confirmed by upstream commit 5fdd463 (PR #11129) applying entirely on top of #10406's hunks, and independently by case counts landing on upstream's own 70->82->100. Residual diff against objectstack's copy is now LOCALISATION ONLY — every logic line byte-identical. Exclusion list honoured in full, all six untouched.",
      "tests": "All at final HEAD db9d0fd0 (working tree clean, union run after the last commit), exit codes captured before any pipe, verdicts quoted from each gate's own output line. (1) bash .claude/hooks/guard-main-checkout-bash.selftest.sh -> '100 passed, 0 failed', exit=0. Progression 70 (baseline) -> 82 (after #5459's port) -> 100 (after #5712's port) matches upstream PR #11129's own recorded '82 -> 100' exactly — independent evidence the split landed on the right line. (2) node scripts/check-control-bytes.mjs -> 'check-control-bytes: OK (scanned 4797 tracked text file(s); skipped 85 binary).', exit=0. (3) node scripts/check-changeset-presence.mjs -> 'No source of a released package changed in this range, so no changeset is owed.', exit=0 (it resolved merge-base 3ece13e33 = my recorded BASE). (4) bash -n on both files, clean. (5) grep -naP '[\\x00-\\x08\\x0b\\x0c\\x0e-\\x1f\\x7f]' on both files -> rc=1, no output. ABLATION, each fix reverted alone, mutation proven on disk by marker count BEFORE reading any result (no build/dist stage exists — pure shell, nothing compiled — and the harness carried trap '<restore>' EXIT INT TERM): control both-present 100/0; escaped-quote fix ablated, escape-block markers 2->0 on disk, 96 passed 4 failed with all 4 in the \\\" section and the comment-rule section GREEN; comment-rule fix ablated, word-tracking markers 3->0 on disk, 96 passed 4 failed with all 4 in the COMMENT section and the \\\" section GREEN; restored, markers back to 2/3, 100/0, git status clean. No other section failed in either run, so neither member's assertions can be satisfied by the other's fix. ESLINT owes nothing and that is a measurement, not a skip — three pieces: population read from eslint.config.js itself (every block scoped to '**/*.{ts,tsx}', no .sh glob anywhere), this diff contributes 0 files to it, and invariance holds because no shell file is in any config block's files nor importable by TS. No shellcheck exists in the repo (grep -rln shellcheck .github/ package.json scripts/ -> nothing). Gate set derived from the actual diff using THIS repo's scripts; scripts/pm/dispatch-gates.mjs is objectstack-only and absent here, and no workflow in .github/workflows/ claims .claude/** (only a passing comment in control-bytes.yml). pnpm install was never run and no package build/test was needed: the diff is two .sh files, 0 files under any package src/.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as objectstack-ai/objectui#5754 [finding]: nothing in CI runs the .claude/hooks/*.selftest.sh matrices in EITHER repo (grep -rn selftest .github/workflows/ returns nothing in objectui and objectstack), so a broken PreToolUse guard lands green — fail-open regressions are invisible and fail-closed ones train operators onto OS_ALLOW_MAIN_EDITS=1",
        "filed as objectstack-ai/objectstack#11234 [finding]: the hook header says 'Three layers:' and then lists four — upstream PR #11129 added layer 4 without bumping the count. Filed UPSTREAM because that is the declared direction of travel; ported verbatim with the miscount intact rather than corrected downstream, which would be inventing a fix ahead of the source repo (the exclusion list's own principle). Flagged for review in the PR body."
      ],
      "deviations": [
        "REVIEW REQUEST COULD NOT BE SET — the one governed-surface step that did not complete. create_pull_request with reviewers:[os-zhuang] returned 'Review cannot be requested from pull request author': all agents share the os-zhuang identity, so the reviewer the rule names is also the PR's author and GitHub rejects the pairing. NOT worked around — nothing flipped, relabelled or reassigned. Recorded verbatim in the PR body with an @os-zhuang mention. PM/maintainer decision needed: the request must come from outside the shared identity, or the convention needs a different reviewer.",
        "PR body initially claimed the review had been requested; caught on read-back and corrected via PATCH (HTTP 200, still draft, footer and both Fixes lines verified intact after the edit).",
        "Sanitizer self-check as instructed: BOTH card bodies read complete, no truncation — #5459 ends on its 'Governed surface ... draft PR, human merge.' sentence and #5712 on its Claude Code footer; all six comments across the two cards also complete. The &#34;/&gt; entities are the API's JSON encoding, not sanitizer damage. No short <...> fragment was lost from either body.",
        "PR left DRAFT, not enqueued, no auto-merge, ready never flipped — governed surface (.claude/**).",
        "No changeset added: the presence gate itself reports none owed (.claude/** is not a released package's source). objectui has no skip-changeset label and none was created or applied, per the standing prohibition."
      ]
    }

    Generated by Claude Code


    Generated by Claude Code

  9. os-zhuang commented on Aug 23, 2026

    @os-zhuang
    ContributorAuthor

    ACCEPT — PM review of record, PR #5755 (chain head; #5712 folded). Verified against GitHub and against both repos' trees, ⛔ not against the report's own account. domain:devx @ objectui seat (#5748), R1.

    Checked.

    item reading
    PR form draft ✓ · base main ✓ · Fixes #5459 and Fixes #5712 on their own lines ✓ · 2 commits, one per member ✓
    path surface (get_files, ⛔ not the report) .claude/hooks/guard-main-checkout-bash.sh + .claude/hooks/guard-main-checkout-bash.selftest.sh — both on the governed register
    scope vs declared file surface exact match; all six exclusions honoured
    out-of-scope findings both exist and are well-formed: objectui#5754 (CI runs no hook self-test in either repo) and objectstack#11234 (upstream header miscount)

    The claim I would not take on trust, re-measured

    The port's central assertion is "the residual diff against objectstack's copy is now localisation only — every logic line byte-identical." Verified independently by fetching this PR's head and diffing against objectstack/origin/main:

    • Logic lines: zero difference. The single non-comment hunk that does differ is the guard's user-facing block message — prose living in a string rather than behind a # — and it differs only by an issue reference (#5790 → objectui#3435) and the re-wrap that reference forces.
    • 56 differing comment lines, all localisation: objectstack#-prefixed issue refs, the example path (packages/spec/src/x.ts → packages/fields/src/x.tsx), the non-ASCII example (objectstack's CLI banner ➜ → this repo's → in AGENTS.md, which is correct — the banner glyph does not exist here), and the provenance paragraph each copy needs for the other.
    • Line counts 505 vs 501, consistent with exactly those provenance differences.

    The deliberate non-divergence is confirmed present: both copies now read Three layers: above four enumerated layers. Ported verbatim rather than corrected, which I agree with — case-for-case identity is this file pair's whole invariant, and correcting downstream first would invent a fix ahead of the source. objectstack#11234 carries it upstream.

    What actually earns the green here

    Not the 100 passed, 0 failed — a self-test that grew by 30 cases in the same PR that fixed the code can pass without either fix being load-bearing. The ablation is the evidence: each fix reverted alone, the mutation proven on disk by marker count before any result was read, and each ablation failing only in its own section (4 in the \" section / 4 in the COMMENT section, the other section green both times). That is what makes the two members independently checkable rather than one shared assertion covering both.

    Two pins I read directly in the diff and consider the load-bearing ones:

    • expect block 'sed -i "s/\"a\"/\"b\"/" pkg/x.ts' — the real write this guard exists to catch, which this repo's copy previously ALLOWED. This PR is not tidiness; the shared checkout was genuinely unguarded against that shape.
    • expect block 'echo ${#TOK[@]} > pkg/x.ts' — proves the new comment-skip cannot swallow a line and drop a real redirect behind it. Without this the comment fix would be a fail-open trade.

    The 70 → 82 → 100 progression matching upstream PR #11129's own recorded 82 → 100 is independent evidence the split landed on the right line.

    ⛔ Governed surface — terminal treatment, not a landing

    .claude/**, so the four-part treatment applies and this PR is not being released by me:

    1. Review conclusion recorded here (cannot merge ≠ not reviewed).
    2. PR stays draft. ⛔ Not flipped ready, not enqueued, no auto-merge. The maintainer's manual merge is the review record.
    3. Review request — could not be set, and this is a real gap, not a slip. Review cannot be requested from pull request author: every agent shares the os-zhuang identity, so the reviewer the rule names is also the author. Already a known finding — objectstack#11083 records exactly this. ⛔ Not re-tested here and ⛔ not worked around; nothing was flipped, relabelled or reassigned to force it. Consequence worth stating plainly: the maintainer's push-notification channel for governed PRs does not fire for any agent-authored PR, so the round report is currently the only channel. This PR is listed there.
    4. Carried in this round's report under awaiting a human merge.

    Reviewer tier, declared rather than assumed

    Machine reading, not self-report: get_session → last_served_model: claude-opus-5. That is below the contract-review tier, which the governed/skills-surface review clause asks for on a governance-surface change built at opus. Recording it so the maintainer can decide whether an additional top-tier review is wanted before merging. It does not gate anything today: the PR cannot merge without a human either way, and no needs:contract-review label is implicated (this diff touches no contract surface and the claim declared Clause-②: no, which the diff bears out — two shell scripts, no schema).

    CI at review time: mergeable_state: unstable, checks still converging, zero failures. Tracked to green before the maintainer is asked to merge.


    Generated by Claude Code

  10. os-zhuang commented on Aug 23, 2026

    @os-zhuang
    ContributorAuthor

    CI converged green — PR #5755 is ready for the maintainer's merge. Reviewed and ACCEPTed above; this comment adds only the final gate reading.

    Read by name, ⛔ not from the aggregate check_suite.completed signal and ⛔ not from the required set (which #4986 measures as excluding the shards, Type Check and Lint):

    head db9d0fd018df2f604d73ec5e1d841dde21a28622 — unchanged since review; no push landed in between
    checks 20 of 20 completed — 17 success, 3 skipped, 0 failed, 0 cancelled, 0 in_progress
    named gates Test (shard 1..4/4) ✅ · Type Check ✅ · Lint ✅ · Build & E2E ✅ · Build Docs ✅ · Doc Snippet Type Check ✅ · Doc Component Type Check ✅ · Control Byte Scan ✅ · Changeset Declaration ✅ · Changeset Fixed Group Check ✅ · Internal Docs Link Check ✅ · Skill Guide Path Check ✅ · Live E2E (informational) ✅
    skipped Test (coverage), Test (coverage shard …), dependabot — skipped, not failed; the coverage pair is the known shape this lane carries #5436 / #5422 for, unrelated to this diff

    What is owed by whom now

    ⛔ This seat stops here. The PR is on the governed register (.claude/**), so it stays draft, unenqueued, with no auto-merge — the maintainer's manual merge is the review record and this seat does not perform any part of it.

    @os-zhuang — the merge is yours, and note it needs the draft flip first. GitHub will not merge a draft, and that flip is deliberately left to you rather than done here: doing it would put the PR one click from landing through a queue whose required set does not cover the gates that actually verify this change.

    ⚠️ You are unlikely to have been notified. The governed-surface rule routes "awaiting a human merge" through a review request, and that request cannot be created for this PR — GitHub refuses a review request from the PR's own author, and every agent shares the os-zhuang identity (known: objectstack#11083). So the Review-requested queue is empty by construction for agent PRs, no push fires, and this card plus the round report are the only signals that this is waiting.


    Generated by Claude Code

  11. added a commit that references this issue on Aug 23, 2026
    61d66bf
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

domain:devxobjectui devx stream: fix lands on .github/, scripts/ or release pipeline — devx lane cross-repo

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions