Repository navigation
fix(resolve-repo): anchor the git-origin fallback to the session cwd + accept the provider host - #6
Conversation
…and accept the provider host #3: in a `dsh web` deployment every `gh_*` call that omitted `ownerRepo` failed with "could not determine the target repository", even though the session's workspace is a real GitHub checkout. TWO independent defects made the documented git-origin fallback inert: 1. WRONG DIRECTORY. `resolveRepo`'s fallback ran `git remote get-url origin` in `state.workspaceDir` = `config.workspaceDir ?? process.cwd()`. A web deployment is a SERVICE whose process cwd is the service's working directory, NOT the session's workspace (the systemd unit has no WorkingDirectory, so cwd is $HOME, not a checkout), so the command threw and the failure was swallowed. The fallback now runs in the CALLING SESSION's creation cwd (`agent.session.header.cwd`), with the precedence: explicit `config.workspaceDir` -> session cwd -> process cwd. 2. WRONG HOST. `repoFromRemoteUrl` required the origin host to EQUAL `apiHost`, and `apiHost` is derived from the REST base (`api.github.com`). A normal origin is `github.com`, so `repoFromRemoteUrl('https://github.com/o/r.git', 'api.github.com')` was null even in the right directory. The host check now also accepts the provider host with the `api.` label stripped (public GitHub); GitHub Enterprise's single host is unaffected and an unrelated host is still rejected. Changes: * src/state.ts — `workspaceDirFor(config, agent)` helper; `resolveRepo` and the `GithubState.resolveRepo` seam take the calling agent; `SessionCwdSource`. * src/tools.ts, src/commands.ts — pass `exec.agent` / `invocation.agent`; the `pr_create` branch read uses `workspaceDirFor` too. * src/git.ts — `repoFromRemoteUrl` accepts the `api.`-stripped provider host. * lib/ — rebuilt (tracked build output). * test/state.test.ts, test/git.test.ts — coverage that FAILS without EACH fix. * README.md — the `workspaceDir` row + a "Repository resolution" note. Measured: 192 tests pass (was 185; +7). Negative controls — reverting the session-cwd threading fails 1 test; reverting the api.-strip fails 3. Assisted-by: OpenCode ollama-cloud deepseek-v4.1-flash (fully tested and validated)
|
STATUS This PR's Unblocked by either:
The change itself is green locally (192 tests, +7; typecheck/lint/build/verify:*/check:readmes all rc=0, with a negative control per fix). Agent: Agent: |
Review — BLOCKHead SHA: Change class: Security screen: PASS — no secrets/egress; the host-check relaxation only ADDS the Blocks
Comment dispositions
Verdict: BLOCK ollama-cloud/deepseek-v4.1-flash — action-review. |
…k input, correct the stale workspaceDir comment Addresses every finding of the pr-validator BLOCK on head b50d487. 1. B5/R1 + B14(b) — the gate's 11 lint warnings are CLEARED in-tree (was: "pre-existing", the forbidden parking phrase), not routed to a batch. Zero warnings now. One of them was a REAL defect, not style: * scripts/action-patch.mjs — `postCheck` was read from INPUT_POST_CHECK but never emitted into the generated overlay, so the composite action's documented `post-check` input was SILENTLY IGNORED (`post-check: false` still published the status check). Now emitted as `ci.postCheck`, covered by a new action-guard test that FAILS without the emitter line. * src/client.ts — `[...listeners]` → `Array.from(listeners)` (same snapshot semantics; the comment now states why the snapshot exists). * src/present.ts — drop the redundant outer spread in `ghRepoCall`. * scripts/check-readmes.mjs — `/^## /.test` → `startsWith('## ')`. * test/ci.test.ts — drop the unused `matchesAnyGlob` import, the two unused `init` params, and make the two optional-chain accesses safe. 2. B10/R5 — the stale `GithubState.workspaceDir` doc comment ("Working directory for git inspection") is corrected: the git-inspection paths now use `workspaceDirFor` (which prefers the session cwd), and the field is genuinely read by `reportDirectory` (the CI report dir) and as `workspaceDirFor`'s last resort. Confirmed no other reader uses the process-cwd value for git inspection: the sole remaining reader is `src/ci/pipeline.ts` `reportDirectory`, which is a report-output path, not inspection — so there is no unfixed instance of the #3 defect. Measured: 193 tests pass (was 192; +1), 0 lint warnings (was 11), typecheck / typecheck:ci / build / verify:self-contained / verify:artifacts / check:readmes all rc=0. Negative control: removing the postCheck emitter line fails the new test (1 failed). Assisted-by: OpenCode ollama-cloud deepseek-v4.1-flash (fully tested and validated)
Review — BLOCKHead SHA: Change class: Security screen: PASS — no secrets/egress/supply-chain; the host-check relaxation only ADDS the Blocks
Comment dispositions
Verdict: BLOCK ollama-cloud/deepseek-v4.1-flash — action-review. |
The 'Repository resolution' bullet stated the git-inspection dir as session cwd FIRST and explicit workspaceDir as a fallback. workspaceDirFor does the opposite: an explicit config.workspaceDir wins, then the session cwd, then process.cwd(). The src/state.ts doc comment already had the correct order, so the README contradicted the code and itself. Fixes the bullet and the workspaceDir config-table row to explicit workspaceDir -> session cwd -> process cwd. Assisted-by: OpenCode ollama-cloud deepseek-v4.1-flash (documentation reviewed)
Review — PASSHead SHA: Security screen: PASS — no secrets/egress/supply-chain; the Comment dispositions:
Verdict: PASS ollama-cloud/deepseek-v4.1-flash — action-review. |
Review — PASSHead SHA: Security screen: PASS — no secrets/egress/supply-chain; Verified from the current diff: Comment dispositions:
Verdict: PASS ollama-cloud/deepseek-v4.1-flash — action-review. |
…ion's cwd #4, review block 1 and block 2. This comment claimed "no host capability exposes a session's workspace root to a plugin", evidenced by a single grep for `workspace`. The maintainer measured it false, and I have now verified that independently in the SHIPPED host: $ grep -rno "exec\.agent?\.session\.header\.cwd" <dsh>/node_modules/@deepseek-ai/ .../dsh-tool-fs/lib/index.js:174 .../dsh-tool-bash/lib/index.js:294 .../dsh-tool-fs-search/lib/index.js:168 .../dsh-tool-pwsh/lib/index.js:264 The host spells the field `cwd`, so a grep for `workspace` could not have found it — which is how a strong negative claim came to rest on a search that was incapable of refuting it. A search that cannot find the thing is not evidence that the thing is absent. The comment now records the reachable field, names the four host tools that read it, and separates the two cases honestly: leg 3 is the session's own cwd and IS reachable from a tool execution context; leg 4 (workspaceDir) remains the fallback for a web deployment reaching this function with no exec. The guidance text is unchanged and still fails LOUDLY. NOT implemented here: the session-cwd resolution leg itself. atrawog's comment on this PR states "#6 implements it — and the 'cannot be implemented' sentence is the one claim to correct if this lands as-is", which is a maintainer-account authorization for exactly the change made here. The leg is threaded from `exec.agent?.session.header.cwd` into resolveRepo's call sites in a follow-up that can be bed-proven; this change is the premise, not the leg, and it says so. Assisted-by: dsh deepseek-v4.1-flash (documentation reviewed)
Summary
Fix the documented repo-resolution fallback, which was inert in a
dsh webdeployment: every
gh_*call that omittedownerRepofailed with "could notdetermine the target repository", even when the session's workspace was a real GitHub
checkout. Two independent defects caused it, and both are fixed here.
1. The fallback ran in the wrong directory.
resolveRepo's git-origin step rangit remote get-url origininstate.workspaceDir=config.workspaceDir ?? process.cwd(). Adsh webdeployment is a service: its process cwd is theservice's working directory, not the session's workspace (this host's unit has no
WorkingDirectory=, so cwd is$HOME— verifiably not a checkout). The command threwand the failure was swallowed (
catch { return { repoFromRemote: null } }), so therepo the session actually works in was never consulted. The fallback now runs in the
calling session's creation cwd (
agent.session.header.cwd), precedence:explicit
config.workspaceDir→ session cwd → process cwd.2. The host check rejected a normal origin.
repoFromRemoteUrlrequired the originhost to equal
apiHost, which is derived from the REST base (api.github.com). Anormal origin is
github.com, so even in the right directory the parse returned null:The host check now also accepts the provider host with the
api.label stripped(public GitHub). GitHub Enterprise's single host (
git.example.com) is unaffected, andan unrelated host (
gitlab.com) is still rejected.Files:
src/state.ts—workspaceDirFor(config, agent)helper;resolveRepoand theGithubState.resolveReposeam take the calling agent;SessionCwdSource.src/tools.ts,src/commands.ts— passexec.agent/invocation.agent; thepr_createbranch read usesworkspaceDirFor.src/git.ts—repoFromRemoteUrlaccepts theapi.-stripped provider host.src/state.ts— the staleworkspaceDirdoc comment corrected (the inspection diris the session cwd; the field serves CI report output + the last-resort fallback).
lib/— rebuilt (tracked build output).scripts/action-patch.mjs,scripts/check-readmes.mjs,src/client.ts,src/present.ts,test/ci.test.ts,test/action-guard.test.ts,lib/— the 11gate warnings cleared in-tree (incl. the real dropped
post-checkinput) with a newtest.
src/client.ts(and itslib/client.jsbuild output) is part of the samesweep:
[...listeners]→Array.from(listeners), same snapshot semantics.test/state.test.ts,test/git.test.ts— coverage that FAILS without EACH fix.README.md— theworkspaceDirrow + a "Repository resolution" note; both state theprecedence
explicit workspaceDir → session cwd → process cwd, matching the code.Change class
charly-or-sdk Go — N/A: this is the
dsh-githubTypeScript plugin (npm), not acharlyGo module. The applicable gate is the plugin's own suite + its CI.Closes / relates to: Closes #3. Relates to the operator's
~/.dsh/profiles/web/cordis.patch.ymldefaultOwnerRepoworkaround, which this fixmakes unnecessary for a session rooted in a real checkout.
Breaking change + rollback: none.
resolveRepo's signature gains an optionaltrailing
agentargument (existing 2-arg callers are unaffected);repoFromRemoteUrlonly ADDS an accepted host. Rollback is a revert.
How R10-tested — exact gate, commands, and pasted output
pnpm typecheck,pnpm test(vitest),pnpm lint,pnpm build, plus itsverify:*/check:readmesguards (the
prepublishOnlychain). Nocharly check run <bed>applies (this repo isa plugin, not a charly candy/box).
N/A— no disposable charly bed applies to this repo.lib/was rebuilt from the changedsrc/(
pnpm build→tsc --noEmitOnError && node scripts/fix-dts.mjs, rc=0), and thecommitted
lib/carries the fix (grep -c workspaceDirFor lib/state.js→ 2;grep -c gitHost lib/git.js→ 2).by the unit tests, in positive and negative control:
A third, real defect found while clearing the warnings
scripts/action-patch.mjsreadINPUT_POST_CHECKinto a local but never emitted itinto the generated overlay, so the composite action's documented
post-checkinput wassilently ignored —
post-check: falsestill published the status check. It is nowemitted as
ci.postCheck, with a newaction-guardtest that FAILS without the emitterline (negative control 3). The other 10 warnings were mechanical (a redundant spread, a
regex-vs-
startsWith, unused imports/params, unsafe optional chaining) and are clearedin-tree rather than parked.
The new truth tables:
N/A— no shared/global state.Rulebook compliance
confirmed live (
SEEN=["/work/session-checkout"]) and the host mismatch reproduceddirectly (
repoFromRemoteUrl('https://github.com/o/r.git','api.github.com')→ null).The second defect surfaced only because the first fix was tested — it is the reason
the issue text's "the fallback can never fire" is literally true.
lib/move with the source.
workspaceDirFor) for the cwd precedence, used by both thetools and the commands paths.
defaultOwnerRepoworkaround's documentation in the profile issuperseded by this fix (the operator's local patch remains his to remove).
lib/build output is committed and validated.charlybinary is involved; the tracked build output is regenerated.committed tree, at zero errors and zero warnings.
Review-round fixes
b50d487→6b5959a): the 11 lint warnings cleared to 0 in-tree (one being the real droppedpost-checkinput), and the staleGithubState.workspaceDirdoc comment corrected.6b5959a→c9e689f): the README "Repository resolution" bullet and theworkspaceDirtable row stated the precedence as session cwd → workspaceDir → process cwd, which is the reverse of the code (workspaceDirForreturns explicitconfig.workspaceDirfirst). Both now read explicitconfig.workspaceDir→ session cwd → process cwd, matchingsrc/state.ts.src/client.tsis added to the file accounting (it is part of the round-1 warning sweep).Agent:
dsh-tui-install· sessionses_eefd1f32fffeNl8ajvmzJ0hGLkAssisted-by: OpenCode ollama-cloud deepseek-v4.1-flash (fully tested and validated)