nono-here - #110
nono-here#110
Conversation
* main: (28 commits) Fixes plans location Lowers local model context size feat: Update agent-harness fix: Drops writing to PLAN and committing on Builders hard stops feat: Adds `git submodule-update` alias fix: Adjusts agents. Splits Testing and HarnessTester agents fix: Fix submodule commit chore: Archives plan chore: Updates harness-sync tag chore: Archives librarian research plan fix: Makes some necessary adjustments fix: Reverts some unneccessary changes and syncs agent-harness Adds git branch-worktree alias docs(harness): mark issue #90 plan tasks complete, code review APPROVED feat(harness): Builder hard-stop protocol — stop and escalate on walls (issue #90) Adjusts orchestrator implementation refactor: sync Librarian research artifacts to the Research Artifact Contract feat: adopt Orchestrator lifecycle from agent-harness (issue #101) docs: mark Task 3 complete feat: persist Librarian research artifacts ...
- Bound symlink-following loop in resolve_script_dir() to 40 hops; die(1, ...) on exhaustion to prevent infinite loop on symlink cycles. - Scope src/dir/target/hops as locals inside resolve_script_dir() instead of leaking as globals. - Export NONO_HERE_HOME (no current consumer; documented as forward-looking for potential future child processes) with a comment explaining the reasoning. - Add a comment on `script_dir="$(resolve_script_dir)"` warning that this assignment must stay bare (no `local`/`export`) or set -e's failure propagation breaks.
Adds HARNESSES array as extension point, NONO_HERE_HARNESS override with validation (die 8 on mismatch), non-interactive TTY guard (die 4), and select-based menu with re-prompt-on-invalid / die 10 on EOF semantics. Marks Task 3 complete in PLAN.md.
Probe templates in defined order; die 5 if none found. Die 7 when run_harness.sh or start.sh is missing or non-executable. Echo resolved workdir/template to stderr for visibility. Purely validates in place, no filesystem mutation.
Implements stale .sandbox handling in nono-here.sh: non-TTY guard, yellow ANSI warning, y/N confirmation prompt, rm -r without -f. PLAN.md reflects Task 5 completion plus reviewer amendments to Task 6.
…sk 7) Move run_harness.sh from .sandbox to workdir root; add post-condition re-checking executable bits on run_harness.sh and .sandbox/start.sh, dying with the template path if cp -R failed to preserve modes.
… (Task 8) Fix $SELF: prefix in preservation log (T8-S1); PLAN.md Task 8 marked complete with review log.
Wires the provisioning path's handover call after the Task 8 preserve/generate if/else block, reusing the existing single exec site in handover() from Task 9. Task 8b in PLAN.md marked complete.
…MAP.md nono-here bootstrap feature complete (Tasks 1-11). Archive PLAN.md to plans/2026-09-11_nono-here-bootstrap.md with an Expectation vs. Reality post-mortem appended. Add PROJECT_MAP.md scoped to the nono/ subsystem. Correct AGENTS.md's Chronicler archiving path (docs/plans/ -> plans/) to match actual convention.
There was a problem hiding this comment.
🟡 Changes recommended
Critical permission and provisioning issues, along with additional startup and compatibility defects, remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a portable nono-here.sh sandbox bootstrap flow with templates, provisioning tests, live sandbox updates, and OpenCode configuration changes.
Changes:
- Adds harness discovery, validation, provisioning, defaults generation, and handover.
- Adds portable scripts, profiles, hooks, and integration tests.
- Updates sandbox configuration and archives the implementation plan.
File summaries
| File | Changes and final findings |
|---|---|
run_harness.sh |
Loads sandbox defaults and updates argument handling. Nit (2 votes): breaks documented defaultless usage. Moderate (1 vote): retains unsafe unguarded argument and array handling. |
PROJECT_MAP.md |
Documents the sandbox subsystem and risks. No final findings. |
plans/2026-09-11_nono-here-bootstrap.md |
Archives the implementation plan and decisions. No final findings. |
PLAN.md |
Removed after plan archival. No final findings. |
opencode.jsonc |
Adds plugins and directory permissions. Moderate (1 vote): /tmp and ~/env access bypasses confirmation boundaries. |
nono/test_nono_here.sh |
Adds bootstrap integration tests. Nit (2 votes): Bash detection accepts all 3.x versions instead of requiring 3.2. |
nono/templates/default/start.sh |
Adds startup and hook logic. Critical (3 votes): hook names do not match lookup names. Moderate (3 votes): GNU-only diff option; Moderate (3 votes): unset tpl_ver fallback failure; Moderate (2 votes): unquoted BASH_SOURCE; Nit (2 votes): unclear error wording. |
nono/templates/default/run_harness.sh |
Adds portable defaults and argument handling. No final findings. |
nono/templates/default/profile.template.json |
Adds the default sandbox profile. Moderate (1 vote): grants access to ~/.gitconfig. |
nono/templates/default/hooks/before-template |
Adds the before-hook template. No final findings. |
nono/templates/default/hooks/after-template |
Adds the after-hook template. No final findings. |
nono/templates/default/.gitignore |
Ignores generated profiles. No final findings. |
nono/nono-here.sh |
Implements workspace discovery and provisioning. Critical (1 vote): unsafe handling of dangling or non-regular defaults.sh paths. Moderate (1 vote): exposes the harness before defaults generation completes. |
AGENTS.md |
Updates the archived plan location. No final findings. |
.sandbox/start.sh |
Updates the deployed startup marker. No final findings. |
.sandbox/profile.template.json |
Updates the deployed profile. No final findings. |
.sandbox/defaults.sh |
Defines deployed harness defaults. No final findings. |
.opencode/cost-guard.config.json |
Adds cost guard settings. No final findings. |
.opencode/agents/Testing.md |
Broadens Testing shell permissions. Critical (3 votes): removes approval gates for unrestricted commands. |
.opencode/agents/Builder.md |
Broadens Builder shell permissions. Critical (2 votes): removes guardrails against arbitrary shell, Git, and network operations. |
Review details
Suppressed comments (4)
nono/nono-here.sh:178
mvmakes the workspace harness visible before the next fallible write that creates or preservesdefaults.sh. If that write fails, the next invocation sees an executablerun_harness.shand takes the fast path, so it never offers the stale-sandbox repair and repeatedly fails on the missing defaults file. Generatedefaults.shbefore this move, or make the fast path validate it before handing over.
mv "$workdir/.sandbox/run_harness.sh" "$workdir/run_harness.sh"
nono/templates/default/profile.template.json:13
- The default profile grants the sandbox read access to
~/.gitconfig, which can contain credential-helper settings, embedded tokens, private repository URLs, and other host secrets. The repo's deployed profile explicitly denies this same file (.sandbox/profile.template.json:13), so the new generic template weakens the default security boundary for every freshly provisioned workspace; remove it or allow only a deliberately sanitized config.
"read_file": ["~/.gitconfig", "~/.gitignore"],
opencode.jsonc:29
- These project permissions grant the agent access to every path under
/tmpand the entire~/envtree without confirmation, overriding the globalexternal_directory: "*": "ask"boundary. That can expose or modify unrelated temporary files and personal repositories; narrow the allowlist to the exact MCP/runtime paths or retain the confirmation gate.
"external_directory": {
"/tmp": "allow",
"~/env": "allow"
}
run_harness.sh:14
- The live root wrapper now sources the generated empty
SANDBOX_COMMAND_DEFAULTS=()file, but it still has the old direct array expansion and unguarded$1checks below. Invoking this wrapper with no arguments therefore forwards a bogus empty argument (and is unsafe on Bash 3.2), unlike the fixed template that this PR provisions. Apply the same use-site guard here and use the absoluteWORKSPACEstart-script path.
source "$DEFAULTS_FILE"
SANDBOX_COMMAND="${SANDBOX_COMMAND:-}"
- Files reviewed: 19/20 changed files
- Comments generated: 10
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Review: nono-here — zero-config sandbox bootstrap
Worktree review of 7ecb263. Test run: nono/test_nono_here.sh — 28/28 pass with an exec-capable TMPDIR. (With the default /tmp on a noexec mount, 19 cases fail with misleading "not executable" messages — see N4.)
Blockers
B1. Agent permission guardrails removed, bundled silently.
.opencode/agents/Builder.md bash: "*": deny → allow, Testing.md ask → allow (commit 79afda0). This strips the approval gates that AGENTS.md's hard-stop rules rely on, for unrestricted shell/git/network. It appears deliberate, but the PR body is empty and it's mixed into a bootstrap feature. Needs explicit sign-off, or split into its own PR with rationale.
B2. start.sh no-jq fallback crashes under set -u (nono/templates/default/start.sh:42). In the no-jq branch, $tpl_ver is never assigned (only set in the jq branch), so a profile.json that differs from the template aborts with tpl_ver: unbound variable instead of printing the warning. Same line also has a broken escape ([0m, not \033[0m). Pre-existing in the deployed script, but the new template propagates it to every freshly provisioned workspace — exactly what this PR is for.
Should-fix (in this PR)
S1. defaults.sh dangling-symlink write-out (nono-here.sh, Task 8). [[ -e "$workdir/.sandbox/defaults.sh" ]] is false for a dangling symlink, so cat > follows it and writes to an arbitrary path. Every other ambiguous path in the script gets a -e || -L guard; this one doesn't. One-line fix.
S2. mv before defaults.sh generation breaks self-healing. If the cat > defaults.sh fails after the move, the next run takes the fast path (run_harness.sh present) and never reaches Task 5's stale-sandbox repair; the user gets "missing defaults file" with no recovery prompt. Generate defaults.sh before the mv.
S3. Live run_harness.sh (repo root) is now inconsistent with the template it ships. This PR modified the live script (added defaults validation, VERSION 2) but left the Task-10 bug it exists to fix: SANDBOX_COMMAND_DEFAULTS=("${...[@]:-}") (empty array → one bogus empty argv element) and unguarded "$1" — while the template (VERSION 3) has both fixed. The post-mortem discloses this as "left open by design… needs its own plan" — acceptable if the PR says so, but the PR body is empty.
Accepted as known/deferred (disclosed in post-mortem — verify before merge, fix in follow-up plans)
- Hook filename mismatch: template ships
hooks/before-template/after-template;start.shlooks uphooks/before/after. A fresh provision fires no hooks, and nothing renames them (unlikeprofile.template.json, which is auto-copied). profile.template.jsongrantsread_file: ["~/.gitconfig", …]while the deployed profile explicitly denies~/.gitconfig. The generic template weakens the security boundary for new workspaces — at minimum drop~/.gitconfigfrom the template's allowlist.
Nits
diff -u --color=alwaysis GNU-only; on macOS the BSD-diff error is swallowed by|| true, so users get a shell error instead of the diff (pre-existing, now baked into the template).- Unquoted
$BASH_SOURCE; "Couldn't find neither" wording instart.sh. - Test case 13's bash discovery accepts any 3.x rather than asserting 3.2-era (the stderr
WARNdisclosure mitigates). - Test-suite portability: fixtures hardcode
${TMPDIR:-/tmp}. On anoexec/tmp(some hardened systems), most cases fail with messages that look like product bugs. Add a preflight probe (write →chmod +x→-x), fail loud with the real cause, or relocate.
Positives
nono-here.shitself is high quality: singleexecsite, documented exit-code contract fully implemented and tested, strict validate-before-destroy ordering, no silent fallbacks anywhere.- The archived plan is exemplary — three review rounds logged, and the post-mortem honestly discloses that three task-level reviews ran without a shell tool (trust in Builder-reported evidence).
Verdict: REQUEST CHANGES — B1 (permission relaxation needs explicit approval), B2 (set -u crash shipped to all new workspaces), S1, S2. S3 and the deferred items need at least PR-body disclosure plus follow-up plans before merge.
Builder bash deny and Testing bash ask are the approval gates AGENTS.md hard-stop rules rely on; validation is delegated to the Testing agent. Also drop the ~/env external_directory allow added in 79afda0: it granted unconfirmed access to the whole personal repo tree.
nono-here.sh: - reject non-regular defaults.sh paths (-e || -L plus ! -f) so a dangling symlink from a user template cannot redirect the generated file outside .sandbox (S1) - generate/preserve defaults.sh before the run_harness.sh move so a failed generation still leaves the workspace on the provisioning path, where stale-sandbox repair is offered again (S2) start.sh (template and deployed, bodies now identical): - quote $BASH_SOURCE so workspace paths with spaces work - no-jq fallback: stop interpolating tpl_ver (unbound under set -u) and terminate the ANSI escape so startup continues on the degraded path (B2) - probe for GNU diff --color=always; BSD diff (macOS) falls back to plain output instead of the error being swallowed by || true - 'Could not find either' wording profile.template.json: drop ~/.gitconfig from read_file; the deployed profile denies it, the generic template must not weaken new workspaces. hooks: ship before/after (the names run_hook resolves) instead of before-template/after-template, which no fresh provision ever fired.
- root run_harness.sh is now byte-identical to templates/default
(VERSION 3): guarded ${1:-}, empty-array expansion guard for Bash 3.2,
absolute start.sh path (S3)
- README: .sandbox/defaults.sh is generated by nono-here.sh and required,
not optional
- PROJECT_MAP: mark high-risk areas 1 (hook mismatch) and 2 (template/
deployed drift) as closed by this PR
- test suite: case13 matches Bash 3.2 explicitly (a 3.0/3.1-only host
must still disclose degraded coverage); new preflight probe fails loud
when the fixture directory is a noexec mount instead of letting every
case fail with misleading 'not executable' messages
There was a problem hiding this comment.
All four blockers and the should-fix items are addressed; commits 52062d1, 9a1667a, d6823bb:
- B1 — reverted: Builder
bash→ deny, Testingbash→ ask. The permission relaxation was not part of the bootstrap feature and is now out of this PR's diff (the~/envallow from79afda0was removed too). - B2 — no-jq fallback fixed: no
$tpl_verinterpolation, ANSI escape terminated; verified with jq removed from PATH — the warning prints and startup continues. - S1 —
defaults.shgeneration rejects non-regular pre-existing paths (-e || -L+! -f, exit 9); verified a dangling symlink from a user template no longer writes outside.sandbox. - S2 —
defaults.shis now generated before themv, so a failed generation keeps the workspace on the provisioning path (stale-sandbox repair still offered). - S3 — live root
run_harness.shis byte-identical to the template (VERSION 3); no longer "left open by design". The PR body now discloses the remaining known items explicitly.
Also fixed along the way: hook template filenames (before/after — fresh provisions now fire both hooks), ~/.gitconfig dropped from the generic profile template's read allowlist, quoted $BASH_SOURCE, portable diff color probe, "Could not find either" wording, case13 matching Bash 3.2 explicitly, and a noexec-TMPDIR preflight probe in the test suite (fails loud with the real cause instead of 19 misleading failures).
Verification: nono/test_nono_here.sh 28/28 with an exec-capable TMPDIR; manual checks for each fix as listed in the PR body. The deferred items (non-executable hook silent skip, no script upgrade mechanism, exit codes 1/10 untested) remain disclosed in PROJECT_MAP.md and the PR body for follow-up plans.
Drop task numbers and reviewer references; each comment now states the reason for the code shape (fast-path silence, generation order, symlink guards). Also fixes a mangled comment block at the handover site.
chore: Drop project map
| @@ -0,0 +1,184 @@ | |||
| # Project Map | |||
There was a problem hiding this comment.
Drop this file (and command and mention on agents.md)
# Conflicts: # README.md
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings remain in permissions, bootstrap validation, template compatibility, and configuration.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (8)
nono/nono-here.sh:126
-ffollows symlinks, but the subsequentcp -Rpreserves them. A symlinked template script therefore passes validation and is moved as a symlink into the workspace; relative links can break after the move, while absolute links make handover execute outside the provisioned template. Reject symlinks here (or explicitly dereference them during copying).
if [[ ! -f "$template/$required" ]]; then
nono/templates/default/profile.template.json:18
- A fresh OpenCode workspace falls back to this default template, but the repository's OpenCode MCP endpoint is
http://localhost:12100/mcpand the checked-in profile explicitly permitslocalhost/port 12100. This template permits no network domain or port, so cold provisioning removes access to the configured web MCP. Provide an OpenCode-specific template or carry the required localhost permissions into the selected template.
"network": {
"allow_domain": []
},
nono/templates/default/run_harness.sh:6
- The template makes the same breaking rename from
CLOUD_DEFAULTS_FILEtoDEFAULTS_FILE, so newly provisioned workspaces will also ignore existing callers that set the old override. Preserve it as a fallback or document the migration, and keep the deployed copy in sync.
DEFAULTS_FILE="${DEFAULTS_FILE:-$WORKSPACE/.sandbox/defaults.sh}"
nono/templates/default/start.sh:2
- The deployed
.sandbox/start.shhas a# VERSION 3marker, but the template copied bynono-here.shdoes not. Because this PR claims the template and deployed runtime are synchronized, fresh provisions will already differ byte-for-byte and future drift checks become misleading; keep the marker in both copies (or remove it from both).
set -euo pipefail
plans/2026-09-11_nono-here-bootstrap.md:41
- This settled-order row still documents
move → defaults, but the implementation and the S2 fix intentionally generatedefaults.shbefore movingrun_harness.sh. Leaving this row unchanged makes the plan contradict the safety invariant it is meant to record; change the order tocopy → defaults → move → exec.
| Q15 | Strict order: workdir → shortcut check → prompt → **template resolution + validation** → stale-`.sandbox` prompt → `rm -r` → copy → move → defaults → exec. Nothing is ever deleted before a valid replacement template has been located and validated. |
plans/2026-09-11_nono-here-bootstrap.md:60
- The exit-code map says code 9 is only for a non-regular workspace
run_harness.sh, but the implementation also returns 9 for a non-directory.sandboxand a non-regular.sandbox/defaults.sh(nono/nono-here.sh:162 and :176). The documented exit contract should include all of these cases or the implementation should use distinct codes.
| 9 | Workspace `run_harness.sh` path exists but is not a regular file (directory, dangling symlink, socket) |
plans/2026-09-11_nono-here-bootstrap.md:705
- This verification record still names
hooks/{after,before}-template, but the current template and copied sandbox containhooks/afterandhooks/before. The archived plan therefore records a file set that cannot match the claimed successful copy; correct the verification text.
`.sandbox` holds `.gitignore`, `hooks/{after,before}-template`, `profile.template.json`,
`run_harness.sh`, `start.sh`, all six digests byte-identical to the template and all four
run_harness.sh:6
- This replaces the previously supported
CLOUD_DEFAULTS_FILEoverride withDEFAULTS_FILE; callers that still set the old variable will now silently ignore it and load the workspace defaults instead. Preserve the old name as a fallback or document this breaking environment-variable change.
DEFAULTS_FILE="${DEFAULTS_FILE:-$WORKSPACE/.sandbox/defaults.sh}"
- Files reviewed: 17/18 changed files
- Comments generated: 5
- Review effort level: Lite
| "websearch": "deny", | ||
|
|
||
| "external_directory": { | ||
| "/tmp": "allow" |
| elif [[ -f "$workdir/run_harness.sh" ]] && [[ ! -x "$workdir/run_harness.sh" ]]; then | ||
| die 2 "$workdir/run_harness.sh is not executable; run: chmod +x \"$workdir/run_harness.sh\"" | ||
| elif [[ -f "$workdir/run_harness.sh" ]] && [[ -x "$workdir/run_harness.sh" ]]; then | ||
| if [[ ! -x "$workdir/.sandbox/start.sh" ]]; then |
| @@ -1,5 +1,6 @@ | |||
| { | |||
| "$schema": "https://opencode.ai/config.json", | |||
| "plugin": ["opencode-cost-guard", "opencode-plugin-statusbar"], | |||
| **Left open, by design, not by oversight:** the hook-filename mismatch and the | ||
| template/deployed version drift (High-Risk Areas 1 and 2 in `PROJECT_MAP.md`) are | ||
| real, currently-active defects in this repo's own provisioned `.sandbox/` — a fresh | ||
| provision from today's template genuinely does not fire its hooks, and this repo's | ||
| own deployed `run_harness.sh` still carries the bug Task 10 fixed in the template. | ||
| Neither was in scope for any task in this plan; both need their own plan rather than | ||
| a patch bolted onto this one's close-out. |
What
Zero-config sandbox bootstrap:
nono/nono-here.shdiscovers the workspace, selects a harness (claude/opencode/codex/copilot/pi), provisions.sandbox/from a template, generates.sandbox/defaults.sh, and hands over torun_harness.sh. Plus portable templates, the test suitenono/test_nono_here.sh, and OpenCode config changes.Review findings — addressed in the latest commits
Blockers (overcommit review)
bashback to deny, Testingbashback to ask (validation is delegated to the Testing agent); the~/envexternal_directory allow from79afda0is removed.jqfallback instart.sh: no longer interpolates the unbound$tpl_verunderset -u; ANSI escape terminated; startup continues on the degraded path.defaults.shgeneration now rejects any pre-existing non-regular path (-e || -L+! -f, exit 9); a dangling symlink from a user template can no longer redirect the write outside.sandbox.defaults.shis generated before therun_harness.shmove, so a failed generation keeps the workspace on the provisioning path where stale-sandbox repair is offered again.run_harness.shis now byte-identical totemplates/default/run_harness.sh(VERSION 3: guarded${1:-}, empty-array expansion guard for Bash 3.2, absolute start.sh path). The post-mortem's "left open by design" is no longer true.Copilot inline findings
hooks/before/after— the namesrun_hookactually resolves. Fresh provisions fire both hooks; no manual rename step.$BASH_SOURCE, GNU-onlydiff --color=always(now probed; BSD diff degrades to plain output), "Couldn't find neither" wording — all fixed in template and deployed.sandbox/start.sh(bodies are now identical).profile.template.json:~/.gitconfigremoved fromread_file(the deployed profile denies it; the generic template no longer weakens new workspaces).run_harness.shdefaults contract:.sandbox/defaults.shis generated bynono-here.shand required; README updated accordingly (this repo's checked-in.sandbox/defaults.shsatisfies it).Test-suite portability nit: new preflight probe — if the fixture directory (
$TMPDIRor/tmp) is a noexec mount, the suite fails loud with the real cause instead of 19 cases failing with misleading "not executable" messages.Verification
nono/test_nono_here.sh: 28/28 pass (with an exec-capable TMPDIR; on a noexec/tmpthe new preflight fails loud as designed).defaults.sh→ exit 9 with no write-out; fresh provision produces workinghooks/before+after; no-jqstart.sh path warns and continues; liverun_harness.shargv correct for no-arg / flag / explicit-command invocations.Remaining known items (disclosed, follow-up)
run_hooksilently skips a present-but-non-executable hook (no warning).# VERSIONmarkers are decorative); template/deployed drift is closed for this PR but can recur.profile.template.jsondivergence exists by design (this repo's own sandbox).See
PROJECT_MAP.md→ High-Risk Areas for the current state of each.