-
Notifications
You must be signed in to change notification settings - Fork 79
docs: Populate READMEs #3047
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
docs: Populate READMEs #3047
Conversation
gibson042
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also consider incorporating #2920 (or review/merge in the near future).
| @@ -1,5 +1,187 @@ | |||
| # Endo CLI | |||
| # `@endo/cli` | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're finally committing to this convention, this PR should update skel and/or create-package.sh accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated skel README to use [package] placeholders that create-package.sh expects.
| ## License | ||
|
|
||
| [Apache-2.0](./LICENSE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likewise this.
| endo ls|list # List known names | ||
| endo ls|list some-directory # List names in a directory | ||
| endo ls|list path.to.directory # List names in a deeply nested directory | ||
| endo show my-value # Print a value | ||
| endo cat my-blob # Dump blob contents | ||
| endo rm|remove old-name # Forget a name | ||
| endo mv|move old-name new-name # Rename | ||
| endo cp|copy src-name dst-name # Duplicate a name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to do a double take to realize that the | in ls|list was meant as a BNF-style lexical alternate, since the sh lang context otherwise primed me to think you were trying to pipe-into some list command
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! Changed to parenthetical notation: endo list # (or: endo ls)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still see | notation. Did you forget to push?
| ```sh | ||
| endo inbox # List received messages | ||
| endo inbox --follow # Follow messages as they arrive | ||
| endo adopt 1 value-name # Adopt a value from message #1 | ||
| endo dismiss 1 # Delete message #1 | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this is supposed to be the "alice" host inbox from above? how do we know?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed! Added explicit --as bob to clarify whose inbox we're viewing, with a header note "(bob's perspective)".
| endo request "I need access to the database" --to HOST | ||
| endo resolve 1 database-ref # Grant request #1 with a named value | ||
| endo reject 1 "Not authorized" | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are interleaving alice/bob viewpoints implicitly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed! Added comments clarifying "Bob requests..." and "Host reviews inbox..." to make the perspective switches explicit.
| ## References | ||
|
|
||
| [Netstring][] | ||
| D. J. Bernstein |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<3 djb ... it's been far too long since I was in the sendmail vs qmail trenches ... world could really do with some more djbtools in current year
|
Thank you for the feedback! I've addressed the comments:
|
erights
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes to the lp32 package look very similar to those in #3044 . I already reviewed that one. Does that review cover this version of those three files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't under the diffs for memoize/README.md . It looks redundant with #3042 , but that's not the confusing part. The confusing part is that this PR is diffing against the kriskowal-docs-memoize branch, which is #3042. But the left side of the diff does not show the README,md from that branch.
In any case, the important question is the same as for #3044 : I already reviewed #3042. Does that take care of the need to review memoize/README.md in this PR?
erights
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same q again for #3045 which I already reviewed.
erights
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just comments so far. More coming.
| This package provides functions to adapt Node.js `Readable` and `Writable` | ||
| streams to Endo's stream interface, which models streams as hardened async | ||
| iterators of `Uint8Array` chunks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since writer users pass an argument to next, I wondered whether it is still ok to call these "async iterators". Indeed it is. The TS def seem to be
interface AsyncIterator<T, TReturn = any, TNext = any> {
// NOTE: 'next' is defined using a tuple to ensure we report the correct assignability errors in all places.
next(...[value]: [] | [TNext]): Promise<IteratorResult<T, TReturn>>;
return?(value?: TReturn | PromiseLike<TReturn>): Promise<IteratorResult<T, TReturn>>;
throw?(e?: any): Promise<IteratorResult<T, TReturn>>;
}and
/**
* Describes a user-defined {@link AsyncIterator} that is also async iterable.
*/
interface AsyncIterableIterator<T, TReturn = any, TNext = any> extends AsyncIterator<T, TReturn, TNext> {
[Symbol.asyncIterator](): AsyncIterableIterator<T, TReturn, TNext>;
}Given those, do we really need our own
// Stream is nearly identical to AsyncGenerator and AsyncGenerator should
// probably be identical to this definition of Stream.
// Stream does not make the mistake of conflating the read and write return
// types.
export interface Stream<
TRead,
TWrite = undefined,
TReadReturn = undefined,
TWriteReturn = undefined,
> {
next(value: TWrite): Promise<IteratorResult<TRead, TReadReturn>>;
return(value: TWriteReturn): Promise<IteratorResult<TRead, TReadReturn>>;
throw(error: Error): Promise<IteratorResult<TRead, TReadReturn>>;
[Symbol.asyncIterator](): Stream<TRead, TWrite, TReadReturn, TWriteReturn>;
}?
Ah. The comment explains:
// Stream does not make the mistake of conflating the read and write return
// types.
That just seems to be a but in the TS type. Should we report it? In any case, I see the problem. No change suggested.
Curious though if the comment should say AsyncIterableIterator rather than AsyncGenerator.
|
|
||
| const reader = makeNodeReader(process.stdin); | ||
| const writer = makeNodeWriter(process.stdout); | ||
| await pump(writer, reader); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example is only self contained if you also understand pump. Brief explanation here?
Btw, since we're only using iterators and not generators, prime is now obsolete. Can we kill it? (though not in this PR of course)
| - The stream must not be in object mode | ||
| - The stream must not have an encoding set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm scared to ask what these are ;) . In any case, this part of the doc is for people who presumably know something about node streams, so no change suggested.
|
|
||
| Adapts a Node.js `Writable` stream to an Endo writer. | ||
|
|
||
| - Respects back-pressure via the `drain` event |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool! I look forward to understanding this.
Perhaps for all of these, how about we link to the relevant Node Stream docs at the top of this README.md ?
|
|
||
| const reader = makeNetstringReader(byteStream, { | ||
| name: '<my-stream>', // optional, for error messages | ||
| maxMessageLength: 1000000, // optional, defaults to 999999999 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the comment, please put either commas of underbars to separate each triple of digits. I'd also appreciate such underbars in the code for long numbers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious why the max here is 99... but the max for another one (stream node?) was 100...
|
|
||
| ### Writing chunked messages | ||
|
|
||
| Messages can be passed as arrays of chunks to avoid pre-concatenation: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that the writer itself does concat these, do you really want to call them "chunks" given your other use of "chunks" in streams?
| using decimal strings as variable width integers. | ||
| For example, the frame `5:hello,` corresponds to the message `hello`, | ||
| where `5` is the length of `hello` in bytes. | ||
| Async iterator streams for [Netstring][] protocol. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somewhere you should point out that several of the make*Reader and make*Writer pairs follow the same conventions. User will make their own pairs and you should encourage them to follow the pattern.
| Through the CLI, users can run confined programs, manage named values, send | ||
| messages, and control the daemon lifecycle. | ||
|
|
||
| See [`@endo/daemon`](../daemon/README.md) for more about the underlying daemon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you expect to similarly flesh out daemon/README.md ? Without knowing it yet, my only suggestion is to turn some mentions into links.
| Or invoke the CLI directly: | ||
|
|
||
| ```sh | ||
| node packages/cli/bin/endo --help |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the scope of the daemon? On a multi-account system (i.e., allegedly "multi-user", like POSIX), if each account follows these instructions, does each get their own daemon? If not, Bob have the ability to stop the daemon Alice started?
| ```sh | ||
| endo run program.js | ||
| endo run program.js --powers NONE # No special powers (default) | ||
| endo run program.js --powers AGENT # All of the primary user's agency |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is an "agent"? What is an "agency"? Do you intend "agency" to simply be the plural of "agent"? If so, I find that weird. Google says the plural is "agents", as I expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the "primary user"? This is the only mention in this doc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this an ACL check? Is the right to act as primary user sharable? transferable? deniable to programs that run as me?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is “agency” in the second sense,
the capacity, condition, or state of acting or of exerting power
— Merriam Webster
The AGENT powers are effectively limited only by the host operating system and the user that started the daemon, since they include creation of unconfined programs.
The name AGENT is left over from the last round of development and clearly suffers from semantic diffusion. We’ll need to change it. The pool of alternatives is mucky. My best idea today is PROFILE because there are to be other profiles like profile-for-that-app, my-alternate-profile, profile-for-friend, &c. So, SELF would be your handle and PROFILE would be the combination of powers and mailbox that the user’s alter-egos, apps, peers, and LLM agents hold.
(Hah! Or, we could use ID for your handle and EGO for your powers. They have the virtue of brevity, and the pun.)
For the purposes of this documentation, I’ll clarify that AGENT designates all the powers available to the user that started the Dæmon.
6b9c56d to
8fa80d6
Compare
No description provided.