feat(cmd): add approve subcommand for workflow gates - #98
Merged
Conversation
Adds `devkit approve <name>` which writes `.devkit/gates/<name>.approved` with a timestamp and git-config approver identity. Idempotent on repeat invocation, rejects path-traversal and otherwise-unsafe names. Unblocks a polling-gate pattern for workflows where a shell step waits on a marker file before the pipeline continues — e.g. a plan-review gate in a long-running feature workflow.
Atomic marker publish (the NO-SHIP blocker) plus cheap cleanups. - Publish marker via temp-file + rename so a failed approve can no longer leave a visible marker that unblocks a polling gate - Single git subprocess via --get-regexp instead of two - 2s timeout on git config so a locked/misconfigured repo can't hang - Route output through cmd.OutOrStdout() for testability and redirection - Include regex anchors in the invalid-name error message - Test mtime preservation on idempotent re-approve; assert null-byte rejection happens at the regex layer; cover parseGitUserRegexp - Use os.DevNull instead of hardcoded /dev/null in test fixture
- Cover approverIdentity fallback chain: $USER and "unknown" paths had
no coverage because newTestRepo always seeded a valid git identity.
Add isolateGitConfig helper + two tests so a regression that reorders
or drops the fallback branches fails loudly.
- Rename wantErr -> wantMatch in the name-validation table. The old
field was readable backwards ("wantErr: false" meant "expect match")
and a reviewer misread the assertion as a no-op. The logic was
correct; the naming was the bug.
- Backdate mtime between the two idempotent approves so the "file was
not rewritten" check works on filesystems with 1-second mtime
resolution (ext3, older HFS+, some CI overlays). Previously flaky
off APFS/ext4 — the two approves finish in microseconds.
- Log git failures in approverIdentity to stderr before falling back.
Silent swallow left users with approver=unknown and no signal that
git was consulted and failed (timeout, corrupt config, locked index).
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
1 participant
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.
Summary
devkit approve <name>CLI — writes.devkit/gates/<name>.approvedwith a timestamp and git-config approver[a-zA-Z0-9][a-zA-Z0-9_-]{0,63}Why
Enables a polling-gate pattern in workflows without any engine changes:
```yaml
command: |
while [ ! -f .devkit/gates/plan.approved ]; do
echo "Waiting for approval — run: devkit approve plan"
sleep 5
done
cat .devkit/gates/plan.approved
expect: success
```
The step blocks until the marker exists. Ctrl+C kills the shell cleanly via `exec.CommandContext`. No new YAML field, no new step type, no session-state changes — this is pure convenience over what command+expect already supported.
Test plan