Doing ;me.moveTo(otherRoom) succeeds but does not cause the world to update to show the room I've arrived in.

Quite often, when trying to navigate away from a function which I have browsed but not edited, I get prompted to save or discard changes.

In the most recent case, I see in the diff view that the editor has removed an apparently-spurious hyphen that had been present in the @-directives at the top:

AFAICT that hyphen should not have been there in the first place.

At the moment, the following code will prevent a, b and c from being garbage collected even though only a is ever accessible:

function foo(a, b, c) {
  return function() {return a;};
}

var longLived = foo('a', 'b', 'c');

Because of eval we can't always know which closed-over local variables are accessible:

function foo(a, b, c, src) {
  return function() {return eval(src);};
}
var f = foo('alpha', 'bravo', 'charlie');
var a = f('a');  // a === 'alpha'

But in the absence of direct calls to eval in the body of the inner function we can—and should.

Note that things get more complicated when separate closures created share variables, e.g:

function foo(a, b, c) {
  return [function ab() {return a + b;},
          function bc() {return b + c;},
          function ca() {return c + a;}];
}

Ideally if only ab is retained c should be garbage collected—but in practice implementing this for arbitrary combinations may be impractical (and V8 doesn't try).

Related reading:

• Closure memory leak of unused variables
  
• Slide 36 ("Unintended retention – runtime black magic") of the tc39 Weak References for EcmaScript proposal.

§15.3.4.5.2 of the ES 5.1 spec gives special behaviour to calling [[Construct]] on a bound function (i.e., applying new to a value returned by Function.prototype.bind).

This is needed in order to ensure that, e.g., new Function.prototype.bind.apply(Date, args) works correctly.

Because bind is currently a polyfill, the interpreter is very probably in violation of this part of the spec.

If the server fails to respond to an attempt to save (e.g. because of network issues or the WWW thread getting killed for running too long) the editor remains locked so no more editing can be done and additionally preventing the save button from being clicked again.

There should be some timeout after which the editor gives up and assumes saving has failed and let you continue editing / re-try save.

At the moment, connections to http://google.codecity.world, http://google.codecity.academy and http://google.codecity.games redirect to https://google.codecity.games, but https://google.codecity.academy and https://google.codecity.games do not. (That is: redirects from HTTP -> HTTPS canonicalise, but no redirect or canonicalisation is done for already-HTTPS connections.)

All connections should redirect to HTTPS to the server's canonical name.

I propose that when the user types into the verb name field, or selects any option from the dobjspec / prepspec / iobjspec dropdowns, the "is a verb" tickbox should get ticked automatically.

I keep trying to create verbs and failing to tick the box, consequently losing whatever other settings I had chosen when I click save.

Presumably the fix from 95ccfbb needs to be applied to the old editor too.

When examining the value of $.foo.bar.Func.prototype.constructor, the editor should try to show it as a selector string, unless the selector string would be "$.foo.bar.Func.protototype.constructor".

In particular, it should usually be shown as $.foo.bar.Func, if that is its actual value.

Per ES7 §21.2.6:

Prior to ECMAScript 2015, RegExp instances were specified as having the own data properties source, global, ignoreCase, and multiline. Those properties are now specified as accessor properties of RegExp.prototype.

We should ensure that this impedance mismatch between the specified ES5.1 behaviour and current Node.js behaviour is dealt with correctly.

This bug tracks things that we must or should to do before beginning our initial dogfooding.

• Verify that we happy with version of server in use, and that no architectural changes affecting safe file format will be needed for minimum 3 months.
  • Create branch for (and tag this version) in GitHub, in case we need to reconstruct from repo + a .city file.
• Make sure checkpoints won't fill server persistent disk.
• Automate back-ups of prod server persistent disk (or at least .city files thereon).
  • Verify that back-ups are actually running.

inspect foo, where foo has never previously been inspected, results in the editor opening but foo not being shown because it has no known selector.

When this happens, we should create a temporary selector (e.g. for an entry in a temp database) for the unknown value.

Implementation of for...in was accidentally cubic; as of cde2631 it is now merely quadratic, but it could be linear. The Array.prototype.sort polyfill uses bubble sort. There are doubtless numerous other examples of avoidably inefficient implementations in the interpreter.

The following code should evaluate to 'baz' (rather than throwing TypeError, as it presently does):

var o = {};
Object.defineProperty(o, 'foo', {writable: true, enumerable: false, configurable:false, value: 'bar'}); 
Object.defineProperty(o, 'foo', {writable: true, enumerable: false, configurable:false, value: 'baz'}); 
o.foo;

Several bits of functionality we support—notably, the functions escape, unescape and String.prototype.substr—appear only in Annex B of the ES5.1 spec.

We should consider removing these since they are not required for backwards compatibility (as they would be in a web browser) and generally duplicate better functionality found elsewhere

loginServer.js and connectServer.js should complain any of the required entries in their respective config files are missing or empty.

I just spent an inordinate amount of time trying to figure out why I was being immediately disconnected from the world upon login, only to find that it was because I'd forgotten I needed to manually add pingPath: "/ping", to connectServer.cfg.

Object.freeze and Object.seal are in ES5.1, but for some reason we do not implement them. (They can be pretty trivially polyfilled, because we do implement Object.preventExtensions.)

I am unable to log in using my @gmail.com account; I get the message "Google Authentication fail: Error: invalid_grant" at https://google.codecity.world/login?code=[CENSORED]&scope=openid+email+https%3A%2F%2Fwww.googleapis.com%2Fauth%2Fuserinfo.email#.

Chrome very helpfully prevents the inspect verb from opening the editor.

We should alert the user to the fact that that this has happened (or, if it isn't easy to detect, at least warn them about the possibility the first time they use the command).

Flatpack (and in particular the methods flatpack.flatten and .unflatten) correctly handle cyclic data and shared substructure involving pointers, but from reading the source for reflect.DeepEqual it's clear that it's possible to encounter cycles that do not involve pointers.

This may not be a problem at the moment because the interpreter makes extensive use of pointers when dealing with cyclic/shared data, but Flatpack should handle all reference types correctly so that we don't need to worry about it elsewhere.

The spec says that this does, in effect,ToObject(this) which would normally produce boxed primitives. That is not what we want, and not what our code does. But what does our code do, and what do we want it to do?

Attempting to save a new value to a read-only property causes the editor to hang, with the editors frame greyed out and a spinning blue ball cursor.

It should be possible to save a new value to any configurable property. Maybe issue a warning for non-writable properties?

I discovered today that, in testing on localhost, I had two ID cookies, one with path /connect as expected, and one with path /.

This caused an issue when upgrading to the new ID checksum algorithm, because only the /connect cookie was updated to the new checksum, and connectServer was rejecting me because it was getting the / cookie with the old (now invalid) checksum.

I thought it might be a one-off, and deleted both cookies, but after a little while they are both back.

I note on google.codecity.world/connect I have just the / cookie.

Per §15.3.2.1 of the ES5.1 spec, Function should accept comma-separated argument lists—i.e., Function('a,b', …) should be equivalent to Function('a', 'b', …) (rather than throwing a SyntaxError, as it presently does).

To reproduce:

• Click on inherited property (one not in the top section of the panel).
• Set a value and click save.
• Note that property is still shown as inherited (not in top section).
• In parent panel, click away and back to object being edited; note that just-set property now correctly shown as non-inherited.

It seems that when a MemberExpression throws a TypeError inside a SwitchStatement, the TypeError's .stack ends up blaming the SwitchStatement instead of the MemberExpression. E.g., executing the following throws TypeError: Can't convert undefined to Object (correct) but the .stack property of the error blames the line containing switch (1) instead of the line containing undefined.hasNoProperties (incorrect):

switch (1) {
  case 1:
    return undefined.hasNoProperties;
}

It's reasonable enough that this function does not preserve circularity (it's only intended to handle the POJSO equivalent of a JSON value), but it should not recurse infinitely if passed circular data.

Using the browser's find function (⌘F in Chrome on macOS) in the code and value editors does not find all occurrences of the specified text in long functions (e.g., search for "data" in $.www.code.editor.www). Which occurrences are found seems to depend somewhat on where the cursor is located, but even if the cursor is on an occurrence near the end of the function that occurrence may not be found.

Presumably this some undesirable interaction between CodeMirror and Chrome.

At the moment, polyfilled builtins like Array.prototype.join:

• are constructible, and
• have a (writable, but non-configurable) .prototype property.

Neither of these should be true.

When property names don't fit in the width of the ObjectPanel, a horizontal scroll bar appears (yay!), but actually scrolling reveals some pretty bad layout:

It's not clear where the type indicators should go: pushing them all the way to the very right of the panel would be inconvenient when looking at shorter property names. But at the black lines, highlight bars, etc., ought to extend the full width of the panel.

Originally I was going to propose merely that the function editor load the list of preposition options from the server.

Then I thought the same should be true for the choices for dobj and iobj (I am thinking in particular about introducing a distinction between "any matchable object" and "any random text").

Better still that whole row of controls be served up by the database, so the fields could be changed appropriately if we write a more sophisticated command parser.

And taking this to its logical conclusion: really, all the editors ought to be loaded from the DB, so they can be entirely suited to whatever kinds of data/objects are stored there.

For some reason the testRoundtripScopeRefAndPropIter causes a 15% performance degradation in subsequent benchmarks (as measured by benchFibbonacci10k).

Here is the offending test, from serialize_test.js:

exports.testRoundtripScopeRefAndPropIter = function(t) {
  runTest(t, 'testRoundtripScopeRefAndPropIter', `
      var r = 0, o = {a: 1, b: 2};
  `,`
      for (var k in o) {
        r += o[k];
      }
  `, 'r;', 3, 1, true);
};

N.B.: Changing seventh parameter to runTest—the number of steps between roundtrips—from 1 to 30 will avoid the performance hit, but setting it to 20 does not—even though in both cases just one roundtrip is done during the test (instead of 33 as would be the case roundtripping every step)!

The google production instance has had the compute API access scope enabled to facilitate simple crontab-based snapshotting.

This should be disabled before launch.

Optionally: create a specific role for our GCE instances with just the required permissions.

CC.checkpoint passes its arguments unmodified to CodeCity.checkpoint. Passing a truthy first argument will cause the interpreter to be paused and never restarted, effectively wedging it.

The other builtins defined in CodeCity.initSystemFunctions should be checked for similar issues.

We need to come up with a plan for how to structure the distribution and updating of code in CodeCity.

Just looking at the server/database (ignoring connect and login servers and client software), there are several components:

1. The server proper, including the interpreter: the code that node.js executes directly; handles networking, checkpointing and running userland code.
2. The polyfills.
3. General-purpose infrastructure: user accounts, REPL, webserver, logging & auditing tools etc.—stuff that would likely be of interest to anyone using CC to serve content on the internet.
4. The CodeCity 'world' infrastructure: prototypes for rooms, avatars, etc.—stuff that is only relevant to MOO-like usage and/or with our comic client, but which will be needed by most/all .codecity.* instances.
5. User content: stuff created by us and our users for use in our instance.

Items 3 & 4 are what we would traditionally think of as "the core". (There are probably further subdivisions, but these are the principle ones.)

Some mutually-contradictory desires:

• Be free to hack on items 2–5 in-world.
• Be able to write and run tests for items 2–5 in-world (especially as a way of setting a good example).
• Have a single source of truth for tests of items 1–2 (ideally based on external ECMA compatibility test suite, though likely with additions and modifications).
• Be able to easily distribute updates to items 1–4 to other instances / installations.
• Be able to mange code for 1-4 in git or similar version control system.

Some possible strategies (not all of which are compatible with each other or all the desires):

• Developing code (esp. for items 2-3) as text files stored in git alongside the server sourcecode.
• Loading and/or updating code by feeding the server a text file at startup (possibly in addition to an existing checkpoint file).
• Keeping some version information in the database (maybe a single version number for each of items 2-4, or perhaps a version number on each prototype object).
• Having "corify" routines to export snapshot of core as checkpoint file or as .js text file.
• Building a git implementation in-core.
• Writing a preprocessor to convert an existing JS test suite to a form we can run.
• Having a "test" database core to run ECMA compatibility tests.

Some difficulties we will have to deal with:

• Corification is quite challenging; LambdaMOO's corification routines were not very robust.
  • Though this might be assisted considerably by having routines that can do checkpointing to plain .js files.
• Other instances will have customisations they will want to keep (especially to item 4, which will likely see considerable customisation for different languages etc.)
• Code to do updates is necessarily going to be a little different than code that creates a new instance (or needs to be more complicated if it is to reliably do both).
• Certain kinds of updating (when touching stuff that has closures) may be difficult / impossible to do in-database.

At the moment, if a checkpoint fails part way through writing the .city file to disk (e.g., because of disk full) it will leave an invalid but validly-named .city file in the checkpoint directory.

• The .city file should be created atomically (i.e., by renaming a .city.partial file or similar).
• Failed attempts to write this file should clean up after themselves.

Some properties shouldn't be preserved

When a function is saved, properties are (by default) preserved. This is useful, but a few properties need special handling; in particular, the .name and .length properties of the newly-created function should not be overwritten by similarly-named properties from the previous function. They should be left alone so their values correspond with the function source code.

.prototype.constructor needs special handling

The .prototype property should be overwritten, and .prototype.constructor should be examined:

• If it is the old function, it should be set to the new function.
• If it is not the old function, some warning should be issued to alert the user to update it manually if desired.

Need to clean up existing mess

We should run a sweep of everything accessible via $ to make sure that .name, .length and .prototype.constructor are correct, because they are now wrong in many cases.

Should converting function to non-function remove .name and .length?

Saving

@copy_properties true
{}

over an existing function works as expected. But maybe it should not preserve .name and .length? What, if anything, should it do with .prototype.constructor, which will be preserving a reference to the previous (or some earlier) function value?

Attempting to autocomplete "$.utils.sel" offers choices "selector" and "Selector" (reasonable enough), but pressing tab then chooses neither but instead deletes "sel", leaving the input as "$.utils."

In ES5 (and V8), Array.prototype.concat.call('foo') returns an array containing a String object (i.e., boxed string)—it's equivalent to [new String('foo')]. In our interpreter, it returns ['foo'].

Since we forbid boxed primitives, the new String() call throws TypeError. The concat call should too.

At the moment, our implementation of Array.prototype.concat calls intrp.toObject, which notably does not throw, and then does nothing further with the result.

Object.setPrototypeOf crashes when given null as second parameter:

> Object.setPrototypeOf({}, null);

TypeError: Cannot read property 'properties' of null
    at intrp.Object.setPrototypeOf (/Users/cpcallen/src/CodeCity/server/interpreter.js:3998:50)
    at intrp.NativeFunction.call (/Users/cpcallen/src/CodeCity/server/interpreter.js:732:18)
    at Interpreter.stepFuncs_.Call (/Users/cpcallen/src/CodeCity/server/interpreter.js:5659:14)
    at Interpreter.stepFuncs_.CallExpression (/Users/cpcallen/src/CodeCity/server/interpreter.js:5622:29)
    at Interpreter.run (/Users/cpcallen/src/CodeCity/server/interpreter.js:309:38)

If p.isPrototypeOf(o) and p.prop exists but o.prop doesn't, then setting the value of o.prop by navigating to that selector and saving a new value creates o.prop but does not update the object panel, making it looks like the inherited value of p.prop has been modified.

At the moment, the browser history for code tabs is not too useful:

It would be good if the window title included the current selector, and was updated as one browses. I think it would be fine if it omitted any other text, but if it does have other text I think it would be best for it to be in the form of a short prefix, such as "CC:" or "Code:", or—when we have multiple instances—the instance name. We don't want to make the title longer than necessary, and we don't want a suffix since that might make it more difficult to spot the most interesting bit of each selector in left-justified list.

A nice refinement would be to intelligently shorten long selectors, perhaps by:

• Omitting the middle of very long single components to start with (especially helpful in the case of the $.userDatabase entries seen above), then if the result is still 'too long',
• omitting any repeated cycles (.prototype.constructor.prototype.constructor…), then
• omitting all but the last few components.

When the user tries to eval code containing a syntax error—e.g.:

;x?

the syntax error is logged by the server to the console, but not reported to the user in any way; they get no feedback from their command at all.

Using the IDE to view inherited values works great, but the effect of editing one is not obvious.

At the moment, if you modify the displayed value and then save it, the editor creates an overriding property on the object being edited. This is perfectly reasonable behaviour (though note that, per #257, the editor gives bad feedback about what has just happened at the moment).

It might alternatively be useful or even expected that editing the inherited property actually modifies the value on the (appropriate) prototype object. To that end:

• There should be some way for the user to choose which object the value will be saved on, in the range of (prototype object from which it is currently inherited to this object).
• Maybe the user should be forced to decide (or at least make an initial decision) before they are actually able to edit the value.

When saving a function fails due to a syntax error, the line number of the error as reported in the butter bar is wrong. It appears to be offset by 5 lines, presumably due to the magic header lines:

// @copy_properties true
// @set_prop verb = "whatever"
// @set_prop dobj = "any"
// @set_prop prep = "none"
// @set_prop iobj = "none"

Per dogfooding log, apparently on 2019-01-31 I created an eval server on port 9999, which simply accepts JS expressions via text-mode telnet and evaluates them as root.

This should be shut down, since it has no access controls whatsoever.

The MobWrite server is written in python, and was adopted by CodeCity. It has numerous python style issues, raised in PR #198, including:

• Extensive use of global variables.
• Generally poor object-orientation / encapsulation.
• Unnecessarily complicated __init__ signatures.
• Using try/finally instead of with for locking, etc.

Rewrite it conform to the style guide (or alternatively rewrite it to conform to the other style guide).

To reproduce:

1. Start with fresh demo database.
2. Log in.
3. Eval ;$.myRoom = Object.create($.room); $.myRoom.name = 'My Room'; me.moveTo($.myRoom)
4. You should be in an empty room, with "=> undefined" as the eval result at the top of the screen.
5. Resize browser window. Note that the contents of Hangout suddenly appear in the main panel!
6. Type look, and observe that you are actually alone, as expected.
7. Eval `me.moveTo($.startRoom)
8. Note that you are back in Hangout.
9. Resize browser window. Note that you now appear to be in an empty room!
10. Type look, and observe that you are not actually alone.

At the moment the code editor will often insert tab characters when one presses the tab key. This is probably not what we want, especially as the IDE code editor treats tabs as two spaces, while the old in-world editor treats them as four spaces.

Suggested arrangement to aim for:

• Tab key automagically indents to correct place or, if already there, indents by two spaces.
• Tab width set to 8, so it's really obvious when there are actual tab characters present.
• Code editor and (maybe) JSSP editor, but not value or string editors, auto-replace tabs with spaces on save. (Or do a full pretty-print, to enforce a consistent style.)

At the moment we use a shared secret between loginServer and connectServer to allow the latter to verify that the former has verified the user's identity. If the sandbox is breached this shared secret makes it pretty easy to sign IDs (real or fictitious).

If we move loginServer off the GCE instance, we should also switch to signing IDs using an asymmetric cipher instead. This would require someone attempting to use a false or stolen ID to actually modify loginServer to have it accepted, thus leaving behind some evidence of ongoing unauthorised access.