Land the server-side half of the git-derived host-repo binding (#348) - #349
Merged
Conversation
…space prose Four harness strings still told the caller the host repo comes from the user or the workspace AGENTS.md -- the provenance #345 demotes to a fallback. Each now names version-control::resolve-host-repo as the source and states the two conditions under which prose applies at all: a workspace that is not a git repo, or a host with no origin remote. The urgent one is workflow-tools.ts, inside the discover return. It is the first surface an orchestrator reads, and it sits directly above the rewritten bootstrap body -- so it contradicted the new contract before any session existed, re-inviting the silent misbinding #345 exists to remove. - src/tools/workflow-tools.ts -- the discover repo_binding line - src/tools/resource-tools.ts -- the start_session tool description - src/utils/session/scope.ts -- the multi-root "repo is required" error - docs/ide-setup.md -- the same claim on a user-facing surface, beyond the three the issue enumerates but the same defect Closes the harness-strings half of #348 (findings register MH-4). The submodule bump and walk re-baseline stay open: #345 is unmerged, and origin/workflows is still f84fe02 -- byte-identical to the pinned corpusSha -- so there is no commit to bump to yet. Refs #348, #345
…walk Moves the workflows submodule pointer from f84fe02 to bd9c03d -- the merge commit of #345, 45 files -- and refreshes everything that pointer invalidates, in one commit, as tests/e2e/__snapshots__/corpus-sha.json and scripts/stamp-corpus-baseline.ts both require. Walk re-baseline (vitest run -u, then baseline:stamp): - corpus-sha.json f84fe02 -> bd9c03d - snapshot.test.ts.snap: one new step, derive-host-repo, before resolve-repo-root in all six policies. That is #345 splitting the old resolve-repo-root in two -- derive-host-repo binds the shared version-control::resolve-host-repo, then resolve-repo-root assembles component identity (findings register WC-1). Nothing else drifted. Two further failures the pointer move surfaced, both corpus-driven: - tests/reference-delivery.test.ts used work-package as its negative case for the inherited activity_rules block, on the premise that work-package declared only orchestrator-scoped rules.workflow. #345 adds a rules.activity bucket to work-package, so start-work-package now carries that block and it collapses to an unchanged marker on a byte-identical refetch -- the server behaving correctly, and what the dedicated case below it already asserts. Assertion and stale comment updated. - binding-fidelity gained one untriaged finding: {target_path} at substrate-node-security-audit/techniques/write-report.md:90 has no producer. #345 renames the output target_path -> component_path on both detect-repo-type and select-target-component, which were the corpus's only producers of that name; the audit workflow declares no target_path variable and binds neither technique, so its read had been resolving against those unrelated meta declarations all along. The rename removed the cover, it did not change how that run behaves. Verified new by running the guard at both shas: 0 untriaged at f84fe02, 1 at bd9c03d. Triaged fix-later / undeclared-seed, which describes this state exactly -- the value is not state, so the agent improvises it at the point of use. The real repair is to declare and seed it in the corpus, which is a change on the workflows lineage, not on this branch. Verification: 17/17 guards pass; 742 tests pass. The one red test, session-crypto EACCES-when-not-writable, fails identically on unmodified main under the same sandbox (chmod does not block writes for that uid) and is unrelated to this change. Closes #348
16 tasks
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.
Closes #348. Both obligations #345 left in the server tree, which was outside that branch's edit surface.
1. Harness strings — point the repo binding at the derivation
The three strings the issue enumerates still told the caller the host repo comes from the user or the workspace
AGENTS.md— the provenance #345 demotes to a fallback. Each now namesversion-control::resolve-host-repoand states the two conditions under which prose applies at all: a workspace that is not a git repo, or a host with no origin remote.src/tools/workflow-tools.ts:326— inside thediscoverreturn. The urgent one: it is the first surface an orchestrator reads and sits directly above the rewritten bootstrap body, so it contradicted the new contract before any session existed. Therepo_binding: requiredprefix is preserved —tests/multi-root-bootstrap.test.ts:68asserts it.src/tools/resource-tools.ts:106— thestart_sessiontool description.src/utils/session/scope.ts:238-241— the multi-root "repo is required" error.Also fixed a fourth surface the issue does not list:
docs/ide-setup.md:52carried the identical claim on a user-facing page. Same defect, so leaving it would have left the contradiction standing. Flagging it as beyond the three checkboxes.2. Submodule bump and walk re-baseline
Pointer
f84fe02b→bd9c03df(#345's merge commit, 45 files), with everything that pointer invalidates refreshed in the same commit, ascorpus-sha.jsonandscripts/stamp-corpus-baseline.tsboth require.corpus-sha.jsonstamped to the new shasnapshot.test.ts.snap— one new step,derive-host-repo, beforeresolve-repo-rootin all six policies. That is feat(meta): derive the host-repo binding from git instead of workspace prose #345 splitting the old step in two (findings register WC-1). Nothing else drifted.The walk was confirmed red before stamping: the stamp guard fired with its named cause plus all six policy snapshots — exactly the failure the issue predicted.
Two further failures the pointer move surfaced
Neither is in the issue; both are corpus-driven, and both had to be fixed for CI to be green at the new pointer.
tests/reference-delivery.test.tsusedwork-packageas its negative case for the inheritedactivity_rulesblock, premised on it declaring only orchestrator-scopedrules.workflow. #345 adds arules.activitybucket towork-package, sostart-work-packagenow carries that block and collapses it to an unchanged marker on a byte-identical refetch — the server behaving correctly, and what the dedicated case directly below it already asserts. Assertion and stale comment updated.binding-fidelity gained one untriaged finding:
{target_path}atsubstrate-node-security-audit/techniques/write-report.md:90has no producer. Worth being precise, because this is not a behavioural regression. #345 renames the outputtarget_path→component_pathon bothdetect-repo-typeandselect-target-component, the corpus's only producers of that name. The audit workflow declares notarget_pathvariable and binds neither technique, so its read had been resolving against those unrelatedmetadeclarations all along. The rename removed the cover; it did not change how that run behaves. Verified new by running the guard at both shas — 0 untriaged atf84fe02b, 1 atbd9c03df.Triaged
fix-later/undeclared-seed, which describes this state verbatim ("the value is simply not state", so the agent improvises it at the point of use), per CLAUDE.md's direction to classify new findings rather than suppress them. The real repair is to declare and seed the value in the corpus — a change on theworkflowslineage, not this branch — so it stays open as counted debt rather than silenced.Verification
tsc --noEmitcleanscripts/check-all.ts)One red test remains:
session-crypto"surfaces actionable EACCES when dir is not writable". It fails identically on unmodifiedmainunder the same sandbox (chmoddoes not block writes for that uid). Pre-existing and unrelated to this change.Not included
The
start_sessionbind-time precondition — rejecting arepowhose mapped directory is not a git work tree — is left untouched. The issue explicitly scopes it out as a separate change.🤖 Generated with Claude Code