Repository navigation
extdeps.os.id - #10285
extdeps.os.id#10285briansrls wants to merge 3 commits into
Conversation
…ero-consumer ruling request The census's §5.A table still read `extdeps.os.id` as a NEW op at `dag/extdeps/os/id.dag` -- the stale row a survey read as "still pending". The op landed on #7194 at its post-#7231 home `dag/extdeps/tools/id.dag` (module `extdeps.tools.id`, service `os.Id`, ops `Uid`/`Gid`/`Lookup`), hermetic witness `test.claim.shell_dag_census_5a_typed_ops_witness` (`witness_os_id_uid_realizes_hermetically`, `witness_os_id_gid_realizes_hermetically`, `witness_os_id_lookup_realizes_hermetically`). The row now says so, cited by module and symbol rather than position, and the consumer column states the true status: `host_effect_realize`'s ssh probes already consume the typed argv rows `id_uid_argv`/`id_gid_argv` (absolute-path variants), and `fleet_posix_accounts` `probe_command` is NOT-a-shell-site (census §4 row) -- authored provenance, live proof `deploy_access_check_observed`. The same PR files a ruling request, not a deletion: `id_lookup_argv` (the `id <user>` verb) has zero production references -- measured denominator, grep of `dag/` + `src/v2/test/` at `75873c2897`: the definition, the hermetic-witness call, and service-name rows in the effect-plan tests, nothing more. Zero references is not deadness in this repo (env-gated and emitted-path consumers exist that grep does not see), and the row's named consumers are reclassified (`fleet_posix_accounts` -- NOT-a-shell-site) or deferred (`spark/managed_access_apply` create-script bodies -- dissolve-on marker, no operator verdict). Retain-as-reserved or retire is the operator's call; the row records the measured denominator and asks for it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d §5.A row RULING (2026-09-03): retire extdeps.tools.id.id_lookup_argv. No measured production consumer; the semantic os.Id.Lookup already names the intended request; a future consumer should consume IdLookup/IdInvocation and choose an SSH realization, not resurrect a raw List<String> argv helper. Keeping it "reserved" would preserve the lower-level interface the migration exists to make unavailable — a future need is the trigger to add a typed remote realization, not evidence that the helper should survive. Deleted from dag/extdeps/tools/id.dag. The module's sole production import (dag/gunbc/host/host_effect_realize.dag:69) imports id_uid_argv, id_gid_argv and id_binary_path only — no reference to id_lookup_argv exists in the code tree. The witness (test.claim.shell_dag_census_5a_typed_ops_witness, witness_os_id_lookup_realizes_hermetically) exercises os.Id.Lookup, the semantic op — NOT the deleted argv helper — and is unchanged. CORRECTION (second finding): the §5.A row overstated by claiming "Done" when the hermetic operation witnesses had landed but the model-before-implement scope (call-site migration a later lane, the superseded argv/string path still standing, remaining SSH callers dispositioned independently) does not support that word. The row now says honestly: "operation model + hermetic operation witnesses have landed" and explicitly what is NOT done instead of claiming closure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The ruling record directed future consumers to `IdLookup`/`IdInvocation`, neither of which exists in the tree. `os.Id.Lookup` is the actual semantic operation — cite it directly, with the two plausible typed SSH-realization paths (`ssh.Session.ExecArgv` on `id <user>`, or a full `IdLookup` invocation modeled on the `os.Hostname.Set`/`HostnameSetInvocation` pattern) so a migration worker is not left to guess. No changed symbols or fabricated interfaces. DESIGN §3 single-authority and "cite the symbol" rules satisfied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf445a80a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | `ssh.Session.ExecArgv` | **add to** `dag/extdeps/diagnostic/ssh.dag` | typed argv over ssh (`ssh host -- argv`) | `host_effect_realize` ssh probes — **IN FLIGHT, C5 #6946** | | ||
| | `systemd.Systemctl.Status` | **add to** `dag/extdeps/os/systemctl.dag` | `systemctl status --no-pager --full <unit>` (models exit-3-for-dead-unit — retires the `\| tail`+`exit 0` absorbing fallback) | `live_deploy/readiness.dag` unit diagnosis | | ||
|
|
||
| **RULING — `extdeps.tools.id.id_lookup_argv` RETIRED and deleted (ruled 2026-09-03).** The retirement is on the ruling, not on the zero-reference count: `os.Id.Lookup` (the semantic operation) already names the intended request; a future consumer should consume `os.Id.Lookup` and choose a typed SSH realization (`ssh.Session.ExecArgv` on the `id` binary with a `{user}` argument, or an `IdLookup` invocation modeled on the `os.Hostname.Set`/`HostnameSetInvocation` pattern) — not resurrect a raw `List<String>` argv helper because it sat in inventory. Keeping it "reserved" would preserve exactly the lower-level interface the migration exists to make unavailable. The measured denominator (filed 2026-09-03) found no production reference — the §5.A witness calls `os.Id.Lookup`, NOT this helper — but a future need is the trigger to add a typed remote realization, not evidence that the helper should survive. |
There was a problem hiding this comment.
When a future lookup receives a username containing whitespace or shell metacharacters, the suggested direct ssh.Session.ExecArgv realization does not preserve that value as one remote argument: dag/extdeps/ssh/session.dag:36-40 explicitly marks this operation for dissolution because SSH carries a single command string, and lines 49-63 explain that OpenSSH joins and remotely shell-parses the words. Since os.Id.Lookup.user is only a NonEmptyStr, following this new ruling can turn the username into shell syntax unless the caller separately uses the portable-word refusal wall. Point the ruling to ExecPortableWords/the guarded fleet SSH seam or only to the proposed typed IdLookup realization instead.
Useful? React with 👍 / 👎.
|
Closing: this PR has no content to land, and merging it would REVERT work already on main. It was auto-opened on session/keen-moth-880 after #10233 squash-merged. Everything it carries is already on main via that merge (20b1b8f) -- the id_lookup_argv retirement, the corrected section 5.A row, and the ruling record that points at os.Id.Lookup. The branch is 37 commits behind main, and the ONLY remaining difference is a stale row: this branch still carries the OLD hostname census entry (extdeps.os.hostname, "hostnamectl set-hostname remains") while main carries the newer one from #10211 (extdeps.tools.hostname, "LANDED, then migrated"). So the sole effect of merging would be to regress that row to a pre-#10211 state. The CI failure on the older run is also not this branch's defect: it inherited the duplicate-declaration refusal in gunbc.recurring_failure_mode that was breaking main itself, fixed since by #10278. No fix is warranted and none should be pushed. Further work on this lane should branch fresh from main. |
1 participant
Auto-opened by session-dashboard for session
keen-moth-880.Pushing to
session/keen-moth-880advances this PR.Worker attestation
Before flipping this PR to ready for review, confirm each item:
npm test,cargo test) and the result.Closes #Ndirective.Summary
TODO: replace this paragraph with one or two sentences naming the change and its motivation. Reviewers read this first.
Test plan