Skip to content

Add a disabled owner-authorized Issue implementation pilot - #507

Merged
robgruen merged 6 commits into
mainfrom
dev/robgruen/accepted-implementation-pilot
Nov 2, 2026
Merged

robgruen merged 6 commits into
mainfrom
dev/robgruen/accepted-implementation-pilot

Conversation

@robgruen

@robgruen robgruen commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Implement the code portion of the exact agreement accepted in #504, separately from completion receipts in #506. #506 has now been merged by the owner and incorporated here without rewriting branch history. The pilot remains disabled; no live coding run, credential probe, budget activation, approval, or merge was performed.

  • Generate a copyable owner-only /typechat build <acceptance-comment-id>:<sha256> command from the durable public acceptance and its prior owner approval, independently of draft-artifact expiry.
  • Freeze scope, discussion, policy, source, and controls; reject competing contributions; reserve one coding run before inference; reject retry spending and stale/withdrawn authorization.
  • Bound generation at 1,000 AI credits and detection at a separate explicit 400 credits. Require native completion, exact-patch scan bindings, relevant deterministic checks, and actual in-budget usage.
  • Keep App credentials out of coding, detection, and generated tests. Run tests in a non-root, network-isolated container without host credentials or workspace/socket access.
  • Revalidate everything before creating one owned branch/PR through the existing App. Verify the complete resulting tree; never execute generated code with write credentials, force-push, overwrite existing work, auto-close, approve, merge, or release.
  • Add offline protocol/workflow tests, a real CI sandbox smoke test using a synthetic TypeScript/Python patch, reproducible compilation, and operator documentation.

Validation

  • 313 selected offline tests passed on Node 24.22.1, covering success, authorization, scope/policy freshness, App-token restrictions, isolation contracts, budgets, exact patch/check evidence, and uncertain-write retries.
  • Pinned gh-aw v0.93.5 strict compilation: all three workflows succeeded with zero warnings; repeat compilation produced no generated drift.
  • 100 local documentation links/anchors and staged whitespace checks passed.
  • The contribution-link GraphQL query was verified against GitHub's read-only schema.
  • Local selection excludes tests that read policy-restricted publisher source. All 14 GitHub CI checks passed on 2106448, including full repository automation tests and the real Linux TypeScript/Python container smoke test (run 41381584967). The smoke test verified credential/network isolation without any coding-model run.

Credential/action review

Compiler safe-update review covered the new use of the existing DEPENDABOT_APP_PRIVATE_KEY binding. It is referenced only in trusted preflight/publisher steps, not generated execution. DEPENDABOT_APP_ID must identify typeagent-bot; short-lived tokens explicitly request only TypeChat contents/PR rights, validate returned permissions/repository selection, mask credentials, and revoke them. Unrelated vulnerability-alert permissions are rejected. No secret values were accessed locally.

All actions use existing full-commit pins; no existing action-lock/runtime/model pins or redirect destinations changed. The new test container uses the verified immutable Node 24.22.1 image digest. Actual scoped App minting and the first coding run remain unverified and require the separate owner-authorized rollout.

Rollout and boundaries

  • TYPECHAT_IMPLEMENTATION_ENABLED remains unset/off. Manual command preparation spends no inference and mints no App token. Activation and a fresh Issue build comment are separate owner decisions after review/merge.
  • Supported patches are bounded ordinary text changes in the library/example/site areas and root README. Automation, policy, dependencies/configuration, credentials, agent instructions, binary/executable-mode changes, and unsafe paths are rejected rather than silently omitted.
  • Automation: pilot owner-triggered implementation for accepted Issues #504 authorizes development of this workflow; it must not be used to make the future coding agent rewrite its own controls. Automation: support implementation notes with work acceptance #502 remains deferred.
  • GOALS, repository/token/PR-approval settings, App configuration, existing Dependabot workflow, and the policy-restricted publisher source are unchanged. Existing library APIs and discussion behavior are preserved.
  • The separately authorized first live pilot must record whether normal PR CI and Copilot review actually ran. Those checks, human approval, and merging remain distinct; Automation: pilot owner-triggered implementation for accepted Issues #504 is not auto-closed by this PR.

Bind one coding run to a durable accepted agreement, isolate generation and tests, verify exact scan/check provenance, and publish one scoped App-owned PR only after fresh authorization. Keep activation and live verification separate.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 2, 2026 02:05
Separate sandbox context and candidate inputs, bound their serialized form, and update the CI contract for the third workflow.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical defects block sandbox execution and workflow validation, while patch collection can corrupt file content or silently mishandle executable-mode changes and renames.

6 open issues
What changed in this PR

Adds a disabled-by-default, owner-authorized implementation pilot for accepted Issues, separate from the completion receipts in #506. It extends repository automation without changing TypeChat’s library APIs; activation, live verification, and merging remain separate human decisions.

Changes:

  • Adds exact acceptance-bound build commands, frozen authorization, spending reservations, and scoped App publication.
  • Adds bounded patch collection, threat-detection evidence, and credential-free container checks.
  • Adds protocol tests, a CI sandbox smoke test, and rollout/recovery documentation.
File Description
tools/​tests/​issue-implementation.test.mjs Tests authorization, patch validation, budgets, publication, and App access.
tools/​tests/​implementation-workflow.test.mjs Tests workflow guards, permissions, artifacts, and compilation contracts.
tools/​tests/​implementation-sandbox.integration.test.mjs Adds a real TypeScript/Python isolation smoke test.
tools/​issue-implementation.mjs Implements build authorization, patch collection, validation, and publication.
tools/​implementation-sandbox.mjs Runs deterministic checks in an isolated container.
tools/​implementation-app.mjs Validates, scopes, masks, and revokes App credentials.
tools/​discussion-draft.mjs Exposes shared control, detection, and intent helpers.
README.md Updates acceptance status and introduces the disabled pilot.
OWNER-CHEATSHEET.md Documents owner commands, budgets, and recovery steps.
AUTOMATION.md Documents implementation safeguards and rollout requirements.
.github/​workflows/​issue-implementation.md Defines the owner-triggered implementation workflow.
.github/​workflows/​agent-workflow-check.yml Adds pilot compilation and sandbox smoke-test CI.

Review effort: Balanced

Comment thread tools/implementation-sandbox.mjs Resolved
Comment thread tools/issue-implementation.mjs Resolved
Comment thread tools/issue-implementation.mjs Resolved
Comment thread tools/issue-implementation.mjs Resolved
Comment thread tools/implementation-sandbox.mjs Resolved
Copilot AI balanced review requested due to automatic review settings November 2, 2026 02:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Binary deletions can reach publication without their contents being scanned because binary-diff notices are accepted as complete patches.

6 open issues
1 resolved since last review

Review effort: Balanced

Comment thread tools/issue-implementation.mjs Resolved
Give detection its required read permissions; preserve renames and reject unsupported modes and bytes; prepare the Python environment as the isolated test user with the existing CI virtualenv constraint.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 2, 2026 02:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The candidate-path filter allows changes to python/hatch.toml that can redefine the sandbox’s test script, undermining the immutable test-configuration boundary and the reliability of check evidence.

No open issues 🎉

3 previously missed issues

In code that hasn't changed since last review

High severity Reject alternate Python config and dependency manifests

tools/​issue-implementation.mjs:441

This filter accepts python/hatch.toml and python/Pipfile as ordinary source changes. In particular, Hatch reads hatch.toml as project configuration, and the sandbox applies the candidate before invoking hatch run test, allowing a candidate to redefine the test script while check evidence still lists the expected Hatch command. Reject these alternate configuration/dependency manifests to preserve the immutable test-configuration boundary, and add them to the protected-path regression cases.

Medium severity Ignore generated dependency and virtual-environment files

.github/​workflows/​issue-implementation.md:484

The sparse checkout omits the root .gitignore, so normal site dependency installation or Python environment creation leaves generated files visible to collectCandidate(). The collector then rejects site/node_modules/ or the root .venv/ as unsupported patch paths, blocking otherwise valid implementations after local checks. Include /.gitignore in this list and regenerate the compiled workflow.

Medium severity Check Issue eligibility after returning existing results

tools/​issue-implementation.mjs:341

Once a successful implementation's Issue is closed or locked, rerunning its original command fails here before looking up the existing PR. Move this eligibility check after the existing-result return so completed commands return their result while closed or locked Issues still block new work.

7 resolved since last review

Review effort: Balanced

Leave acceptance-status updates to the receipt PR and place the implementation guide outside its edited sections.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 2, 2026 02:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Human assessment is needed to evaluate detector execution after failed authorization, dependency-collection failures, and acceptance of paths incompatible with Windows checkouts.

No open issues 🎉

Review effort: Balanced

robgruen and others added 2 commits November 1, 2026 19:56
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Require successful generation and trusted staging before releasing detector credentials, propagate collection/dependency failures, and reject trailing-dot or case-aliased path components. Add focused regression coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 2, 2026 03:10
@robgruen

robgruen commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up to the broader review note on 5a3c0f8: addressed in 2106448 after integrating merged #506 without rewriting history.

  • Detector eligibility now requires successful preparation and successful sole-noop generation. Its only configured inference credential is withheld unless trusted authorization/exact-patch staging succeeded and preceding job steps have not failed or been cancelled. This handles the pinned runtime launcher continuing after a failed custom pre-step without weakening scan or publication gates.
  • Regression tests confirm failed Git enumeration, source archival/extraction, and dependency installation cannot produce a partial candidate or passing check evidence.
  • Windows validation now rejects trailing-dot path components and directory-case aliases, including collisions against the base tree and between candidate paths.

Validation: 313 selected offline tests pass on Node 24.22.1; all three workflows compile strictly with zero warnings and no repeat-compilation drift; all 14 CI checks pass, including the real isolated TypeScript/Python container test (run 41381584967). Policy-excluded publisher-source cases remain a CI result, not a local inspection claim. No new credentials, runtime/model pin changes, activation, merge, or live coding run were performed. Human review and separately authorized live verification remain required.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The path guard allows python/hatch.toml to redefine required sandbox checks and permits generated site/_site files that the build can overwrite instead of validating.

2 open issues

Review effort: Balanced

!name.split("/").some(part => !part || part === "." || part === ".." || part.startsWith(".") || part.endsWith(".") ||
/^(?:node_modules|dist|out|build|coverage|__pycache__|con|prn|aux|nul|com[0-9]|lpt[0-9])(?:\.|$)/i.test(part)) &&
!/(?:^|\/)(?:AGENTS|CLAUDE|SKILL|GOALS|AUTOMATION|OWNER-CHEATSHEET)\.md$/i.test(name) &&
!/(?:^|\/)(?:package(?:-lock)?\.json|npm-shrinkwrap\.json|pyproject\.toml|(?:yarn|pnpm|poetry|uv|hatch)\.lock|pnpm-lock\.yaml|(?:tsconfig|jsconfig|pyrightconfig)(?:\.[^/]+)?\.json|[^/]*\.config\.[^/]+|Dockerfile|Makefile|requirements[^/]*|conftest\.py|(?:pytest|tox|setup)\.(?:ini|cfg|py))$/i.test(name) &&
requireValue(typeof name === "string" && name.length <= 240 &&
/^(?:README\.md|(?:typescript|python|site)\/[A-Za-z0-9_./-]+)$/.test(name) &&
!name.split("/").some(part => !part || part === "." || part === ".." || part.startsWith(".") || part.endsWith(".") ||
/^(?:node_modules|dist|out|build|coverage|__pycache__|con|prn|aux|nul|com[0-9]|lpt[0-9])(?:\.|$)/i.test(part)) &&
@robgruen
robgruen merged commit a36fe49 into main Nov 2, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants